feat(governance): reserva REAL del stake de propuesta — bond og/props (audit A, David)

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 <noreply@anthropic.com>
This commit is contained in:
davidherranandClaude Fable 5 committed 2026-07-05 19:55:39 -05:00
1 parent f3de48b1b8
commit 304072173e
3 files changed
+149 -7

No files matched your search

+77
View File
@@ -336,6 +336,17 @@ pub mod pallet {
pub type Disbursed<T: Config> =
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<T: Config> =
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<T: Config> Pallet<T> {
@@ -571,6 +591,20 @@ pub mod pallet {
let id = crate::derive_proposal_id(&proposer_acct, &title_hash, &body_hash);
ensure!(!Proposals::<T>::contains_key(id), Error::<T>::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::<T>::StakeInsufficientBalance)?;
ProposalBond::<T>::insert(id, (who.clone(), stake));
let start_block: u64 = <frame_system::Pallet<T>>::block_number()
.saturated_into::<u64>();
let end_block = start_block.saturating_add(voting_period_blocks);
@@ -699,6 +733,10 @@ pub mod pallet {
Proposals::<T>::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::<T>::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::<T>::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::<T>::StakeInsufficientBalance)?;
ProposalBond::<T>::insert(id, (who.clone(), stake));
let start_block: u64 = <frame_system::Pallet<T>>::block_number()
.saturated_into::<u64>();
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::<T>::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.
+10 -4
View File
@@ -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::<Test>::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::<Test> {
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()
}
@@ -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!(
<Balances as frame_support::traits::fungible::Inspect<_>>::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 {
<Balances as frame_support::traits::fungible::Inspect<_>>::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::<Test>::StakeInsufficientBalance
);
});
}
// ─── Scenario Council 7: 2/3 threshold canonical compile-detect ─────
#[test]