From 304072173e645b476f39c090a9e3f84333f1e0cf Mon Sep 17 00:00:00 2001 From: DavidHerran Date: Sun, 5 Jul 2026 19:55:39 -0500 Subject: [PATCH] =?UTF-8?q?feat(governance):=20reserva=20REAL=20del=20stak?= =?UTF-8?q?e=20de=20propuesta=20=E2=80=94=20bond=20og/props=20(audit=20A,?= =?UTF-8?q?=20David)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decisión A (David 2026-07-04). El audit ultra encontró [HIGH] que el "mínimo 1.000 OGC" de submit_proposal era un número caller-supplied SIN respaldo: una cuenta vacía podía spamear propuestas ilimitadas, y ese `stake` fabricado era el `baseline` denominador del override_council_veto (manipulable). Fix (sin esperar a NPoS — el stake de propuesta es piel-en-el-juego anti-spam, distinto del voto pesado por stake): el stake se RETIENE de verdad en una cuenta- bond determinista `og/props` (transfer, Preservation::Preserve → falla si el proponente no tiene fondos) y se DEVUELVE al proponente cuando la propuesta se decide. Clave del lifecycle: el release ocurre en AMBAS transiciones fuera de Voting — finalize_proposal Y veto_proposal — o el bond de una propuesta vetada quedaría atrapado (no hay withdraw dispatchable). `og/props` separada del vault para que treasury_disbursement jamás toque un bond. Enfoque escrow-account: usa solo Currency::transfer (ya en el bound) — cero cambios de Config/runtime-wiring. submit_proposal + submit_emergency_proposal ambos bond. +3 regression tests (retiene+devuelve al finalizar, devuelve al vetar, rechaza si no alcanza). Mock refondeado >= MIN_PROPOSAL_STAKE. 48 gov tests, workspace 283 verde. Co-Authored-By: Claude Fable 5 --- crates/origin-chain-governance/src/lib.rs | 77 +++++++++++++++++++ crates/origin-chain-governance/src/mock.rs | 14 +++- .../src/tests_integration.rs | 65 +++++++++++++++- 3 files changed, 149 insertions(+), 7 deletions(-) diff --git a/crates/origin-chain-governance/src/lib.rs b/crates/origin-chain-governance/src/lib.rs index c82a5a6..bbddc88 100644 --- a/crates/origin-chain-governance/src/lib.rs +++ b/crates/origin-chain-governance/src/lib.rs @@ -336,6 +336,17 @@ pub mod pallet { pub type Disbursed = StorageMap<_, Blake2_128Concat, ProposalId, (), OptionQuery>; + /// Bond REAL retenido de cada propuesta viva: `(proponente, monto)`. Audit + /// 2026-07-04: antes el `stake` era caller-supplied SIN respaldo (una cuenta + /// vacía spameaba propuestas gratis, y el `baseline` del override era + /// manipulable). Ahora `submit_proposal` transfiere el stake a la cuenta-bond + /// `og/props` y lo registra aquí; se DEVUELVE al proponente cuando la + /// propuesta se decide (finalize o veto). Presencia = bond aún retenido. + #[pallet::storage] + #[pallet::getter(fn proposal_bond_of)] + pub type ProposalBond = + StorageMap<_, Blake2_128Concat, ProposalId, (T::AccountId, u128), 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 @@ -542,12 +553,21 @@ pub mod pallet { /// Audit 2026-07-04: la propuesta Approved ya fue desembolsada (single-use, /// anti-replay del treasury_disbursement). AlreadyDisbursed, + /// Audit 2026-07-04: el proponente NO tiene balance libre para el bond del + /// stake (el stake ahora se retiene de verdad — piel en el juego anti-spam). + StakeInsufficientBalance, } /// Minimum stake canonical proposal submission per CHN-4 5.3. /// 1,000 OGC = 1_000 × 10^18 (18 decimals OGC canonical). pub const MIN_PROPOSAL_STAKE: u128 = 1_000 * 10u128.pow(18); + /// PalletId de la cuenta-bond de propuestas (`og/props`) — escrow determinista + /// sin llave humana donde vive el stake retenido mientras la propuesta está + /// viva (audit 2026-07-04). Separada del vault de tesorería (`og/gvtry`) para + /// que `treasury_disbursement` JAMÁS pueda gastar un bond de propuesta. + pub const PROPOSAL_BOND_PALLET_ID: PalletId = PalletId(*b"og/props"); + /// Dispatchables canonical (real impl v0.0.30). #[pallet::call] impl Pallet { @@ -571,6 +591,20 @@ pub mod pallet { let id = crate::derive_proposal_id(&proposer_acct, &title_hash, &body_hash); ensure!(!Proposals::::contains_key(id), Error::::ProposalIdCollision); + // Audit 2026-07-04: RESERVA REAL del stake (piel en el juego anti-spam). + // Antes el `stake` era caller-supplied SIN respaldo — una cuenta vacía + // spameaba propuestas gratis y el `baseline` del override era + // manipulable. Ahora se retiene de verdad en la cuenta-bond og/props + // (falla si el proponente no tiene los fondos) y se devuelve al decidir. + T::Currency::transfer( + &who, + &Self::proposal_bond_account(), + stake, + Preservation::Preserve, + ) + .map_err(|_| Error::::StakeInsufficientBalance)?; + ProposalBond::::insert(id, (who.clone(), stake)); + let start_block: u64 = >::block_number() .saturated_into::(); let end_block = start_block.saturating_add(voting_period_blocks); @@ -699,6 +733,10 @@ pub mod pallet { Proposals::::insert(proposal_id, proposal); + // Audit 2026-07-04: la propuesta salió de Voting → devuelve el bond al + // proponente (el stake es anti-spam, no un castigo por perder el voto). + Self::release_proposal_bond(proposal_id); + Self::deposit_event(Event::ProposalFinalized { id: proposal_id, status: final_status, @@ -850,6 +888,10 @@ pub mod pallet { // Marca: rechazado por VETO del council (no por voto popular) → // única vía elegible para override_council_veto. VetoRejected::::insert(proposal_id, ()); + // Audit 2026-07-04: la propuesta salió de Voting (vetada) → devuelve + // el bond al proponente (si no, el stake de una propuesta vetada + // quedaría atrapado para siempre — no hay withdraw dispatchable). + Self::release_proposal_bond(proposal_id); Self::deposit_event(Event::ProposalVetoed { id: proposal_id, @@ -1025,6 +1067,17 @@ pub mod pallet { Error::::ProposalIdCollision ); + // Audit 2026-07-04: bond real, igual que submit_proposal (piel en el + // juego; se devuelve al proponente cuando la propuesta se decide). + T::Currency::transfer( + &who, + &Self::proposal_bond_account(), + stake, + Preservation::Preserve, + ) + .map_err(|_| Error::::StakeInsufficientBalance)?; + ProposalBond::::insert(id, (who.clone(), stake)); + let start_block: u64 = >::block_number() .saturated_into::(); let end_block = start_block.saturating_add(EMERGENCY_VOTING_PERIOD_BLOCKS); @@ -1146,6 +1199,30 @@ pub mod pallet { pub fn treasury_balance() -> u128 { T::Currency::balance(&Self::treasury_account()) } + + /// Cuenta-bond de propuestas (`og/props`) — escrow determinista sin llave + /// donde vive el stake retenido de las propuestas vivas (audit 2026-07-04). + pub fn proposal_bond_account() -> T::AccountId { + PROPOSAL_BOND_PALLET_ID.into_account_truncating() + } + + /// Devuelve el bond retenido al proponente cuando la propuesta se decide + /// (finalize o veto). Idempotente: `take` consume la entrada, así que un + /// segundo intento es no-op (no doble-refund). Expendable: la cuenta-bond + /// puede quedar en 0. Un fallo del transfer deja el bond en el escrow (no + /// se pierde) — best-effort para no bloquear la transición de estado. + fn release_proposal_bond(proposal_id: ProposalId) { + if let Some((staker, amount)) = ProposalBond::::take(proposal_id) { + if amount > 0 { + let _ = T::Currency::transfer( + &Self::proposal_bond_account(), + &staker, + amount, + Preservation::Expendable, + ); + } + } + } } /// Helper canonical para convert T::AccountId → 32-byte canonical. diff --git a/crates/origin-chain-governance/src/mock.rs b/crates/origin-chain-governance/src/mock.rs index a5e3f3f..1ffcd77 100644 --- a/crates/origin-chain-governance/src/mock.rs +++ b/crates/origin-chain-governance/src/mock.rs @@ -45,17 +45,23 @@ impl origin_chain_governance::Config for Test { type TreasuryPalletId = TestTreasuryPalletId; } +/// Balance inicial de cada cuenta de test. Audit 2026-07-04: debe ser +/// >= MIN_PROPOSAL_STAKE (10^21) para que el BOND REAL de submit_proposal no +/// falle por fondos; con margen para bonds + treasury deposits simultáneos. +pub const INITIAL_BALANCE: u128 = crate::pallet::MIN_PROPOSAL_STAKE * 10; + /// Test externalities builder canonical (block_number=1). pub fn new_test_ext() -> sp_io::TestExternalities { let mut storage = frame_system::GenesisConfig::::default() .build_storage() .expect("genesis canonical builds"); - // FIX-6: fondea cuentas para los tests de tesorería real (transfers OGC). + // FIX-6 + bond audit 2026-07-04: fondea cuentas para tesorería real Y para el + // bond del stake de propuesta (>= MIN_PROPOSAL_STAKE). pallet_balances::GenesisConfig:: { balances: vec![ - (alice_account(), 100_000_000), - (bob_account(), 100_000_000), - (charlie_account(), 100_000_000), + (alice_account(), INITIAL_BALANCE), + (bob_account(), INITIAL_BALANCE), + (charlie_account(), INITIAL_BALANCE), ], ..Default::default() } diff --git a/crates/origin-chain-governance/src/tests_integration.rs b/crates/origin-chain-governance/src/tests_integration.rs index a27ef38..e3bda27 100644 --- a/crates/origin-chain-governance/src/tests_integration.rs +++ b/crates/origin-chain-governance/src/tests_integration.rs @@ -17,7 +17,7 @@ use crate::mock::{ alice_account, alice_origin, bob_account, bob_origin, charlie_account, charlie_origin, - new_test_ext, run_to_block, Balances, Governance, RuntimeEvent, System, Test, + new_test_ext, run_to_block, Balances, Governance, RuntimeEvent, System, Test, INITIAL_BALANCE, }; use crate::pallet::{Proposal, ProposalStatus, MIN_PROPOSAL_STAKE}; use crate::{derive_proposal_id, Error, Event, Proposals, Votes}; @@ -842,10 +842,11 @@ fn treasury_scenario_4_successful_disbursement_canonical() { assert_eq!(Governance::treasury_balance(), 700_000); // FIX-6: el recipient recibió OGC REAL (lo que distingue un transfer real - // de un contador): bob pasó de 100M a 100M + 300k en pallet_balances. + // de un contador): bob pasó de INITIAL_BALANCE a +300k en pallet_balances + // (bob no somete propuesta, así que no paga bond — solo recibe). assert_eq!( >::balance(&bob_account()), - 100_000_000 + 300_000 + INITIAL_BALANCE + 300_000 ); System::assert_last_event(RuntimeEvent::Governance(Event::TreasuryDisbursed { @@ -956,6 +957,64 @@ fn veto_no_cuenta_votos_de_miembros_removidos() { }); } +// ─── Regresión audit 2026-07-04: bond REAL del stake de propuesta ─── + +fn bal(who: &sp_runtime::AccountId32) -> u128 { + >::balance(who) +} + +#[test] +fn bond_se_retiene_al_someter_y_se_devuelve_al_finalizar() { + new_test_ext().execute_with(|| { + let bond_acct = Governance::proposal_bond_account(); + let before = bal(&alice_account()); + let id = submit_default(alice_origin()); + // Bond retenido: alice bajó MIN_PROPOSAL_STAKE; la cuenta-bond lo tiene. + assert_eq!(bal(&alice_account()), before - MIN_PROPOSAL_STAKE, "bond retenido"); + assert_eq!(bal(&bond_acct), MIN_PROPOSAL_STAKE, "og/props tiene el bond"); + // Finaliza (sin votos → Rejected) → bond devuelto íntegro. + run_to_block(200); + assert_ok!(Governance::finalize_proposal(alice_origin(), id)); + assert_eq!(bal(&alice_account()), before, "bond devuelto al finalizar"); + assert_eq!(bal(&bond_acct), 0, "cuenta-bond vacía"); + }); +} + +#[test] +fn bond_se_devuelve_al_vetar() { + new_test_ext().execute_with(|| { + use crate::pallet::CouncilRole; + use frame_support::dispatch::RawOrigin; + assert_ok!(Governance::add_council_member( + RawOrigin::Root.into(), + alice_account(), + CouncilRole::Chair, + )); + let before = bal(&alice_account()); + let id = submit_default(alice_origin()); + assert_eq!(bal(&alice_account()), before - MIN_PROPOSAL_STAKE); + // Veto (council de 1 → auto-rechazo) → bond devuelto (si no, quedaría + // atrapado: una propuesta vetada nunca pasa por finalize). + assert_ok!(Governance::veto_proposal(alice_origin(), id)); + assert_eq!(bal(&alice_account()), before, "bond devuelto al vetar"); + }); +} + +#[test] +fn submit_rechazado_si_no_alcanza_para_el_bond() { + new_test_ext().execute_with(|| { + // Cuenta sin fondos (no está en el génesis del mock) → no puede pagar el bond. + let pobre = frame_support::dispatch::RawOrigin::Signed( + sp_runtime::AccountId32::new([9u8; 32]), + ) + .into(); + assert_noop!( + Governance::submit_proposal(pobre, [7u8; 32], [8u8; 32], 100, MIN_PROPOSAL_STAKE), + Error::::StakeInsufficientBalance + ); + }); +} + // ─── Scenario Council 7: 2/3 threshold canonical compile-detect ───── #[test]