fix(governance): 3 bugs de correctitud del audit ultra (CRITICAL override + HIGH ×2)

Audit ultra 2026-07-04 (dimensión governance-hardening, nunca antes corrida):

1. [CRITICAL] override_council_veto revertía CUALQUIER propuesta Rejected a
   Approved — incluidas las que la COMUNIDAD votó en contra (finalize_proposal
   marca Rejected cuando nay>=yes). Un proponente podía resucitar una propuesta
   rechazada en las urnas y drenar el treasury, invirtiendo el voto popular.
   Fix: marca VetoRejected (solo la pone veto_proposal al alcanzar el umbral);
   el override exige esa marca ⇒ solo revierte VETOS del council, no votos.

2. [HIGH] treasury_disbursement podía re-desembolsar la MISMA propuesta Approved
   N veces (drenaje por replay) — el guard status==Approved no cambiaba tras el
   pago. Fix: marca Disbursed (single-use) + Error::AlreadyDisbursed.

3. [HIGH] veto_proposal usaba un contador MONÓTONO: el veto de un miembro
   removido seguía contando y encoger el council bajaba el umbral retroactivamente
   ⇒ una minoría podía vetar (derrota el safeguard 2/3). Fix: conteo EN VIVO
   (itera VetoVotes filtrando por membresía actual del council, O(≤7)).

+3 regression tests. Governance 45 tests verde. Storage aditivo (VetoRejected,
Disbursed) — sin migración (pre-mainnet, StorageVersion 0). Pendientes para
David (diseño): reserva real del stake de propuesta (HoldReason), emergency 5/7.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
davidherranandClaude Fable 5 committed 2026-07-04 14:50:28 -05:00
1 parent d6dd08da66
commit 80bf7e5ddf
2 files changed
+163 -7

No files matched your search

+64 -7
View File
@@ -316,6 +316,26 @@ pub mod pallet {
pub type AdvisorFlags<T: Config> =
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<T: Config> =
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<T: Config> =
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::<T>::AlreadyVetoed
);
// Step 6: insert vote + increment count
// Step 6: insert vote
VetoVotes::<T>::insert(proposal_id, &who, ());
let new_count = VetoCount::<T>::get(proposal_id).saturating_add(1);
VetoCount::<T>::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::<T>::iter_prefix(proposal_id)
.filter(|(acct, _)| Council::<T>::contains_key(acct))
.count() as u32;
VetoCount::<T>::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::<T>::insert(proposal_id, proposal);
// Marca: rechazado por VETO del council (no por voto popular) →
// única vía elegible para override_council_veto.
VetoRejected::<T>::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::<T>::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::<T>::contains_key(proposal_id),
Error::<T>::ProposalNotVetoRejected
);
ensure!(
!AdvisorFlags::<T>::contains_key(&who),
Error::<T>::AdvisorMayNotVote
@@ -1038,6 +1084,17 @@ pub mod pallet {
Error::<T>::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::<T>::contains_key(proposal_id),
Error::<T>::AlreadyDisbursed
);
// Marcar ANTES del transfer (defensa en profundidad; los pallets no
// reentran, pero el marker refleja la intención de gasto atómica).
Disbursed::<T>::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
@@ -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::<Test>::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::<Test>::ProposalNotVetoRejected
);
// Sigue Rejected — el voto popular se respeta.
assert!(matches!(
Proposals::<Test>::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::<Test>::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::<Test>::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::<Test>::get(id), 1, "solo el veto vivo de Bob cuenta");
assert!(
matches!(Proposals::<Test>::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]