From a0daadf38e73dfa42425dd32ba676c3e61c039da Mon Sep 17 00:00:00 2001 From: Kayos Date: Wed, 6 May 2026 08:06:44 -0700 Subject: [PATCH] =?UTF-8?q?fix(dao):=20audit=20punch=20list=20=E2=80=94=20?= =?UTF-8?q?H-1=20to=20H-4=20+=20M-2=20+=20pallas=20bump?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lands the high-priority fixes from the 2026-05-06 audit before any mainnet submit of the new vote/cosign/advance/destroy txs. ## H-1: Locked→Finished gate (MCP tool) `dao_proposal_advance_unsigned` now refuses LockedToFinished unless `tx_lower_ms > executing_end`. During the executing period the validator demands gstMoved=true (governor input present); builder doesn't include the governor input, so a tx in that window would fee-burn. The proper Locked→Finished + GAT-mint flow is Phase 4c-bis; this gate keeps us out of the broken middle. ## H-2 + H-4: strict-boundary + tx-upper-inside-period (MCP tool) Validator's pgetRelation is strict on PAfter (`period_end < lb`) and demands `ub <= period_end` on PWithin. Tool now picks PWithin only when `tx_lower_ms >= period_start && tx_upper_ms <= period_end`, PAfter only when `tx_lower_ms > period_end` strictly, and explicit- errors on the boundary-straddling case (when tx validity range crosses out of the target period). Same logic mirrored for the VotingReady→Locked + VotingReady→Finished branches. ## H-3: vote builder lower-bound preflight (MCP tool) `dao_proposal_vote_unsigned` previously checked only validity_upper vs voting_end_ms. Validator demands BOTH `voting_start <= lb` AND `ub <= voting_end`. Vote-too-early would hit "too early or invalid" script error. New preflight on tx_lower_ms vs voting_start. ## M-2: DRep deposit pulled from ProtocolParams Hardcoded constant (500 ADA) was wrong if the protocol changes drep_deposit OR if the DRep was originally registered at a different deposit amount (deregistration must match registration). Added `drep_deposit_lovelace: u64` to ProtocolParams (default 500 ADA), governance.rs build_signed_drep_registration / deregistration now read from params instead of the constant. Constant kept for backward compat with a doc note pointing at the params field. ## Pallas fork bump 507fd9da → 8091abd1 M-4 from the audit landed on the fork: voting_procedures builder debug_assert_ne!s against empty CBOR map (0xa0) and docs the upstream NonEmptyKeyValuePairs::decode footgun. L-1 from the audit was a false finding — the audit subagent misread the constants. PROPOSAL_CREATE_*_EX_UNITS are already at the post-2026-05-05-H-2 values (5M mem / 2G steps per spend, 2M / 1G per mint). The new builders alias these correctly. No change needed. --- Cargo.lock | 14 ++--- crates/aldabra-core/src/governance.rs | 26 ++++---- crates/aldabra-core/src/tx.rs | 6 ++ crates/aldabra-mcp/src/tools.rs | 90 ++++++++++++++++++++++++--- 4 files changed, 110 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 56fc282..ba7dcce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1253,7 +1253,7 @@ checksum = "c08d65885ee38876c4f86fa503fb49d7b507c2b62552df7c70b2fce627e06381" [[package]] name = "pallas-addresses" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "base58", "bech32", @@ -1268,7 +1268,7 @@ dependencies = [ [[package]] name = "pallas-codec" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "hex", "minicbor", @@ -1279,7 +1279,7 @@ dependencies = [ [[package]] name = "pallas-crypto" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "cryptoxide", "hex", @@ -1293,7 +1293,7 @@ dependencies = [ [[package]] name = "pallas-primitives" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "base58", "bech32", @@ -1308,7 +1308,7 @@ dependencies = [ [[package]] name = "pallas-traverse" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "hex", "itertools", @@ -1324,7 +1324,7 @@ dependencies = [ [[package]] name = "pallas-txbuilder" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "hex", "pallas-addresses", @@ -1341,7 +1341,7 @@ dependencies = [ [[package]] name = "pallas-wallet" version = "0.32.1" -source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#507fd9da15f1239ff2df866e0d7601d4518e83a3" +source = "git+ssh://git@192.168.0.5:23/Sulkta-Coop/pallas.git?branch=feat-aux-data#8091abd1b45c716453b7360def29311cf4600c0d" dependencies = [ "bech32", "bip39", diff --git a/crates/aldabra-core/src/governance.rs b/crates/aldabra-core/src/governance.rs index 84db3bb..2cae9c1 100644 --- a/crates/aldabra-core/src/governance.rs +++ b/crates/aldabra-core/src/governance.rs @@ -36,8 +36,11 @@ use crate::tx::InputUtxo; use crate::{Network, PaymentKey, ProtocolParams, StakeKey, WalletError}; /// Conway DRep registration deposit. Mainnet protocol parameter -/// `drep_deposit` is currently 500 ADA. Caller can pass an override -/// via `params` if a hardfork changes it; default constant here. +/// `drep_deposit` is currently 500 ADA. **Use `params.drep_deposit_lovelace` +/// instead of this constant** — it's kept here for backward-compat callers +/// only. AUDIT-2026-05-06 M-2: hardcoding the deposit means a protocol +/// change (or an old DRep registered at a different deposit) will silently +/// fail ledger validation. Always pull from current chain params. pub const DREP_REGISTRATION_DEPOSIT_LOVELACE: u64 = 500_000_000; /// Two witnesses (payment + stake) — same overhead as @@ -263,7 +266,7 @@ pub fn build_signed_drep_registration( } }; - let cert = Certificate::RegDRepCert(drep_credential, DREP_REGISTRATION_DEPOSIT_LOVELACE, anchor); + let cert = Certificate::RegDRepCert(drep_credential, params.drep_deposit_lovelace, anchor); let cert_bytes = minicbor::to_vec(&cert) .map_err(|e| WalletError::Derivation(format!("encode RegDRep cert: {e}")))?; @@ -274,7 +277,7 @@ pub fn build_signed_drep_registration( available_utxos, change_address_bech32, vec![cert_bytes], - DREP_REGISTRATION_DEPOSIT_LOVELACE, + params.drep_deposit_lovelace, params, ) } @@ -483,16 +486,15 @@ pub fn build_signed_drep_deregistration( ) -> Result, WalletError> { let stake_pkh = stake_key.public_key_hash(); let drep_credential = StakeCredential::AddrKeyhash(stake_pkh); - let cert = Certificate::UnRegDRepCert(drep_credential, DREP_REGISTRATION_DEPOSIT_LOVELACE); + let cert = Certificate::UnRegDRepCert(drep_credential, params.drep_deposit_lovelace); let cert_bytes = minicbor::to_vec(&cert) .map_err(|e| WalletError::Derivation(format!("encode UnRegDRep cert: {e}")))?; - // Negative deposit — we get it back. Two-pass fee accounts for it - // by leaving `deposit` at 0 here and letting the wallet output absorb - // the refund. Note: pallas-txbuilder writes the deposit as a negative - // contribution implicitly via the cert; the change calc here just - // needs to know we DON'T owe the protocol anything. Caller should - // expect their wallet output to grow by 500 ADA - fee. + // Refund equals the deposit originally paid. Critical: this MUST match + // what the DRep was originally registered with, not "current chain + // drep_deposit." If the protocol changed deposit between registration + // and deregistration, caller needs to override `params.drep_deposit_lovelace` + // to the original-registration value. Otherwise ledger silently fails. sign_cert_tx_with_refund( payment_key, stake_key, @@ -500,7 +502,7 @@ pub fn build_signed_drep_deregistration( available_utxos, change_address_bech32, vec![cert_bytes], - DREP_REGISTRATION_DEPOSIT_LOVELACE, + params.drep_deposit_lovelace, params, ) } diff --git a/crates/aldabra-core/src/tx.rs b/crates/aldabra-core/src/tx.rs index 3957204..a078ef8 100644 --- a/crates/aldabra-core/src/tx.rs +++ b/crates/aldabra-core/src/tx.rs @@ -79,6 +79,11 @@ pub struct ProtocolParams { /// `None`, Plutus paths skip script_data_hash and the chain will /// reject with `PPViewHashesDontMatch`. pub plutus_v3_cost_model: Option>, + /// Conway DRep registration deposit (ledger param `drep_deposit`). + /// Mainnet default: 500 ADA. Used by `governance::build_signed_drep_*`. + /// Pass the chain's current value when constructing — registering + /// with the wrong amount fails ledger validation silently. + pub drep_deposit_lovelace: u64, } impl Default for ProtocolParams { @@ -95,6 +100,7 @@ impl Default for ProtocolParams { // or fetched from `epoch_params`. None by default keeps // the ada-only / mint paths zero-cost. plutus_v3_cost_model: None, + drep_deposit_lovelace: 500_000_000, } } } diff --git a/crates/aldabra-mcp/src/tools.rs b/crates/aldabra-mcp/src/tools.rs index 33b33c3..bf67a14 100644 --- a/crates/aldabra-mcp/src/tools.rs +++ b/crates/aldabra-mcp/src/tools.rs @@ -2243,31 +2243,81 @@ impl WalletService { .ok_or_else(|| format!("tip response missing abs_slot: {tip_resp}"))?; let tip_ms = mainnet_slot_to_posix_ms(tip_slot)?; - // Compute the transition based on current status + tip time vs windows. + // Compute the transition from current status + tx-validity vs window + // boundaries. The validator (Proposal/Scripts.hs PAdvanceProposal) + // checks `getTimingRelation period`, which evaluates to: + // - PWithin if `period_start <= lb && ub <= period_end` + // - PAfter if `period_end < lb` (strict) + // - script-error otherwise (including the boundary-straddling case) + // So our tx validity range [tip_ms, tip_ms + 1799s] must fully sit + // either inside the period OR strictly after period_end. Any + // straddle = waste of fees. + // + // AUDIT-2026-05-06 H-1/H-2/H-4 fixes: use STRICT > on PAfter + // boundary, require tx-upper to land inside the target period for + // PWithin, AND gate Locked→Finished on tx_lower > executing_end so + // we never hit the "missing GAT-mint" path. use aldabra_dao::agora::proposal::ProposalStatus as PS; + const VALIDITY_RANGE_MS: i64 = aldabra_dao::builder::proposal_create::VALIDITY_RANGE_SLOTS as i64 * 1000; + let tx_lower_ms = tip_ms; + let tx_upper_ms = tip_ms + VALIDITY_RANGE_MS; let st = target.datum.starting_time; let tc = &target.datum.timing_config; let drafting_end = st + tc.draft_time; let voting_end = drafting_end + tc.voting_time; let locking_end = voting_end + tc.locking_time; + let executing_end = locking_end + tc.executing_time; let transition = match target.datum.status { PS::Draft => { - if tip_ms < drafting_end { + if tx_lower_ms >= st && tx_upper_ms <= drafting_end { + // Fully inside drafting period — happy path. AdvanceTransition::DraftToVotingReady - } else { + } else if tx_lower_ms > drafting_end { + // Strictly after — failed-too-late path. AdvanceTransition::DraftToFinished + } else { + return Err(format!( + "tx validity range [{tx_lower_ms}, {tx_upper_ms}] ms straddles drafting \ + period boundary [{st}, {drafting_end}]; wait ~{} ms for tx upper to clear, \ + OR refuse to advance until status is past Draft", + drafting_end.saturating_sub(tx_lower_ms) + )); } } PS::VotingReady => { - // Window for V→L is [voting_end, locking_end]. After that → Finished. - if tip_ms < locking_end { + if tx_lower_ms >= voting_end && tx_upper_ms <= locking_end { AdvanceTransition::VotingReadyToLocked - } else { + } else if tx_lower_ms > locking_end { AdvanceTransition::VotingReadyToFinished + } else if tx_lower_ms < voting_end { + return Err(format!( + "tx validity range starts at {tx_lower_ms} ms, before voting_end \ + {voting_end} ms — voting window not yet closed; cannot advance to Locked" + )); + } else { + return Err(format!( + "tx validity range [{tx_lower_ms}, {tx_upper_ms}] ms straddles locking \ + period boundary [{voting_end}, {locking_end}]; wait ~{} ms for tx upper \ + to clear", + locking_end.saturating_sub(tx_lower_ms) + )); + } + } + PS::Locked => { + if tx_lower_ms > executing_end { + AdvanceTransition::LockedToFinished + } else { + return Err(format!( + "tip too early to advance Locked→Finished without GAT mint — executing \ + period ends at {executing_end} ms, currently {tx_lower_ms} ms (~{} ms \ + remaining). The GAT-mint Locked→Finished path (effected proposals) is \ + Phase 4c-bis; for now the InfoOnly path requires the executing period \ + to fully elapse first.", + executing_end.saturating_sub(tx_lower_ms) + )); } } - PS::Locked => AdvanceTransition::LockedToFinished, PS::Finished => { return Err(format!( "proposal #{} is already Finished — cannot advance further", @@ -2653,6 +2703,32 @@ impl WalletService { let validity_upper_slot = tip_slot + aldabra_dao::builder::proposal_create::VALIDITY_RANGE_SLOTS; let validity_upper_ms = mainnet_slot_to_posix_ms(validity_upper_slot)?; + let tx_lower_ms = mainnet_slot_to_posix_ms(tip_slot)?; + + // AUDIT-2026-05-06 H-3 fix: validator (Proposal/Scripts.hs PVote + // ~L511) demands `pgetRelation == PWithin VotingPeriod`, where + // PWithin requires BOTH `voting_start <= lb` AND `ub <= voting_end`. + // The builder's existing preflight only verified the upper bound; + // a vote-too-early call (tip < voting_start) would burn fees on a + // "too early or invalid" script error. Catch lb-vs-voting_start + // here too. + let voting_start_check = target.datum.starting_time + + target.datum.timing_config.draft_time; + let voting_end_check = voting_start_check + + target.datum.timing_config.voting_time; + if tx_lower_ms < voting_start_check { + return Err(format!( + "tx lower bound {tx_lower_ms} ms is before voting window start {voting_start_check} ms \ + (proposal #{proposal_id} draft period not over yet); wait ~{} ms", + voting_start_check.saturating_sub(tx_lower_ms) + )); + } + if validity_upper_ms > voting_end_check { + return Err(format!( + "tx upper bound {validity_upper_ms} ms is after voting window end {voting_end_check} ms \ + — voting closed for proposal #{proposal_id}" + )); + } // Wallet utxos with H-5-style asset propagation. let wallet_utxos: Vec = {