From ea2ee015031afdce14438c5d0e8ed7c3cd37ef7f Mon Sep 17 00:00:00 2001 From: Kayos Date: Tue, 5 May 2026 20:55:20 -0700 Subject: [PATCH] =?UTF-8?q?fix(dao):=20audit=20C-1=20+=20C-2=20+=20C-3=20?= =?UTF-8?q?=E2=80=94=20informed=20by=20reference=20tx=207c8db1432a07?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/builder/proposal_create.rs | 273 +++++++++++++++--- 1 file changed, 230 insertions(+), 43 deletions(-) diff --git a/crates/aldabra-dao/src/builder/proposal_create.rs b/crates/aldabra-dao/src/builder/proposal_create.rs index 6355535..fe59d9b 100644 --- a/crates/aldabra-dao/src/builder/proposal_create.rs +++ b/crates/aldabra-dao/src/builder/proposal_create.rs @@ -40,6 +40,7 @@ use pallas_addresses::Address; use pallas_codec::minicbor; +use pallas_codec::utils::KeyValuePairs; use pallas_crypto::hash::Hash; use pallas_primitives::PlutusData; use pallas_txbuilder::{BuildConway, ExUnits, Input, Output, StagingTransaction}; @@ -48,7 +49,9 @@ use crate::agora::governor::GovernorDatum; use crate::agora::proposal::{ ProposalDatum, ProposalStatus, ProposalThresholds, ProposalTimingConfig, ProposalVotes, }; -use crate::agora::stake::Credential; +use crate::agora::stake::{ + Credential, ProposalAction, ProposalLock, StakeDatum, StakeRedeemer, +}; use crate::config::{DaoConfig, DaoNetwork}; use crate::error::{DaoError, DaoResult}; @@ -111,6 +114,33 @@ pub struct GovernorUtxoIn { pub output_index: u32, pub lovelace: u64, pub datum: GovernorDatum, + /// 56-hex Governor State Thread (GST) policy id. The new governor + /// output must carry +1 of this token to keep the singleton invariant. + pub gst_policy_hex: String, + /// Asset name (hex) of the GST token. Empty for Sulkta. + pub gst_asset_name_hex: String, +} + +/// On-chain stake state we need to spend (proposer's existing stake). +/// +/// AUDIT-C2 fix 2026-05-05: governor's `CreateProposal` branch hard-asserts +/// `Stake input should present`. Builder MUST take a stake utxo to spend. +/// The owner of the stake's datum must equal the tx's signer (proposer_pkh). +#[derive(Debug, Clone)] +pub struct StakeUtxoIn { + pub tx_hash_hex: String, + pub output_index: u32, + pub lovelace: u64, + /// Current Terrapin/gov-token quantity on the UTxO. Must equal + /// `datum.staked_amount` per stake validator invariant. + pub gov_token_qty: u64, + /// StakeST asset name (= stake validator script hash) for the +1 token + /// the UTxO carries. Both stakes_addr UTxOs we've seen use the + /// stake-validator script hash here. + pub stake_st_asset_name_hex: String, + /// Current StakeDatum on the UTxO. We append a `Created` ProposalLock + /// to its `locked_by` field for the new stake output. + pub datum: StakeDatum, } /// Reference UTxO citing a deployed Agora script. @@ -141,6 +171,11 @@ impl ReferenceUtxo { pub struct ProposalCreateArgs { pub cfg: DaoConfig, pub governor: GovernorUtxoIn, + /// Proposer's existing stake UTxO. AUDIT-C2 — required for the + /// governor's CreateProposal branch to find a stake input. The stake's + /// owner pkh must equal `proposer_pkh`, and `staked_amount + deposit` + /// must clear `governor.proposal_thresholds.create`. + pub stake_in: StakeUtxoIn, /// Proposer's payment-credential hash (28 bytes). pub proposer_pkh: Vec, /// Proposer wallet's bech32 address (for change). @@ -148,10 +183,19 @@ pub struct ProposalCreateArgs { /// Spendable wallet UTxOs. pub wallet_utxos: Vec, /// Chain tip's POSIX time in milliseconds. Embedded in the new - /// `ProposalDatum.starting_time` field. + /// `ProposalDatum.starting_time` field. Must lie inside the tx's + /// validity range when converted to slots — caller handles the + /// slot↔ms conversion. pub starting_time_ms: i64, + /// Current chain tip slot. AUDIT-C3 — sets `valid_from_slot(tip_slot)` + /// and `invalid_from_slot(tip_slot + VALIDITY_RANGE_SLOTS)`. The + /// resulting window must satisfy + /// `governor.create_proposal_time_range_max_width` (Sulkta: 30min). + pub tip_slot: u64, /// Reference UTxO to cite for the governor validator script. pub governor_validator_ref: ReferenceUtxo, + /// Reference UTxO to cite for the stake validator script. AUDIT-C2. + pub stake_validator_ref: ReferenceUtxo, /// Reference UTxO to cite for the ProposalST minting policy script. pub proposal_st_policy_ref: ReferenceUtxo, /// Estimated total fee. v1: caller-supplied. Phase-4-late: refine @@ -159,6 +203,11 @@ pub struct ProposalCreateArgs { pub fee_lovelace: u64, } +/// Validity range width in slots. Sulkta's reference tx (`7c8db1432a07...`) +/// used 1799 slots (~30 min - 1s) which fits inside +/// `create_proposal_time_range_max_width = 1_800_000ms`. Match it. +pub const VALIDITY_RANGE_SLOTS: u64 = 1799; + /// What `build_unsigned_proposal_create` returns. #[derive(Debug, Clone)] pub struct UnsignedProposalCreate { @@ -186,6 +235,38 @@ pub fn build_unsigned_proposal_create( let new_proposal_id = args.governor.datum.next_proposal_id; let proposer_cred = Credential::PubKey(args.proposer_pkh.clone()); + // ---- preflight: stake's owner must match proposer; stake meets create-threshold ---- + // + // AUDIT-C2 + governor's `CreateProposal` invariants. Catch these + // client-side rather than waste fees on a phase-2 reject. + if !matches!(&args.stake_in.datum.owner, Credential::PubKey(h) if *h == args.proposer_pkh) { + return Err(DaoError::State(format!( + "stake owner pkh does not match proposer pkh — proposer must own the stake input" + ))); + } + let create_threshold = args.governor.datum.proposal_thresholds.create; + if (args.stake_in.datum.staked_amount as i128) < (create_threshold as i128) { + return Err(DaoError::State(format!( + "stake amount {} < create threshold {} — cosigner support not yet implemented; \ + use a wallet with stake >= threshold OR (later) pass multiple cosigner stakes", + args.stake_in.datum.staked_amount, create_threshold + ))); + } + let max_proposals_per_stake = args.governor.datum.maximum_created_proposals_per_stake; + let n_created = args + .stake_in + .datum + .locked_by + .iter() + .filter(|l| matches!(l.action, ProposalAction::Created)) + .count() as i64; + if n_created >= max_proposals_per_stake { + return Err(DaoError::State(format!( + "stake already has {} Created proposal locks; max is {}", + n_created, max_proposals_per_stake + ))); + } + // ---- pick funding + collateral --------------------------------------- // // Same rule as `aldabra_core::build_signed_plutus_spend`: smallest @@ -235,79 +316,114 @@ pub fn build_unsigned_proposal_create( }; // Proposal: fresh ProposalDatum (Draft, sole cosigner, copied params). + // + // AUDIT-C1 fix 2026-05-05: effects must be a NON-empty map with at least + // one neutral (empty inner map) entry, AND its keys must equal the votes + // map's keys. Per Agora `Governor/Scripts.hs:437-462` validators + // `phasNeutralEffect` (pany # pnull over inner maps) and + // `pisEffectsVotesCompatible` (effects keys == votes keys). + // + // For InfoOnly: both ResultTag(0) and ResultTag(1) map to empty inner + // maps (no effect scripts trigger regardless of vote outcome). + let empty_inner: PlutusData = PlutusData::Map(KeyValuePairs::from( + Vec::<(PlutusData, PlutusData)>::new(), + )); + let effects_pd = PlutusData::Map(KeyValuePairs::from(vec![ + (crate::agora::plutus_data::int(0)?, empty_inner.clone()), + (crate::agora::plutus_data::int(1)?, empty_inner), + ])); + let new_proposal = ProposalDatum { proposal_id: new_proposal_id, - // InfoOnly action — empty effects map (Map ResultTag (Map ScriptHash _)) - // encodes as a CBOR map with zero entries: `a0`. - effects_raw: PlutusData::Map(pallas_codec::utils::KeyValuePairs::from( - Vec::<(PlutusData, PlutusData)>::new(), - )), + effects_raw: effects_pd, status: ProposalStatus::Draft, cosigners: vec![proposer_cred.clone()], - // Copied verbatim from governor. thresholds: ProposalThresholds { ..args.governor.datum.proposal_thresholds.clone() }, - // Init: every result tag we care about → 0 votes. For an InfoOnly - // proposal we use two tags (yes=1, no=0) by convention — Agora's - // execute logic treats votes as a winner-take-all contest by tag. + // Vote keys MUST equal effects keys (per pisEffectsVotesCompatible). votes: ProposalVotes(vec![(0, 0), (1, 0)]), timing_config: ProposalTimingConfig { ..args.governor.datum.proposal_timings.clone() }, starting_time: args.starting_time_ms, }; + // New stake datum: copy old, append Created lock for the new proposal. + let mut new_stake = args.stake_in.datum.clone(); + new_stake.locked_by.push(ProposalLock { + proposal_id: new_proposal_id, + action: ProposalAction::Created, + }); + let new_governor_datum_pd = new_governor.to_plutus_data()?; let new_proposal_datum_pd = new_proposal.to_plutus_data()?; + let new_stake_datum_pd = new_stake.to_plutus_data()?; let new_governor_datum_cbor = minicbor::to_vec(&new_governor_datum_pd) .map_err(|e| DaoError::Cbor(format!("new governor datum encode: {e}")))?; let new_proposal_datum_cbor = minicbor::to_vec(&new_proposal_datum_pd) .map_err(|e| DaoError::Cbor(format!("new proposal datum encode: {e}")))?; + let new_stake_datum_cbor = minicbor::to_vec(&new_stake_datum_pd) + .map_err(|e| DaoError::Cbor(format!("new stake datum encode: {e}")))?; // ---- redeemers -------------------------------------------------------- // - // Spend redeemer: GovernorRedeemer::CreateProposal = Integer 0 (per - // EnumIsData encoding correction 2026-05-05). - // Mint redeemer: unit (Constr 0 []) — we don't know the exact shape - // ProposalST policy expects without source; this is the most common - // Agora pattern. Iterate via on-chain failure messages if wrong. + // Governor spend: GovernorRedeemer::CreateProposal = Integer 0 (per + // EnumIsData encoding fix 2026-05-05). + // + // Stake spend: AUDIT-C2 — the reference tx (7c8db1432a07...) shows the + // stake input being spent with the stake validator invoked. Per Agora's + // design the stake validator's DepositWithdraw branch handles "modify + // stake AND register a Created lock" when the same tx has the governor's + // CreateProposal redeemer. For an InfoOnly proposal with no deposit: + // DepositWithdraw(0). The reference tx had DepositWithdraw(200) (50→250). + // + // Mint redeemer: per `Agora/Proposal/Scripts.hs:118` the policy is + // `\_gst _redeemer ctx -> ...` — redeemer is unused. Constr 0 [] is fine. - let spend_redeemer_pd = crate::agora::plutus_data::int(0)?; - let mint_redeemer_pd = crate::agora::plutus_data::constr(0, vec![]); - - let spend_redeemer_cbor = minicbor::to_vec(&spend_redeemer_pd) - .map_err(|e| DaoError::Cbor(format!("spend redeemer encode: {e}")))?; - let mint_redeemer_cbor = minicbor::to_vec(&mint_redeemer_pd) - .map_err(|e| DaoError::Cbor(format!("mint redeemer encode: {e}")))?; + let governor_spend_redeemer_cbor = + minicbor::to_vec(&crate::agora::plutus_data::int(0)?) + .map_err(|e| DaoError::Cbor(format!("governor spend redeemer encode: {e}")))?; + let stake_spend_redeemer_cbor = + minicbor::to_vec(&StakeRedeemer::DepositWithdraw(0).to_plutus_data()?) + .map_err(|e| DaoError::Cbor(format!("stake spend redeemer encode: {e}")))?; + let mint_redeemer_cbor = + minicbor::to_vec(&crate::agora::plutus_data::constr(0, vec![])) + .map_err(|e| DaoError::Cbor(format!("mint redeemer encode: {e}")))?; // ---- balance + change ------------------------------------------------- // - // total_in = governor + funding (collateral is held separately). - // outputs = new_governor + new_proposal + change - // The new_governor preserves the OLD governor's lovelace (Agora - // convention — script UTxOs hold a stable min-UTxO floor). - // The new_proposal needs SCRIPT_OUTPUT_MIN_LOVELACE. - // Change = funding + (governor lovelace pass-through) - new_governor_lovelace - new_proposal_lovelace - fee. + // total_in = governor + stake + funding (collateral held separately). + // outputs = new_governor + new_stake + new_proposal + change. + // Change = total_in - sum(script outputs) - fee. let new_governor_lovelace = args.governor.lovelace.max(SCRIPT_OUTPUT_MIN_LOVELACE); + let new_stake_lovelace = args.stake_in.lovelace.max(SCRIPT_OUTPUT_MIN_LOVELACE); let new_proposal_lovelace = SCRIPT_OUTPUT_MIN_LOVELACE; let total_in = args .governor .lovelace - .checked_add(funding.lovelace) + .checked_add(args.stake_in.lovelace) + .and_then(|x| x.checked_add(funding.lovelace)) .ok_or_else(|| DaoError::State("input lovelace overflow".into()))?; let total_out = new_governor_lovelace - .checked_add(new_proposal_lovelace) + .checked_add(new_stake_lovelace) + .and_then(|x| x.checked_add(new_proposal_lovelace)) .and_then(|x| x.checked_add(args.fee_lovelace)) .ok_or_else(|| DaoError::State("output lovelace overflow".into()))?; let change_lovelace = total_in.checked_sub(total_out).ok_or_else(|| { DaoError::State(format!( "insufficient input: total_in={total_in} need={total_out} \ - (governor_out={new_governor_lovelace} + proposal_out={new_proposal_lovelace} + fee={})", + (governor_out={new_governor_lovelace} + stake_out={new_stake_lovelace} + \ + proposal_out={new_proposal_lovelace} + fee={})", args.fee_lovelace )) })?; - if change_lovelace > 0 && change_lovelace < SCRIPT_OUTPUT_MIN_LOVELACE { + // Wallet change can be a regular pubkey output — lower min-utxo floor. + // AUDIT-M2: previous code required script-floor (2 ADA) for wallet + // change; that's wrong, use 1 ADA (still conservative for Conway). + const WALLET_CHANGE_MIN_LOVELACE: u64 = 1_000_000; + if change_lovelace > 0 && change_lovelace < WALLET_CHANGE_MIN_LOVELACE { return Err(DaoError::State(format!( - "change lovelace {change_lovelace} below min UTxO; top up wallet or increase funding" + "change lovelace {change_lovelace} below min UTxO ({}); top up wallet or increase funding", + WALLET_CHANGE_MIN_LOVELACE ))); } @@ -319,9 +435,17 @@ pub fn build_unsigned_proposal_create( "proposal_addr not set on DaoConfig — register or discover_scripts first".into(), ) })?)?; + let stakes_addr = parse_address(&args.cfg.stakes_addr)?; let change_addr = parse_address(&args.change_address)?; - let governor_input = Input::new(parse_tx_hash(&args.governor.tx_hash_hex)?, args.governor.output_index as u64); + let governor_input = Input::new( + parse_tx_hash(&args.governor.tx_hash_hex)?, + args.governor.output_index as u64, + ); + let stake_input = Input::new( + parse_tx_hash(&args.stake_in.tx_hash_hex)?, + args.stake_in.output_index as u64, + ); let funding_input = Input::new(parse_tx_hash(&funding.tx_hash_hex)?, funding.output_index as u64); let collateral_input = Input::new( parse_tx_hash(&collateral.tx_hash_hex)?, @@ -331,6 +455,10 @@ pub fn build_unsigned_proposal_create( parse_tx_hash(&args.governor_validator_ref.tx_hash_hex)?, args.governor_validator_ref.output_index as u64, ); + let stake_validator_ref_input = Input::new( + parse_tx_hash(&args.stake_validator_ref.tx_hash_hex)?, + args.stake_validator_ref.output_index as u64, + ); let proposal_st_policy_ref_input = Input::new( parse_tx_hash(&args.proposal_st_policy_ref.tx_hash_hex)?, args.proposal_st_policy_ref.output_index as u64, @@ -341,30 +469,62 @@ pub fn build_unsigned_proposal_create( "proposal_st_policy not set on DaoConfig — register or discover_scripts first".into(), ) })?)?; + let stake_st_policy_hash = parse_script_hash(args.cfg.stake_st_policy.as_deref().ok_or_else(|| { + DaoError::Config( + "stake_st_policy not set on DaoConfig — register or discover_scripts first".into(), + ) + })?)?; + let stake_st_asset_name = hex::decode(&args.stake_in.stake_st_asset_name_hex) + .map_err(|e| DaoError::Config(format!("stake_st_asset_name_hex decode: {e}")))?; + let gov_token_policy_hash = parse_script_hash(&args.cfg.gov_token_policy)?; + let gov_token_name_bytes = hex::decode(&args.cfg.gov_token_name_hex) + .map_err(|e| DaoError::Config(format!("gov_token_name_hex decode: {e}")))?; + let gst_policy_hash = parse_script_hash(&args.governor.gst_policy_hex)?; + let gst_name_bytes = hex::decode(&args.governor.gst_asset_name_hex) + .map_err(|e| DaoError::Config(format!("gst_asset_name_hex decode: {e}")))?; let network_id = match args.cfg.network { DaoNetwork::Mainnet => 1u8, DaoNetwork::Preprod | DaoNetwork::Preview => 0u8, }; - // Inline-datum outputs use `add_output_datum` / `Output::new(..).set_inline_datum(..)`. - // Pallas-txbuilder's API: `Output::new(addr, lovelace).set_inline_datum(cbor_bytes)`. + // New governor output: same address, +1 GST, updated datum. let new_governor_output = Output::new(governor_addr, new_governor_lovelace) - .set_inline_datum(new_governor_datum_cbor.clone()); + .set_inline_datum(new_governor_datum_cbor.clone()) + .add_asset(gst_policy_hash, gst_name_bytes.clone(), 1) + .map_err(|e| DaoError::Backend(format!("add gst asset to governor output: {e}")))?; - // The new proposal output also carries 1 ProposalST token. + // New stake output: same stakes_addr, same StakeST + same gov-token qty, + // datum carries the new Created lock. + let new_stake_output = Output::new(stakes_addr, new_stake_lovelace) + .set_inline_datum(new_stake_datum_cbor.clone()) + .add_asset(stake_st_policy_hash, stake_st_asset_name.clone(), 1) + .and_then(|o| o.add_asset( + gov_token_policy_hash, + gov_token_name_bytes.clone(), + args.stake_in.gov_token_qty, + )) + .map_err(|e| DaoError::Backend(format!("add stake-output assets: {e}")))?; + + // New proposal output: ProposalST + min-utxo + datum. let new_proposal_output = Output::new(proposal_addr, new_proposal_lovelace) .set_inline_datum(new_proposal_datum_cbor.clone()) .add_asset(proposal_st_policy_hash, vec![], 1) - .map_err(|e| DaoError::Backend(format!("add proposal_st asset to output: {e}")))?; + .map_err(|e| DaoError::Backend(format!("add proposal_st asset: {e}")))?; let mut staging = StagingTransaction::new(); + // 3 regular inputs: governor (script), stake (script), funding (wallet). staging = staging.input(governor_input.clone()); + staging = staging.input(stake_input.clone()); staging = staging.input(funding_input.clone()); staging = staging.collateral_input(collateral_input); + // 3 reference inputs: governor + stake validators + ProposalST policy. staging = staging.reference_input(governor_validator_ref_input); + staging = staging.reference_input(stake_validator_ref_input); staging = staging.reference_input(proposal_st_policy_ref_input); + // 4 outputs (3 script + maybe 1 change): governor, stake, proposal, change. staging = staging.output(new_governor_output); + staging = staging.output(new_stake_output); staging = staging.output(new_proposal_output); if change_lovelace > 0 { @@ -383,15 +543,20 @@ pub fn build_unsigned_proposal_create( staging = staging.output(change_output); } - // Mint +1 ProposalST. + // Mint +1 ProposalST (asset_name = empty bytes per Sulkta convention). staging = staging .mint_asset(proposal_st_policy_hash, vec![], 1) .map_err(|e| DaoError::Backend(format!("mint_asset: {e}")))?; - // Spend redeemer for the governor input + mint redeemer for ProposalST. + // Three plutus contract invocations: spend governor, spend stake, mint ProposalST. staging = staging.add_spend_redeemer( governor_input, - spend_redeemer_cbor, + governor_spend_redeemer_cbor, + Some(PROPOSAL_CREATE_SPEND_EX_UNITS), + ); + staging = staging.add_spend_redeemer( + stake_input, + stake_spend_redeemer_cbor, Some(PROPOSAL_CREATE_SPEND_EX_UNITS), ); staging = staging.add_mint_redeemer( @@ -400,6 +565,28 @@ pub fn build_unsigned_proposal_create( Some(PROPOSAL_CREATE_MINT_EX_UNITS), ); + // AUDIT-C3 fix: tx validity range + disclosed_signer. + // + // pvalidateProposalStartingTime requires a bounded validRange ≤ + // create_proposal_time_range_max_width that includes starting_time. + // Reference tx (7c8db1432a07...) used 1799 slots = ~30 min. + // + // pauthorizedBy on the stake checks proposer's pkh appears in + // txInfoSignatories — we disclose it explicitly so pallas-txbuilder + // knows to require + emit the corresponding witness. + staging = staging.valid_from_slot(args.tip_slot); + staging = staging.invalid_from_slot(args.tip_slot + VALIDITY_RANGE_SLOTS); + + let proposer_pkh_arr: [u8; 28] = args + .proposer_pkh + .as_slice() + .try_into() + .map_err(|_| DaoError::Datum(format!( + "proposer_pkh must be 28 bytes, got {}", + args.proposer_pkh.len() + )))?; + staging = staging.disclosed_signer(Hash::<28>::from(proposer_pkh_arr)); + staging = staging.fee(args.fee_lovelace).network_id(network_id); let built = staging