fix(dao): audit H-2 + H-5 + H-6 (per memory/audit-aldabra-dao-2026-05-05)
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:
parent
101c85c0a0
commit
9556b7812d
3 changed files with 128 additions and 43 deletions
|
|
@ -52,19 +52,27 @@ use crate::agora::stake::Credential;
|
|||
use crate::config::{DaoConfig, DaoNetwork};
|
||||
use crate::error::{DaoError, DaoResult};
|
||||
|
||||
/// Generous default ExUnits — we burn slightly higher fees but avoid
|
||||
/// "missing budget" rejections. Refine via Koios `tx_evaluate` later.
|
||||
/// Per-script ExUnits budget for proposal_create.
|
||||
///
|
||||
/// Same shape as `aldabra_core::DEFAULT_EX_UNITS` but two of them
|
||||
/// (one for the spend, one for the mint redeemer).
|
||||
/// **AUDIT-H2 fix 2026-05-05:** Original values were 14M mem / 10G steps
|
||||
/// 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 {
|
||||
mem: 14_000_000,
|
||||
steps: 10_000_000_000,
|
||||
mem: 5_000_000,
|
||||
steps: 2_000_000_000,
|
||||
};
|
||||
|
||||
pub const PROPOSAL_CREATE_MINT_EX_UNITS: ExUnits = ExUnits {
|
||||
mem: 14_000_000,
|
||||
steps: 10_000_000_000,
|
||||
mem: 2_000_000,
|
||||
steps: 1_000_000_000,
|
||||
};
|
||||
|
||||
/// Conway-era min UTxO floor we apply to script outputs. Real value
|
||||
|
|
|
|||
|
|
@ -179,9 +179,14 @@ pub async fn discover_scripts(
|
|||
|
||||
// 2. StakeST policy from any stake UTxO at stakes_addr.
|
||||
//
|
||||
// A stake UTxO carries (gov_token, qty) + (stake_st_token, 1). Filter to
|
||||
// ones that have BOTH our gov-token AND a non-gov-token asset; the
|
||||
// non-gov-token's policy_id is the StakeST policy.
|
||||
// A stake UTxO carries (gov_token, qty) + (stake_st_token, 1).
|
||||
//
|
||||
// **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 {
|
||||
Ok(infos) => {
|
||||
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 {
|
||||
continue;
|
||||
}
|
||||
if let Some(other) = assets.iter().find(|a| a.policy_id != cfg.gov_token_policy) {
|
||||
found_stake_st = Some(other.policy_id.clone());
|
||||
// Match on asset_name == stakes_validator_hash (StakeST tokens
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
|
|
@ -206,7 +216,8 @@ pub async fn discover_scripts(
|
|||
report.stake_st_policy = Some(p);
|
||||
} else {
|
||||
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"
|
||||
.into(),
|
||||
);
|
||||
|
|
@ -388,6 +399,7 @@ mod tests {
|
|||
let cfg = sulkta_cfg();
|
||||
let mut responses = std::collections::HashMap::new();
|
||||
// A fake stake UTxO at stakes_addr carrying gov-token + StakeST.
|
||||
// StakeST asset_name == Sulkta stake validator hash (per H-6 fix).
|
||||
responses.insert(
|
||||
cfg.stakes_addr.clone(),
|
||||
vec![AddressInfo {
|
||||
|
|
@ -403,6 +415,7 @@ mod tests {
|
|||
},
|
||||
UtxoAsset {
|
||||
policy_id: "732ff23ade752d46c903c16866b0cb3e2e977216db594bb47c434696".into(),
|
||||
// asset_name MUST match the stakes_addr's script hash for H-6 to pass:
|
||||
asset_name: "f70e7830cde5fc7288f8a4892c148eebbb7a982b5413dd32712d3da4".into(),
|
||||
quantity: "1".into(),
|
||||
},
|
||||
|
|
@ -528,4 +541,59 @@ mod tests {
|
|||
apply_discovery(&mut cfg, &report);
|
||||
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")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1602,35 +1602,44 @@ impl WalletService {
|
|||
.ok_or_else(|| format!("governor utxo {governor_utxo_ref} no longer present on chain"))?
|
||||
.lovelace;
|
||||
|
||||
let wallet_utxos = self
|
||||
.inner
|
||||
.chain
|
||||
.get_utxos(&self.inner.address)
|
||||
.await
|
||||
.map_err(|e| format!("koios get wallet utxos: {e}"))?
|
||||
.into_iter()
|
||||
.map(|u| DaoWalletUtxo {
|
||||
tx_hash_hex: u.tx_hash,
|
||||
output_index: u.output_index,
|
||||
lovelace: u.lovelace,
|
||||
// Chain backend gives `assets: BTreeMap<concatenated_key, qty>`
|
||||
// where the key is `policy_id_hex || asset_name_hex` with the
|
||||
// policy taking the first 56 chars (28 bytes). Split for
|
||||
// pallas-txbuilder which wants the parts separate.
|
||||
assets: u
|
||||
.assets
|
||||
.into_iter()
|
||||
.filter_map(|(k, q)| {
|
||||
if k.len() >= 56 {
|
||||
let (p, n) = k.split_at(56);
|
||||
Some((p.to_string(), n.to_string(), q))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect(),
|
||||
})
|
||||
.collect();
|
||||
let wallet_utxos: Vec<DaoWalletUtxo> = {
|
||||
let raw = self
|
||||
.inner
|
||||
.chain
|
||||
.get_utxos(&self.inner.address)
|
||||
.await
|
||||
.map_err(|e| format!("koios get wallet utxos: {e}"))?;
|
||||
|
||||
// AUDIT-H5 fix: assets in the chain backend are
|
||||
// `BTreeMap<policy_id_hex || asset_name_hex, qty>`. Previous
|
||||
// implementation silently dropped any key < 56 chars via filter_map
|
||||
// — that could let a corrupt Koios response burn assets on submit.
|
||||
// Now: any malformed key surfaces as an explicit error.
|
||||
let mut out = Vec::with_capacity(raw.len());
|
||||
for u in raw {
|
||||
let mut assets = Vec::with_capacity(u.assets.len());
|
||||
for (k, q) in u.assets {
|
||||
if k.len() < 56 {
|
||||
return Err(format!(
|
||||
"malformed asset key in wallet utxo {tx_hash}#{idx}: \
|
||||
{k:?} is {len} chars, need ≥ 56 (policy_id_hex || asset_name_hex)",
|
||||
tx_hash = u.tx_hash,
|
||||
idx = u.output_index,
|
||||
len = k.len(),
|
||||
));
|
||||
}
|
||||
let (p, n) = k.split_at(56);
|
||||
assets.push((p.to_string(), n.to_string(), q));
|
||||
}
|
||||
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.
|
||||
// For Sulkta the values are known from the audit; user must pass
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue