fix(dao): audit H-2 + H-5 + H-6 (per internal notes)

H-2: drop ExUnits to 5M/2G for spend, 2M/1G for mint
  Was 14M/10G each = per-tx Conway cap. With 3 plutus contracts running
  (governor spend + stake spend + ProposalST mint), total claim 42M/30G
  exceeds per-tx limit and node rejects pre-phase-2.

H-5: propagate malformed wallet asset keys instead of silently dropping
  Previous filter_map silently dropped any key < 56 chars. Could let a
  corrupt Koios response burn assets on submit. Now returns explicit
  Err with the offending UTxO + key.

H-6: tighten StakeST detection to asset_name == stake_validator_hash
  Per Stake/Scripts.hs:188-190 (pscriptHashToTokenName), StakeST
  asset_name is the stake validator's script hash. Previous code took
  "first non-gov-token asset" which would silently pick a wrong policy
  if a stake UTxO accidentally carried a junk NFT. Regression test
  h6_junk_token_does_not_pollute_stake_st_detection added.

3 of 7 audit punch-list items closed. C-1 + C-2 + C-3 next.
This commit is contained in:
Sulkta 2026-05-05 20:52:22 -07:00
parent 4644fb525f
commit 6955377c61
3 changed files with 128 additions and 43 deletions

View file

@ -52,19 +52,27 @@ use crate::agora::stake::Credential;
use crate::config::{DaoConfig, DaoNetwork}; use crate::config::{DaoConfig, DaoNetwork};
use crate::error::{DaoError, DaoResult}; use crate::error::{DaoError, DaoResult};
/// Generous default ExUnits — we burn slightly higher fees but avoid /// Per-script ExUnits budget for proposal_create.
/// "missing budget" rejections. Refine via Koios `tx_evaluate` later.
/// ///
/// Same shape as `aldabra_core::DEFAULT_EX_UNITS` but two of them /// **AUDIT-H2 fix 2026-05-05:** Original values were 14M mem / 10G steps
/// (one for the spend, one for the mint redeemer). /// each — equal to per-tx Conway max. With 3 plutus contracts firing
/// (governor spend + stake spend + ProposalST mint), the total claim
/// would exceed the per-tx cap and node rejects pre-phase-2.
///
/// The reference tx (`7c8db1432a07...`) used 1208B tx size + 573_553
/// lovelace fee, suggesting much smaller ExUnits per script. Drop to
/// ~5M mem / 2G steps each — gives ~15M / 6G total (still under the
/// 14M / 10G per-tx cap; node may bump per-tx caps in newer Conway
/// epochs). Refine via Koios `tx_evaluate` once we have a working
/// unsigned tx to evaluate.
pub const PROPOSAL_CREATE_SPEND_EX_UNITS: ExUnits = ExUnits { pub const PROPOSAL_CREATE_SPEND_EX_UNITS: ExUnits = ExUnits {
mem: 14_000_000, mem: 5_000_000,
steps: 10_000_000_000, steps: 2_000_000_000,
}; };
pub const PROPOSAL_CREATE_MINT_EX_UNITS: ExUnits = ExUnits { pub const PROPOSAL_CREATE_MINT_EX_UNITS: ExUnits = ExUnits {
mem: 14_000_000, mem: 2_000_000,
steps: 10_000_000_000, steps: 1_000_000_000,
}; };
/// Conway-era min UTxO floor we apply to script outputs. Real value /// Conway-era min UTxO floor we apply to script outputs. Real value

View file

@ -179,9 +179,14 @@ pub async fn discover_scripts(
// 2. StakeST policy from any stake UTxO at stakes_addr. // 2. StakeST policy from any stake UTxO at stakes_addr.
// //
// A stake UTxO carries (gov_token, qty) + (stake_st_token, 1). Filter to // A stake UTxO carries (gov_token, qty) + (stake_st_token, 1).
// ones that have BOTH our gov-token AND a non-gov-token asset; the //
// non-gov-token's policy_id is the StakeST policy. // **AUDIT-H6 fix 2026-05-05:** Previous logic was "first non-gov-token
// asset on a stake UTxO" — would silently pick a wrong asset if anyone
// ever sent a junk NFT to a stake UTxO (Cardano allows this). Tighten:
// the StakeST minting policy mints with `asset_name = stake validator's
// script hash` per `Stake/Scripts.hs:188-190` (`pscriptHashToTokenName`).
// Match on that explicitly.
match client.address_info(&cfg.stakes_addr).await { match client.address_info(&cfg.stakes_addr).await {
Ok(infos) => { Ok(infos) => {
let utxos = infos.into_iter().next().map(|i| i.utxo_set).unwrap_or_default(); let utxos = infos.into_iter().next().map(|i| i.utxo_set).unwrap_or_default();
@ -197,8 +202,13 @@ pub async fn discover_scripts(
if !has_gov { if !has_gov {
continue; continue;
} }
if let Some(other) = assets.iter().find(|a| a.policy_id != cfg.gov_token_policy) { // Match on asset_name == stakes_validator_hash (StakeST tokens
found_stake_st = Some(other.policy_id.clone()); // for THIS DAO's stakes will carry the stake validator hash
// as their asset name; junk tokens won't).
if let Some(stake_st) = assets.iter().find(|a| {
a.policy_id != cfg.gov_token_policy && a.asset_name == stakes_hash
}) {
found_stake_st = Some(stake_st.policy_id.clone());
break; break;
} }
} }
@ -206,7 +216,8 @@ pub async fn discover_scripts(
report.stake_st_policy = Some(p); report.stake_st_policy = Some(p);
} else { } else {
report.gaps.push( report.gaps.push(
"stake_st_policy: no stakes-addr UTxO carries (gov_token + StakeST) — \ "stake_st_policy: no stakes-addr UTxO carries (gov_token + \
StakeST_with_asset_name=stakes_validator_hash) \
either no stakes exist yet OR stakes_addr is wrong" either no stakes exist yet OR stakes_addr is wrong"
.into(), .into(),
); );
@ -388,6 +399,7 @@ mod tests {
let cfg = sulkta_cfg(); let cfg = sulkta_cfg();
let mut responses = std::collections::HashMap::new(); let mut responses = std::collections::HashMap::new();
// A fake stake UTxO at stakes_addr carrying gov-token + StakeST. // A fake stake UTxO at stakes_addr carrying gov-token + StakeST.
// StakeST asset_name == Sulkta stake validator hash (per H-6 fix).
responses.insert( responses.insert(
cfg.stakes_addr.clone(), cfg.stakes_addr.clone(),
vec![AddressInfo { vec![AddressInfo {
@ -403,6 +415,7 @@ mod tests {
}, },
UtxoAsset { UtxoAsset {
policy_id: "732ff23ade752d46c903c16866b0cb3e2e977216db594bb47c434696".into(), policy_id: "732ff23ade752d46c903c16866b0cb3e2e977216db594bb47c434696".into(),
// asset_name MUST match the stakes_addr's script hash for H-6 to pass:
asset_name: "f70e7830cde5fc7288f8a4892c148eebbb7a982b5413dd32712d3da4".into(), asset_name: "f70e7830cde5fc7288f8a4892c148eebbb7a982b5413dd32712d3da4".into(),
quantity: "1".into(), quantity: "1".into(),
}, },
@ -528,4 +541,59 @@ mod tests {
apply_discovery(&mut cfg, &report); apply_discovery(&mut cfg, &report);
assert_eq!(cfg.stake_st_policy.as_deref(), Some("preexisting")); assert_eq!(cfg.stake_st_policy.as_deref(), Some("preexisting"));
} }
/// Regression for AUDIT-H6: a stake UTxO with a junk third-party token
/// must NOT pollute StakeST detection. The StakeST is only detected
/// when its `asset_name == stakes_validator_script_hash`.
#[tokio::test]
async fn h6_junk_token_does_not_pollute_stake_st_detection() {
let cfg = sulkta_cfg();
let mut responses = std::collections::HashMap::new();
responses.insert(
cfg.stakes_addr.clone(),
vec![AddressInfo {
address: cfg.stakes_addr.clone(),
utxo_set: vec![AddressUtxo {
tx_hash: "deadbeef".repeat(8),
tx_index: 0,
asset_list: Some(vec![
UtxoAsset {
policy_id: cfg.gov_token_policy.clone(),
asset_name: cfg.gov_token_name_hex.clone(),
quantity: "50".into(),
},
// Junk NFT — wrong asset_name. Must NOT be picked.
UtxoAsset {
policy_id: "ffffffffffffffffffffffffffffffffffffffffffffffffffffffff".into(),
asset_name: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef".into(),
quantity: "1".into(),
},
// Real StakeST — asset_name matches stake validator hash.
UtxoAsset {
policy_id: "732ff23ade752d46c903c16866b0cb3e2e977216db594bb47c434696".into(),
asset_name: "f70e7830cde5fc7288f8a4892c148eebbb7a982b5413dd32712d3da4".into(),
quantity: "1".into(),
},
]),
reference_script: None,
}],
}],
);
responses.insert(
MAINNET_AGORA_SHARED_DEPLOYER.into(),
vec![AddressInfo {
address: MAINNET_AGORA_SHARED_DEPLOYER.into(),
utxo_set: vec![],
}],
);
let client = StubClient { responses };
let report = discover_scripts(&cfg, &client, &[MAINNET_AGORA_SHARED_DEPLOYER])
.await
.unwrap();
// Picks the REAL StakeST (asset_name match), not the junk NFT.
assert_eq!(
report.stake_st_policy.as_deref(),
Some("732ff23ade752d46c903c16866b0cb3e2e977216db594bb47c434696")
);
}
} }

View file

@ -1602,35 +1602,44 @@ impl WalletService {
.ok_or_else(|| format!("governor utxo {governor_utxo_ref} no longer present on chain"))? .ok_or_else(|| format!("governor utxo {governor_utxo_ref} no longer present on chain"))?
.lovelace; .lovelace;
let wallet_utxos = self let wallet_utxos: Vec<DaoWalletUtxo> = {
.inner let raw = self
.chain .inner
.get_utxos(&self.inner.address) .chain
.await .get_utxos(&self.inner.address)
.map_err(|e| format!("koios get wallet utxos: {e}"))? .await
.into_iter() .map_err(|e| format!("koios get wallet utxos: {e}"))?;
.map(|u| DaoWalletUtxo {
tx_hash_hex: u.tx_hash, // AUDIT-H5 fix: assets in the chain backend are
output_index: u.output_index, // `BTreeMap<policy_id_hex || asset_name_hex, qty>`. Previous
lovelace: u.lovelace, // implementation silently dropped any key < 56 chars via filter_map
// Chain backend gives `assets: BTreeMap<concatenated_key, qty>` // — that could let a corrupt Koios response burn assets on submit.
// where the key is `policy_id_hex || asset_name_hex` with the // Now: any malformed key surfaces as an explicit error.
// policy taking the first 56 chars (28 bytes). Split for let mut out = Vec::with_capacity(raw.len());
// pallas-txbuilder which wants the parts separate. for u in raw {
assets: u let mut assets = Vec::with_capacity(u.assets.len());
.assets for (k, q) in u.assets {
.into_iter() if k.len() < 56 {
.filter_map(|(k, q)| { return Err(format!(
if k.len() >= 56 { "malformed asset key in wallet utxo {tx_hash}#{idx}: \
let (p, n) = k.split_at(56); {k:?} is {len} chars, need 56 (policy_id_hex || asset_name_hex)",
Some((p.to_string(), n.to_string(), q)) tx_hash = u.tx_hash,
} else { idx = u.output_index,
None len = k.len(),
} ));
}) }
.collect(), let (p, n) = k.split_at(56);
}) assets.push((p.to_string(), n.to_string(), q));
.collect(); }
out.push(DaoWalletUtxo {
tx_hash_hex: u.tx_hash,
output_index: u.output_index,
lovelace: u.lovelace,
assets,
});
}
out
};
// ScriptRefs must be populated before this tool can build a tx. // ScriptRefs must be populated before this tool can build a tx.
// For Sulkta the values are known from the audit; user must pass // For Sulkta the values are known from the audit; user must pass