diff --git a/crates/origin-chain-governance/src/lib.rs b/crates/origin-chain-governance/src/lib.rs index 2d5308a..c82a5a6 100644 --- a/crates/origin-chain-governance/src/lib.rs +++ b/crates/origin-chain-governance/src/lib.rs @@ -316,6 +316,26 @@ pub mod pallet { pub type AdvisorFlags = StorageMap<_, Blake2_128Concat, T::AccountId, (), OptionQuery>; + /// Marca (presence) de que una propuesta fue rechazada POR VETO del council + /// (no por voto popular). Audit 2026-07-04: `override_council_veto` solo veía + /// `status == Rejected`, pero una propuesta rechazada por la comunidad (voto + /// en contra en `finalize_proposal`) también queda `Rejected` — y podía ser + /// resucitada a `Approved` por el override, invirtiendo el voto popular. Solo + /// las propuestas con esta marca (vetadas por el council) son override-elegibles. + #[pallet::storage] + #[pallet::getter(fn veto_rejected_of)] + pub type VetoRejected = + StorageMap<_, Blake2_128Concat, ProposalId, (), OptionQuery>; + + /// Marca (presence) de que una propuesta Approved YA fue desembolsada. Audit + /// 2026-07-04: `treasury_disbursement` no marcaba la propuesta como gastada, + /// así que la MISMA propuesta Approved podía re-desembolsarse N veces (drenaje + /// por replay). Esta marca lo convierte en single-use. + #[pallet::storage] + #[pallet::getter(fn disbursed_of)] + pub type Disbursed = + StorageMap<_, Blake2_128Concat, ProposalId, (), OptionQuery>; + // FIX-6 (V2-CA2/V3-C3): la tesorería ya NO es un contador `u128` desconectado. // El balance real vive en pallet_balances, en la cuenta-tesorería derivada del // PalletId (`Self::treasury_account()`). Esto cierra el hallazgo del audit de @@ -516,6 +536,12 @@ pub mod pallet { TreasuryInsufficientBalance, /// CHN-4 5.8 v0.0.35 canonical: proposal NO canonical Approved (cannot disburse Treasury). ProposalNotApproved, + /// Audit 2026-07-04: la propuesta Rejected NO fue vetada por el council + /// (fue rechazada por voto popular) — el override solo aplica a vetos. + ProposalNotVetoRejected, + /// Audit 2026-07-04: la propuesta Approved ya fue desembolsada (single-use, + /// anti-replay del treasury_disbursement). + AlreadyDisbursed, } /// Minimum stake canonical proposal submission per CHN-4 5.3. @@ -802,28 +828,39 @@ pub mod pallet { Error::::AlreadyVetoed ); - // Step 6: insert vote + increment count + // Step 6: insert vote VetoVotes::::insert(proposal_id, &who, ()); - let new_count = VetoCount::::get(proposal_id).saturating_add(1); - VetoCount::::insert(proposal_id, new_count); - // Step 7: check threshold canonical + // Step 7: cuenta SOLO los vetos de miembros AÚN en el council (audit + // 2026-07-04). Antes se usaba un contador MONÓTONO (VetoCount++), así + // que (a) el veto de un miembro REMOVIDO seguía contando, y (b) al + // encoger el council el umbral bajaba retroactivamente → una minoría + // (o ex-miembros) podía vetar, derrotando el safeguard 2/3. El conteo + // en vivo (Council ≤ 7 ⇒ O(7)) elimina los votos huérfanos. + let live_count = VetoVotes::::iter_prefix(proposal_id) + .filter(|(acct, _)| Council::::contains_key(acct)) + .count() as u32; + VetoCount::::insert(proposal_id, live_count); // cache = lo vivo + let required = council_veto_threshold(council_size); - if new_count >= required { + if live_count >= required { // Veto threshold reached canonical → flip status canonical proposal.status = ProposalStatus::Rejected; Proposals::::insert(proposal_id, proposal); + // Marca: rechazado por VETO del council (no por voto popular) → + // única vía elegible para override_council_veto. + VetoRejected::::insert(proposal_id, ()); Self::deposit_event(Event::ProposalVetoed { id: proposal_id, - veto_count: new_count, + veto_count: live_count, council_size, }); } else { Self::deposit_event(Event::CouncilVetoVoteCast { id: proposal_id, voter: who, - current_veto_count: new_count, + current_veto_count: live_count, required_threshold: required, }); } @@ -902,6 +939,15 @@ pub mod pallet { Error::::ProposalNotRejected ); + // Audit 2026-07-04: el override SOLO revierte un VETO del council, no + // un rechazo por voto popular. Sin esta marca, una propuesta que la + // comunidad votó EN CONTRA (Rejected vía finalize_proposal) podía + // resucitarse a Approved, invirtiendo el voto. Solo las vetadas aplican. + ensure!( + VetoRejected::::contains_key(proposal_id), + Error::::ProposalNotVetoRejected + ); + ensure!( !AdvisorFlags::::contains_key(&who), Error::::AdvisorMayNotVote @@ -1038,6 +1084,17 @@ pub mod pallet { Error::::ProposalNotApproved ); + // Audit 2026-07-04: single-use. Sin esta marca, la MISMA propuesta + // Approved podía re-desembolsarse N veces (drenaje del treasury por + // replay) — el guard `status == Approved` no cambiaba tras el pago. + ensure!( + !Disbursed::::contains_key(proposal_id), + Error::::AlreadyDisbursed + ); + // Marcar ANTES del transfer (defensa en profundidad; los pallets no + // reentran, pero el marker refleja la intención de gasto atómica). + Disbursed::::insert(proposal_id, ()); + // FIX-6: transfer REAL de OGC desde la cuenta-tesorería al recipient // (cierra el hallazgo de FIX-5: antes solo decrementaba un contador // u128 desconectado de pallet_balances). Si la tesorería no tiene diff --git a/crates/origin-chain-governance/src/tests_integration.rs b/crates/origin-chain-governance/src/tests_integration.rs index 2a797f9..a27ef38 100644 --- a/crates/origin-chain-governance/src/tests_integration.rs +++ b/crates/origin-chain-governance/src/tests_integration.rs @@ -857,6 +857,105 @@ fn treasury_scenario_4_successful_disbursement_canonical() { }); } +// ─── Regresión audit 2026-07-04: hardening de governance ──────────── + +#[test] +fn override_no_puede_revertir_un_rechazo_por_voto_popular() { + // CRÍTICO: una propuesta que la COMUNIDAD votó en contra (Rejected vía + // finalize) NO es override-elegible — solo los VETOS del council lo son. + // Antes, el override solo veía `status == Rejected` y podía resucitarla. + new_test_ext().execute_with(|| { + let id = submit_default(alice_origin()); + // La comunidad la rechaza en las urnas (Nay > Aye). + assert_ok!(Governance::cast_vote(bob_origin(), id, VoteStance::Aye, 100)); + assert_ok!(Governance::cast_vote(charlie_origin(), id, VoteStance::Nay, 900)); + run_to_block(200); + assert_ok!(Governance::finalize_proposal(alice_origin(), id)); + let p = Proposals::::get(id).unwrap(); + assert!(matches!(p.status, ProposalStatus::Rejected), "rechazada por voto"); + + // El override DEBE rechazarse: no fue un veto del council. + assert_noop!( + Governance::override_council_veto(bob_origin(), id, u128::MAX), + Error::::ProposalNotVetoRejected + ); + // Sigue Rejected — el voto popular se respeta. + assert!(matches!( + Proposals::::get(id).unwrap().status, + ProposalStatus::Rejected + )); + }); +} + +#[test] +fn treasury_disbursement_es_single_use_anti_replay() { + // La MISMA propuesta Approved no puede re-desembolsarse (drenaje por replay). + new_test_ext().execute_with(|| { + use frame_support::dispatch::RawOrigin; + assert_ok!(Governance::treasury_deposit(alice_origin(), 1_000_000)); + let id = submit_default(alice_origin()); + assert_ok!(Governance::cast_vote(bob_origin(), id, VoteStance::Aye, 1000)); + run_to_block(200); + assert_ok!(Governance::finalize_proposal(alice_origin(), id)); + + // Primer desembolso: OK. + assert_ok!(Governance::treasury_disbursement( + RawOrigin::Root.into(), + id, + bob_account(), + 300_000, + )); + // Segundo desembolso de la MISMA propuesta: rechazado. + assert_noop!( + Governance::treasury_disbursement(RawOrigin::Root.into(), id, bob_account(), 300_000), + Error::::AlreadyDisbursed + ); + // El treasury solo perdió UN desembolso. + assert_eq!(Governance::treasury_balance(), 700_000); + }); +} + +#[test] +fn veto_no_cuenta_votos_de_miembros_removidos() { + // El veto de un miembro removido NO cuenta (conteo en vivo). Antes, el + // contador monótono dejaba que votos huérfanos + shrink del council + // permitieran a una minoría vetar. + new_test_ext().execute_with(|| { + use crate::pallet::{CouncilRole, CouncilSize, VetoCount}; + use frame_support::dispatch::RawOrigin; + + // Council de 3 (threshold 2/3 = 2). + for (acct, _) in [ + (alice_account(), ()), + (bob_account(), ()), + (charlie_account(), ()), + ] { + assert_ok!(Governance::add_council_member( + RawOrigin::Root.into(), + acct, + CouncilRole::Member, + )); + } + let id = submit_default(alice_origin()); + + // Alice veta (1 vivo < 2). Luego Alice es removida del council. + assert_ok!(Governance::veto_proposal(alice_origin(), id)); + assert_ok!(Governance::remove_council_member( + RawOrigin::Root.into(), + alice_account() + )); + assert_eq!(CouncilSize::::get(), 2); + + // Bob veta: el voto de Alice (removida) NO cuenta ⇒ live_count = 1 < 2. + assert_ok!(Governance::veto_proposal(bob_origin(), id)); + assert_eq!(VetoCount::::get(id), 1, "solo el veto vivo de Bob cuenta"); + assert!( + matches!(Proposals::::get(id).unwrap().status, ProposalStatus::Voting), + "una minoría (1 vivo) NO puede vetar — sigue en Voting" + ); + }); +} + // ─── Scenario Council 7: 2/3 threshold canonical compile-detect ───── #[test]