From 735a7e14bd2e182bbcf467ac30103721b204fb45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Wed, 28 Aug 2024 14:45:34 +0100 Subject: [PATCH 1/4] mv crates/node/src/shell/governance.rs crates/governance/src/finalize_block.rs --- .../src/shell/governance.rs => governance/src/finalize_block.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename crates/{node/src/shell/governance.rs => governance/src/finalize_block.rs} (100%) diff --git a/crates/node/src/shell/governance.rs b/crates/governance/src/finalize_block.rs similarity index 100% rename from crates/node/src/shell/governance.rs rename to crates/governance/src/finalize_block.rs From 0281369b9b1544e03971ab71ea9a23efcee357e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Aug 2024 11:25:24 +0100 Subject: [PATCH 2/4] governance: fix finalize_block with DI --- Cargo.lock | 1 + crates/governance/src/finalize_block.rs | 527 +++++++++++------------- crates/governance/src/lib.rs | 2 + crates/node/src/protocol.rs | 8 +- crates/node/src/shell/finalize_block.rs | 92 ++++- crates/node/src/shell/mod.rs | 5 +- crates/node/src/shell/utils.rs | 14 - crates/proof_of_stake/src/lib.rs | 57 +++ crates/proof_of_stake/src/storage.rs | 4 +- crates/sdk/src/queries/vp/pos.rs | 2 +- crates/systems/Cargo.toml | 1 + crates/systems/src/proof_of_stake.rs | 44 ++ crates/systems/src/trans_token.rs | 13 + crates/trans_token/src/lib.rs | 54 ++- wasm/Cargo.lock | 1 + wasm_for_tests/Cargo.lock | 1 + 16 files changed, 506 insertions(+), 320 deletions(-) delete mode 100644 crates/node/src/shell/utils.rs diff --git a/Cargo.lock b/Cargo.lock index b024136a0c..681fcfd485 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5339,6 +5339,7 @@ dependencies = [ "cargo_metadata", "lazy_static", "namada_core", + "namada_events", "namada_storage", ] diff --git a/crates/governance/src/finalize_block.rs b/crates/governance/src/finalize_block.rs index 04842694c5..6f1666a7f3 100644 --- a/crates/governance/src/finalize_block.rs +++ b/crates/governance/src/finalize_block.rs @@ -1,122 +1,132 @@ -use namada_sdk::chain::Epoch; -use namada_sdk::collections::HashMap; -use namada_sdk::events::extend::{ComposeEvent, Height, UserAccount}; -use namada_sdk::events::{EmitEvents, EventLevel}; -use namada_sdk::governance::event::GovernanceEvent; -use namada_sdk::governance::pgf::storage::keys as pgf_storage; -use namada_sdk::governance::pgf::storage::steward::StewardDetail; -use namada_sdk::governance::pgf::{storage as pgf, ADDRESS}; -use namada_sdk::governance::storage::proposal::{ +//! Governance logic applied on an end of a block. + +use std::collections::BTreeSet; +use std::str::FromStr; + +use borsh::BorshDeserialize; +use namada_core::address::Address; +use namada_core::chain::{ChainId, Epoch}; +use namada_core::collections::HashMap; +use namada_core::encode; +use namada_core::ibc::PGFIbcTarget; +use namada_events::extend::{ComposeEvent, Height}; +use namada_events::{EmitEvents, EventLevel}; +use namada_state::{Key, Result, State, StateRead, StorageRead, StorageWrite}; +use namada_systems::{proof_of_stake, trans_token as token}; +use namada_tx::data::TxType; +use namada_tx::{Code, Data, Tx}; + +use crate::event::GovernanceEvent; +use crate::pgf::storage::keys as pgf_keys; +use crate::pgf::storage::steward::StewardDetail; +use crate::pgf::{storage as pgf_storage, ADDRESS as PGF_ADDRESS}; +use crate::storage::proposal::{ AddRemove, PGFAction, PGFTarget, ProposalType, StoragePgfFunding, }; -use namada_sdk::governance::storage::{keys as gov_storage, load_proposals}; -use namada_sdk::governance::utils::{ +use crate::storage::{keys, load_proposals}; +use crate::utils::{ compute_proposal_result, ProposalVotes, TallyResult, TallyType, VotePower, }; -pub use namada_sdk::governance::Store; -use namada_sdk::governance::{ - storage as gov_api, ProposalVote, ADDRESS as gov_address, -}; -use namada_sdk::proof_of_stake::bond_amount; -use namada_sdk::proof_of_stake::parameters::PosParams; -use namada_sdk::proof_of_stake::storage::{ - read_total_active_stake, read_validator_stake, validator_state_handle, -}; -use namada_sdk::proof_of_stake::types::{BondId, ValidatorState}; -use namada_sdk::state::StorageWrite; -use namada_sdk::token::event::{TokenEvent, TokenOperation}; -use namada_sdk::token::read_balance; -use namada_sdk::tx::{Code, Data}; -use namada_sdk::{encode, ibc, parameters}; - -use super::utils::force_read; -use super::*; - -pub fn finalize_block( - shell: &mut Shell, +use crate::{storage, ProposalVote, ADDRESS as GOV_ADDRESS}; + +/// Apply governance updates for a block. On a new epoch, this will look for +/// proposals to tally completed proposals and execute accepted proposals. +#[allow(clippy::too_many_arguments)] +pub fn finalize_block( + state: &mut S, events: &mut impl EmitEvents, current_epoch: Epoch, is_new_epoch: bool, -) -> ShellResult<()> + dispatch_tx: FnTx, + transfer_over_ibc: FnIbcTransfer, +) -> Result<()> where - D: 'static + DB + for<'iter> DBIter<'iter> + Sync, - H: 'static + StorageHasher + Sync, + S: StateRead + State, + Token: token::Read + token::Write + token::Events, + PoS: proof_of_stake::Read, + FnTx: FnMut(&Tx, &mut S) -> Result, + FnIbcTransfer: Fn(&mut S, &Address, &Address, &PGFIbcTarget) -> Result<()>, { if is_new_epoch { - load_and_execute_governance_proposals(shell, events, current_epoch)?; + load_and_execute_governance_proposals::< + S, + Token, + PoS, + FnTx, + FnIbcTransfer, + >(state, events, current_epoch, dispatch_tx, transfer_over_ibc)?; } Ok(()) } -#[derive(Default)] -pub struct ProposalsResult { - passed: Vec, - rejected: Vec, -} - -pub fn load_and_execute_governance_proposals( - shell: &mut Shell, +fn load_and_execute_governance_proposals( + state: &mut S, events: &mut impl EmitEvents, current_epoch: Epoch, -) -> ShellResult + dispatch_tx: FnTx, + transfer_over_ibc: FnIbcTransfer, +) -> Result<()> where - D: DB + for<'iter> DBIter<'iter> + Sync + 'static, - H: StorageHasher + Sync + 'static, + S: StateRead + State, + Token: token::Read + token::Write + token::Events, + PoS: proof_of_stake::Read, + FnTx: FnMut(&Tx, &mut S) -> Result, + FnIbcTransfer: Fn(&mut S, &Address, &Address, &PGFIbcTarget) -> Result<()>, { - let proposal_ids = load_proposals(&shell.state, current_epoch)?; - - let proposals_result = - execute_governance_proposals(shell, events, proposal_ids)?; - - Ok(proposals_result) + let proposal_ids = load_proposals(state, current_epoch)?; + + execute_governance_proposals::( + state, + events, + proposal_ids, + dispatch_tx, + transfer_over_ibc, + ) } -fn execute_governance_proposals( - shell: &mut Shell, +fn execute_governance_proposals( + state: &mut S, events: &mut impl EmitEvents, proposal_ids: BTreeSet, -) -> ShellResult + mut dispatch_tx: FnTx, + mut transfer_over_ibc: FnIbcTransfer, +) -> Result<()> where - D: DB + for<'iter> DBIter<'iter> + Sync + 'static, - H: StorageHasher + Sync + 'static, + S: StateRead + State, + Token: token::Read + token::Write + token::Events, + PoS: proof_of_stake::Read, + FnTx: FnMut(&Tx, &mut S) -> Result, + FnIbcTransfer: Fn(&mut S, &Address, &Address, &PGFIbcTarget) -> Result<()>, { - let mut proposals_result = ProposalsResult::default(); - let params = read_pos_params::<_, Store<_>>(&shell.state)?; - for id in proposal_ids { - let proposal_funds_key = gov_storage::get_funds_key(id); - let proposal_end_epoch_key = gov_storage::get_voting_end_epoch_key(id); - let proposal_type_key = gov_storage::get_proposal_type_key(id); - let proposal_author_key = gov_storage::get_author_key(id); + let proposal_funds_key = keys::get_funds_key(id); + let proposal_end_epoch_key = keys::get_voting_end_epoch_key(id); + let proposal_type_key = keys::get_proposal_type_key(id); + let proposal_author_key = keys::get_author_key(id); - let funds: token::Amount = - force_read(&shell.state, &proposal_funds_key)?; + let funds: token::Amount = force_read(state, &proposal_funds_key)?; let proposal_end_epoch: Epoch = - force_read(&shell.state, &proposal_end_epoch_key)?; + force_read(state, &proposal_end_epoch_key)?; let proposal_type: ProposalType = - force_read(&shell.state, &proposal_type_key)?; - let proposal_author: Address = - force_read(&shell.state, &proposal_author_key)?; + force_read(state, &proposal_type_key)?; + let proposal_author: Address = force_read(state, &proposal_author_key)?; - let is_steward = pgf::is_steward(&shell.state, &proposal_author)?; + let is_steward = pgf_storage::is_steward(state, &proposal_author)?; - let total_active_voting_power = - read_total_active_stake(&shell.state, ¶ms, proposal_end_epoch)?; + let total_active_voting_power = PoS::total_active_stake::< + crate::Store<_>, + >(state, proposal_end_epoch)?; let tally_type = TallyType::from(proposal_type.clone(), is_steward); - let votes = compute_proposal_votes( - &shell.state, - ¶ms, - id, - proposal_end_epoch, - )?; + let votes = + compute_proposal_votes::(state, id, proposal_end_epoch)?; let proposal_result = compute_proposal_result( votes, total_active_voting_power, tally_type, ) .expect("Proposal result calculation must not over/underflow"); - gov_api::write_proposal_result(&mut shell.state, id, proposal_result)?; + storage::write_proposal_result(state, id, proposal_result)?; let transfer_address = match proposal_result.result { TallyResult::Passed => { @@ -131,12 +141,13 @@ where } ProposalType::DefaultWithWasm(_) => { let proposal_code = - gov_api::get_proposal_code(&shell.state, id)? + storage::get_proposal_code(state, id)? .unwrap_or_default(); let result = execute_default_proposal( - shell, + state, id, proposal_code.clone(), + &mut dispatch_tx, )?; tracing::info!( "Governance proposal #{} (default with wasm) has \ @@ -148,10 +159,8 @@ where GovernanceEvent::passed_proposal(id, true, result) } ProposalType::PGFSteward(stewards) => { - let result = execute_pgf_steward_proposal( - &mut shell.state, - stewards, - )?; + let result = + execute_pgf_steward_proposal(state, stewards)?; tracing::info!( "Governance proposal #{} for PGF stewards has \ been executed. {}.", @@ -167,13 +176,13 @@ where GovernanceEvent::passed_proposal(id, false, false) } ProposalType::PGFPayment(payments) => { - let native_token = &shell.state.get_native_token()?; - execute_pgf_funding_proposal( - &mut shell.state, - events, - native_token, + let native_token = state.get_native_token()?; + execute_pgf_funding_proposal::( + state, + &native_token, payments, id, + &mut transfer_over_ibc, )?; tracing::info!( "Governance proposal #{} for PGF funding has \ @@ -185,31 +194,26 @@ where } }; events.emit(proposal_event); - proposals_result.passed.push(id); // Take events that could have been emitted by PGF // over IBC, governance proposal execution, etc let current_height = - shell.state.in_mem().get_last_block_height().next_height(); + state.in_mem().get_last_block_height().next_height(); events.emit_many( - shell - .state + state .write_log_mut() .take_events() .into_iter() .map(|event| event.with(Height(current_height))), ); - gov_api::get_proposal_author(&shell.state, id)? + storage::get_proposal_author(state, id)? } TallyResult::Rejected => { if let ProposalType::PGFPayment(_) = proposal_type { if proposal_result.two_thirds_nay_over_two_thirds_total() { - pgf::remove_steward( - &mut shell.state, - &proposal_author, - )?; + pgf_storage::remove_steward(state, &proposal_author)?; tracing::info!( "Governance proposal {} was rejected with 2/3 of \ @@ -226,7 +230,6 @@ where matches!(proposal_type, ProposalType::DefaultWithWasm(_)), ); events.emit(proposal_event); - proposals_result.rejected.push(id); tracing::info!( "Governance proposal {} has been executed and rejected.", @@ -237,74 +240,54 @@ where } }; - let native_token = shell.state.get_native_token()?; + let native_token = state.get_native_token()?; if let Some(address) = transfer_address { - token::transfer( - &mut shell.state, + Token::transfer( + state, &native_token, - &gov_address, + &GOV_ADDRESS, &address, funds, )?; const DESCRIPTOR: &str = "governance-locked-funds-refund"; - let final_gov_balance = - read_balance(&shell.state, &native_token, &gov_address)?.into(); - let final_target_balance = - read_balance(&shell.state, &native_token, &address)?.into(); - - events.emit(TokenEvent { - descriptor: DESCRIPTOR.into(), - level: EventLevel::Block, - operation: TokenOperation::transfer( - UserAccount::Internal(gov_address), - UserAccount::Internal(address), - native_token.clone(), - funds.into(), - final_gov_balance, - Some(final_target_balance), - ), - }); - } else { - token::burn_tokens( - &mut shell.state, + Token::emit_transfer_event( + state, + DESCRIPTOR.into(), + EventLevel::Tx, &native_token, - &gov_address, funds, + token::UserAccount::Internal(GOV_ADDRESS), + token::UserAccount::Internal(address), )?; + } else { + Token::burn_tokens(state, &native_token, &GOV_ADDRESS, funds)?; const DESCRIPTOR: &str = "governance-locked-funds-burn"; - let final_gov_balance = - read_balance(&shell.state, &native_token, &gov_address)?.into(); - - events.emit(TokenEvent { - descriptor: DESCRIPTOR.into(), - level: EventLevel::Block, - operation: TokenOperation::Burn { - token: native_token.clone(), - amount: funds.into(), - target_account: UserAccount::Internal(gov_address), - post_balance: final_gov_balance, - }, - }); + Token::emit_burn_event( + state, + DESCRIPTOR.into(), + &native_token, + funds, + &GOV_ADDRESS, + )?; } } - - Ok(proposals_result) + Ok(()) } -fn compute_proposal_votes( +fn compute_proposal_votes( storage: &S, - params: &PosParams, proposal_id: u64, epoch: Epoch, -) -> namada_sdk::state::Result +) -> Result where S: StorageRead, + PoS: proof_of_stake::Read, { - let votes = gov_api::get_proposal_votes(storage, proposal_id)?; + let votes = storage::get_proposal_votes(storage, proposal_id)?; let mut validators_vote: HashMap = HashMap::default(); @@ -317,25 +300,24 @@ where HashMap, > = HashMap::default(); + let mut validator_cache: HashMap = HashMap::default(); + for vote in votes { + let validator = &vote.validator; + // Skip votes involving jailed or inactive validators - let validator = vote.validator.clone(); - let validator_state = - validator_state_handle(&validator).get(storage, epoch, params)?; - - if matches!( - validator_state, - Some(ValidatorState::Jailed) | Some(ValidatorState::Inactive) - ) { - continue; - } - if validator_state.is_none() { - tracing::error!( - "While computing votes for proposal id {proposal_id} in epoch \ - {epoch}, encountered validator {validator} that has no \ - stored state. Please report this as a bug. Skipping this \ - vote." - ); + let is_active_validator = if let Some(is_active_validator) = + validator_cache.get(validator) + { + *is_active_validator + } else { + let is_active_validator = PoS::is_active_validator::< + crate::Store<_>, + >(storage, validator, epoch)?; + validator_cache.insert(validator.clone(), is_active_validator); + is_active_validator + }; + if !is_active_validator { continue; } @@ -343,29 +325,27 @@ where if vote.is_validator() { let vote_data = vote.data.clone(); - let validator_stake = - read_validator_stake(storage, params, &validator, epoch) - .unwrap_or_default(); + let validator_stake = PoS::read_validator_stake::>( + storage, validator, epoch, + ) + .unwrap_or_default(); validators_vote.insert(validator.clone(), vote_data); - validator_voting_power.insert(validator, validator_stake); + validator_voting_power.insert(validator.clone(), validator_stake); } else { let delegator = vote.delegator.clone(); let vote_data = vote.data.clone(); - let bond_id = BondId { - source: delegator.clone(), - validator: validator.clone(), - }; - let delegator_stake = - bond_amount::<_, Store<_>>(storage, &bond_id, epoch); + let delegator_stake = PoS::bond_amount::>( + storage, validator, &delegator, epoch, + ); if let Ok(stake) = delegator_stake { delegators_vote.insert(delegator.clone(), vote_data); delegator_voting_power .entry(delegator) .or_default() - .insert(validator, stake); + .insert(validator.clone(), stake); } else { continue; } @@ -380,85 +360,40 @@ where }) } -fn execute_default_proposal( - shell: &mut Shell, +fn execute_default_proposal( + state: &mut S, id: u64, proposal_code: Vec, -) -> namada_sdk::state::Result + dispatch_tx: &mut FnTx, +) -> Result where - D: DB + for<'iter> DBIter<'iter> + Sync + 'static, - H: StorageHasher + Sync + 'static, + S: StateRead + State, + FnTx: FnMut(&Tx, &mut S) -> Result, { - let pending_execution_key = gov_storage::get_proposal_execution_key(id); - shell.state.write(&pending_execution_key, ())?; + let pending_execution_key = keys::get_proposal_execution_key(id); + state.write(&pending_execution_key, ())?; let mut tx = Tx::from_type(TxType::Raw); - tx.header.chain_id = shell.chain_id.clone(); + tx.header.chain_id = ChainId::from_str(&state.get_chain_id()?).unwrap(); tx.set_data(Data::new(encode(&id))); tx.set_code(Code::new(proposal_code, None)); - // Ok to unwrap cause we constructed the tx in protocol - let cmt = tx.first_commitments().unwrap().to_owned(); - - let dispatch_result = protocol::dispatch_tx( - &tx, - protocol::DispatchArgs::Raw { - wrapper_hash: None, - tx_index: TxIndex::default(), - wrapper_tx_result: None, - vp_wasm_cache: &mut shell.vp_wasm_cache, - tx_wasm_cache: &mut shell.tx_wasm_cache, - }, - // No gas limit for governance proposal - &RefCell::new(TxGasMeter::new(u64::MAX)), - &mut shell.state, - ); - shell - .state + + let dispatch_result = dispatch_tx(&tx, state); + state .delete(&pending_execution_key) .expect("Should be able to delete the storage."); - match dispatch_result { - Ok(extended_tx_result) => match extended_tx_result - .tx_result - .get_inner_tx_result(None, either::Right(&cmt)) - { - Some(Ok(batched_result)) if batched_result.is_accepted() => { - shell.state.commit_tx_batch(); - Ok(true) - } - Some(Err(e)) => { - tracing::warn!( - "Error executing governance proposal {}", - e.to_string() - ); - shell.state.drop_tx_batch(); - Ok(false) - } - _ => { - tracing::warn!("not sure what happen"); - shell.state.drop_tx_batch(); - Ok(false) - } - }, - Err(e) => { - tracing::warn!( - "Error executing governance proposal {}", - e.error.to_string() - ); - shell.state.drop_tx_batch(); - Ok(false) - } - } + dispatch_result } fn execute_pgf_steward_proposal( storage: &mut S, stewards: BTreeSet>, -) -> ShellResult +) -> Result where S: StorageRead + StorageWrite, { let maximum_number_of_pgf_steward_key = - pgf_storage::get_maximum_number_of_pgf_steward_key(); + pgf_keys::get_maximum_number_of_pgf_steward_key(); let maximum_number_of_pgf_steward = storage .read::(&maximum_number_of_pgf_steward_key)? .expect( @@ -470,11 +405,11 @@ where AddRemove::Add(_) => None, AddRemove::Remove(address) => Some(address), }) { - pgf_storage::stewards_handle().remove(storage, address)?; + pgf_keys::stewards_handle().remove(storage, address)?; } // Then add new addresses - let mut steward_count = pgf_storage::stewards_handle().len(storage)?; + let mut steward_count = pgf_keys::stewards_handle().len(storage)?; for address in stewards.iter().filter_map(|action| match action { AddRemove::Add(address) => Some(address), AddRemove::Remove(_) => None, @@ -483,7 +418,7 @@ where if steward_count + 1 > maximum_number_of_pgf_steward { return Ok(false); } - pgf_storage::stewards_handle().insert( + pgf_keys::stewards_handle().insert( storage, address.to_owned(), StewardDetail::base(address.to_owned()), @@ -498,23 +433,24 @@ where Ok(true) } -fn execute_pgf_funding_proposal( - state: &mut WlState, - events: &mut impl EmitEvents, +fn execute_pgf_funding_proposal( + storage: &mut S, token: &Address, fundings: BTreeSet, proposal_id: u64, -) -> ShellResult + transfer_over_ibc: &mut FnIbcTransfer, +) -> Result where - D: DB + for<'iter> DBIter<'iter> + Sync + 'static, - H: StorageHasher + Sync + 'static, + S: StorageRead + StorageWrite, + Token: token::Write + token::Events, + FnIbcTransfer: Fn(&mut S, &Address, &Address, &PGFIbcTarget) -> Result<()>, { for funding in fundings { match funding { PGFAction::Continuous(action) => match action { AddRemove::Add(target) => { - pgf_storage::fundings_handle().insert( - state, + pgf_keys::fundings_handle().insert( + storage, target.target().clone(), StoragePgfFunding::new(target.clone(), proposal_id), )?; @@ -527,8 +463,8 @@ where ); } AddRemove::Remove(target) => { - pgf_storage::fundings_handle() - .remove(state, &target.target())?; + pgf_keys::fundings_handle() + .remove(storage, &target.target())?; tracing::info!( "Removed Continuous PGF from proposal id {}: set {} \ to {}.", @@ -539,53 +475,52 @@ where } }, PGFAction::Retro(target) => { - let (result, event) = match &target { - PGFTarget::Internal(target) => ( - token::transfer( - state, + let result = match &target { + PGFTarget::Internal(target) => { + let result = Token::transfer( + storage, token, - &ADDRESS, + &PGF_ADDRESS, &target.target, target.amount, - ), - TokenEvent { - descriptor: "pgf-payments".into(), - level: EventLevel::Block, - operation: TokenOperation::transfer( - UserAccount::Internal(ADDRESS), - UserAccount::Internal(target.target.clone()), - token.clone(), - target.amount.into(), - read_balance(state, token, &ADDRESS)?.into(), - Some( - read_balance(state, token, &target.target)? - .into(), + ); + if result.is_ok() { + Token::emit_transfer_event( + storage, + "pgf-payments".into(), + EventLevel::Block, + token, + target.amount, + token::UserAccount::Internal(PGF_ADDRESS), + token::UserAccount::Internal( + target.target.clone(), ), - ), - }, - ), - PGFTarget::Ibc(target) => ( - ibc::transfer_over_ibc::< - _, - parameters::Store<_>, - token::Store<_>, - token::Transfer, - >( - state, token, &ADDRESS, target - ), - TokenEvent { - descriptor: "pgf-payments-over-ibc".into(), - level: EventLevel::Block, - operation: TokenOperation::transfer( - UserAccount::Internal(ADDRESS), - UserAccount::External(target.target.clone()), - token.clone(), - target.amount.into(), - read_balance(state, token, &ADDRESS)?.into(), - None, - ), - }, - ), + )?; + } + result + } + PGFTarget::Ibc(target) => { + let result = transfer_over_ibc( + storage, + token, + &PGF_ADDRESS, + target, + ); + if result.is_ok() { + Token::emit_transfer_event( + storage, + "pgf-payments-over-ibc".into(), + EventLevel::Block, + token, + target.amount, + token::UserAccount::Internal(PGF_ADDRESS), + token::UserAccount::External( + target.target.clone(), + ), + )?; + } + result + } }; match result { Ok(()) => { @@ -596,7 +531,6 @@ where target.amount().to_string_native(), target.target() ); - events.emit(event); } Err(e) => tracing::warn!( "Error in Retroactive PGF transfer from proposal id \ @@ -613,3 +547,14 @@ where Ok(true) } + +fn force_read(storage: &S, key: &Key) -> Result +where + S: StorageRead, + T: BorshDeserialize, +{ + storage + .read::(key) + .transpose() + .expect("Storage key must be present.") +} diff --git a/crates/governance/src/lib.rs b/crates/governance/src/lib.rs index a97a02c536..ec2ea7e5a0 100644 --- a/crates/governance/src/lib.rs +++ b/crates/governance/src/lib.rs @@ -24,6 +24,7 @@ use namada_core::address::{self, Address}; /// governance CLI structures pub mod cli; pub mod event; +mod finalize_block; /// governance parameters pub mod parameters; /// governance public good fundings @@ -34,6 +35,7 @@ pub mod storage; pub mod utils; pub mod vp; +pub use finalize_block::finalize_block; use namada_state::{StorageRead, StorageWrite}; pub use namada_systems::governance::*; use parameters::GovernanceParameters; diff --git a/crates/node/src/protocol.rs b/crates/node/src/protocol.rs index be8c2e3d17..62918073cb 100644 --- a/crates/node/src/protocol.rs +++ b/crates/node/src/protocol.rs @@ -1,7 +1,7 @@ //! The ledger's protocol use std::cell::RefCell; use std::collections::BTreeSet; -use std::fmt::Debug; +use std::fmt::{Debug, Display}; use either::Either; use eyre::{eyre, WrapErr}; @@ -144,6 +144,12 @@ pub struct DispatchError { pub tx_result: Option>, } +impl Display for DispatchError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.error) + } +} + impl From for DispatchError { fn from(error: Error) -> Self { Self { diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 57c52ffac6..e30a018b08 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -98,12 +98,7 @@ where // Sub-system updates: // - Governance - applied first in case a proposal changes any of the // other syb-systems - governance::finalize_block( - self, - emit_events, - current_epoch, - new_epoch, - )?; + gov_finalize_block(self, emit_events, current_epoch, new_epoch)?; // - Token token_finalize_block(&mut self.state, emit_events, is_masp_new_epoch)?; // - PoS @@ -1133,6 +1128,91 @@ fn pos_votes_from_abci( .collect() } +/// Dependency-injection indirection for governance system +fn gov_finalize_block( + shell: &mut Shell, + emit_events: &mut Vec, + current_epoch: Epoch, + is_new_epoch: bool, +) -> Result<()> +where + D: DB + for<'iter> DBIter<'iter> + Sync, + H: StorageHasher + Sync, +{ + let vp_wasm_cache = &mut shell.vp_wasm_cache; + let tx_wasm_cache = &mut shell.tx_wasm_cache; + governance::finalize_block::< + _, + token::Store<_>, + proof_of_stake::Store<_>, + _, + _, + >( + &mut shell.state, + emit_events, + current_epoch, + is_new_epoch, + |tx, state| { + let dispatch_result = protocol::dispatch_tx( + tx, + protocol::DispatchArgs::Raw { + wrapper_hash: None, + tx_index: TxIndex::default(), + wrapper_tx_result: None, + vp_wasm_cache, + tx_wasm_cache, + }, + // No gas limit for governance proposal + &RefCell::new(TxGasMeter::new(u64::MAX)), + state, + ); + // Governance must construct the tx with data and code commitments + let cmt = tx.first_commitments().unwrap().to_owned(); + match dispatch_result { + Ok(extended_tx_result) => match extended_tx_result + .tx_result + .get_inner_tx_result(None, either::Right(&cmt)) + { + Some(Ok(batched_result)) + if batched_result.is_accepted() => + { + state.write_log_mut().commit_batch_and_current_tx(); + Ok(true) + } + Some(Err(e)) => { + tracing::warn!( + "Error executing governance proposal {e}", + ); + state.write_log_mut().drop_batch(); + Ok(false) + } + _ => { + tracing::warn!("not sure what happen"); + state.write_log_mut().drop_batch(); + Ok(false) + } + }, + Err(e) => { + tracing::warn!( + "Error executing governance proposal {}", + e.error + ); + state.write_log_mut().drop_batch(); + Ok(false) + } + } + }, + |state, token, source, target| { + ibc::transfer_over_ibc::< + _, + parameters::Store<_>, + token::Store<_>, + token::Transfer, + >(state, token, source, target) + }, + ) +} + /// Dependency-injection indirection for token system fn token_finalize_block( storage: &mut S, diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index 728a141ddb..4911a4c6f8 100644 --- a/crates/node/src/shell/mod.rs +++ b/crates/node/src/shell/mod.rs @@ -7,7 +7,6 @@ //! More info in . pub mod block_alloc; mod finalize_block; -mod governance; mod init_chain; pub use init_chain::InitChainValidation; use namada_apps_lib::config::NodeLocalConfig; @@ -23,7 +22,6 @@ mod stats; #[cfg(any(test, feature = "testing"))] #[allow(dead_code)] pub mod testing; -pub mod utils; mod vote_extensions; use std::cell::RefCell; @@ -65,7 +63,8 @@ pub use namada_sdk::tx::data::ResultCode; use namada_sdk::tx::data::{TxType, WrapperTx}; use namada_sdk::tx::{Section, Tx}; use namada_sdk::{ - eth_bridge, hints, migrations, parameters, proof_of_stake, token, + eth_bridge, governance, hints, migrations, parameters, proof_of_stake, + token, }; use namada_vm::wasm::{TxCache, VpCache}; use namada_vm::{WasmCacheAccess, WasmCacheRwAccess}; diff --git a/crates/node/src/shell/utils.rs b/crates/node/src/shell/utils.rs deleted file mode 100644 index 6e405f54c8..0000000000 --- a/crates/node/src/shell/utils.rs +++ /dev/null @@ -1,14 +0,0 @@ -use borsh::BorshDeserialize; -use namada_sdk::state::{self, StorageRead}; -use namada_sdk::storage::Key; - -pub(super) fn force_read(storage: &S, key: &Key) -> state::Result -where - S: StorageRead, - T: BorshDeserialize, -{ - storage - .read::(key) - .transpose() - .expect("Storage key must be present.") -} diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index 3e266849a5..fdfc2de19e 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -139,6 +139,63 @@ where let params = storage::read_owned_pos_params(storage)?; Ok(params.pipeline_len) } + + fn total_active_stake( + storage: &S, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read, + { + let params = storage::read_pos_params::(storage)?; + storage::read_total_active_stake(storage, ¶ms, epoch) + } + + fn is_active_validator( + storage: &S, + validator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read, + { + let validator_state = + storage::read_validator_state::(storage, validator, epoch)?; + + Ok(!matches!( + validator_state, + None | Some(ValidatorState::Jailed) + | Some(ValidatorState::Inactive) + )) + } + + fn read_validator_stake( + storage: &S, + validator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read, + { + let params = storage::read_pos_params::(storage)?; + read_validator_stake(storage, ¶ms, validator, epoch) + } + + fn bond_amount( + storage: &S, + validator: &Address, + delegator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read, + { + let bond_id = BondId { + source: delegator.clone(), + validator: validator.clone(), + }; + bond_amount::(storage, &bond_id, epoch) + } } /// Address of the PoS account implemented as a native VP diff --git a/crates/proof_of_stake/src/storage.rs b/crates/proof_of_stake/src/storage.rs index 7cd402bbcb..a3d0a61cea 100644 --- a/crates/proof_of_stake/src/storage.rs +++ b/crates/proof_of_stake/src/storage.rs @@ -455,14 +455,14 @@ where pub fn read_validator_state( storage: &S, validator: &Address, - epoch: &Epoch, + epoch: Epoch, ) -> Result> where S: StorageRead, Gov: governance::Read, { let params = read_pos_params::(storage)?; - validator_state_handle(validator).get(storage, *epoch, ¶ms) + validator_state_handle(validator).get(storage, epoch, ¶ms) } /// Read PoS validator's delta value. diff --git a/crates/sdk/src/queries/vp/pos.rs b/crates/sdk/src/queries/vp/pos.rs index 0c2568f973..2497c4cb23 100644 --- a/crates/sdk/src/queries/vp/pos.rs +++ b/crates/sdk/src/queries/vp/pos.rs @@ -317,7 +317,7 @@ where let state = namada_proof_of_stake::storage::read_validator_state::< _, governance::Store<_>, - >(ctx.state, &validator, &epoch)?; + >(ctx.state, &validator, epoch)?; Ok((state, epoch)) } diff --git a/crates/systems/Cargo.toml b/crates/systems/Cargo.toml index 8bfa748544..57cbe11a00 100644 --- a/crates/systems/Cargo.toml +++ b/crates/systems/Cargo.toml @@ -14,6 +14,7 @@ version.workspace = true [dependencies] namada_core = { path = "../core" } +namada_events = { path = "../events" } namada_storage = { path = "../storage" } [dev-dependencies] diff --git a/crates/systems/src/proof_of_stake.rs b/crates/systems/src/proof_of_stake.rs index d81957054d..4f92a414d7 100644 --- a/crates/systems/src/proof_of_stake.rs +++ b/crates/systems/src/proof_of_stake.rs @@ -2,8 +2,11 @@ use namada_core::address::Address; use namada_core::chain::Epoch; +use namada_core::token; pub use namada_storage::Result; +use crate::governance; + /// Abstract PoS storage read interface pub trait Read { /// Check if the provided address is a validator address @@ -19,4 +22,45 @@ pub trait Read { /// Read PoS pipeline length parameter fn pipeline_len(storage: &S) -> Result; + + /// Read total active stake + fn total_active_stake( + storage: &S, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read; + + /// Returns `Ok(true)` if the given address is a validator and it's not + /// jailed or inactive + fn is_active_validator( + storage: &S, + validator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read; + + /// Read PoS validator's stake. + /// For non-validators and validators with `0` stake, this returns the + /// default - `token::Amount::zero()`. + fn read_validator_stake( + storage: &S, + validator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read; + + /// Get the total bond amount, including slashes, for a given bond ID and + /// epoch. Returns the bond amount after slashing. For future epochs, + /// the value is subject to change. + fn bond_amount( + storage: &S, + validator: &Address, + delegator: &Address, + epoch: Epoch, + ) -> Result + where + Gov: governance::Read; } diff --git a/crates/systems/src/trans_token.rs b/crates/systems/src/trans_token.rs index 16440d3f2f..715a841685 100644 --- a/crates/systems/src/trans_token.rs +++ b/crates/systems/src/trans_token.rs @@ -5,6 +5,8 @@ use std::borrow::Cow; use namada_core::address::Address; pub use namada_core::token::*; use namada_core::{storage, token}; +pub use namada_events::extend::UserAccount; +pub use namada_events::EventLevel; pub use namada_storage::Result; /// Abstract token keys interface @@ -94,6 +96,17 @@ pub trait Write: Read { /// Abstract token events interface pub trait Events: Read { + /// Emit transfer token event + fn emit_transfer_event( + storage: &mut S, + descriptor: Cow<'static, str>, + level: EventLevel, + token: &Address, + amount: token::Amount, + source: UserAccount, + target: UserAccount, + ) -> Result<()>; + /// Emit mint token event fn emit_mint_event( storage: &mut S, diff --git a/crates/trans_token/src/lib.rs b/crates/trans_token/src/lib.rs index 6865014817..56513caee6 100644 --- a/crates/trans_token/src/lib.rs +++ b/crates/trans_token/src/lib.rs @@ -22,14 +22,14 @@ mod storage; pub mod storage_key; pub mod vp; +use std::collections::BTreeMap; use std::marker::PhantomData; use event::{TokenEvent, TokenOperation}; use namada_core::address::Address; use namada_core::token; use namada_core::uint::Uint; -use namada_events::extend::UserAccount; -use namada_events::{EmitEvents, EventLevel}; +use namada_events::EmitEvents; pub use namada_state::{ Error, Key, Result, ResultExt, StorageRead, StorageWrite, }; @@ -181,4 +181,54 @@ where }); Ok(()) } + + fn emit_transfer_event( + storage: &mut S, + descriptor: std::borrow::Cow<'static, str>, + level: EventLevel, + token: &Address, + amount: token::Amount, + source: UserAccount, + target: UserAccount, + ) -> Result<()> { + let post_source_balance: Option = match &source { + UserAccount::Internal(addr) => { + Some(Self::read_balance(storage, token, addr)?.into()) + } + UserAccount::External(_) => None, + }; + let post_target_balance: Option = match &target { + UserAccount::Internal(addr) => { + Some(Self::read_balance(storage, token, addr)?.into()) + } + UserAccount::External(_) => None, + }; + + let sources = BTreeMap::from_iter([( + (source.clone(), token.clone()), + amount.into(), + )]); + let targets = BTreeMap::from_iter([( + (target.clone(), token.clone()), + amount.into(), + )]); + + let mut post_balances = BTreeMap::new(); + if let Some(balance) = post_source_balance { + post_balances.insert((source, token.clone()), balance); + } + if let Some(balance) = post_target_balance { + post_balances.insert((target, token.clone()), balance); + } + storage.emit(TokenEvent { + descriptor, + level, + operation: TokenOperation::Transfer { + sources, + targets, + post_balances, + }, + }); + Ok(()) + } } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index ff9d872023..8af9b5f22a 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3924,6 +3924,7 @@ name = "namada_systems" version = "0.43.0" dependencies = [ "namada_core", + "namada_events", "namada_storage", ] diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index bc9fb2d037..53205d180b 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2147,6 +2147,7 @@ name = "namada_systems" version = "0.43.0" dependencies = [ "namada_core", + "namada_events", "namada_storage", ] From 74ea21b7ac6e57fc01ebdac5588e760864248875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Aug 2024 11:54:30 +0100 Subject: [PATCH 3/4] governance: refactor proposal tx error handling --- crates/node/src/shell/finalize_block.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index e30a018b08..68e4ffe17c 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -1172,25 +1172,28 @@ where Ok(extended_tx_result) => match extended_tx_result .tx_result .get_inner_tx_result(None, either::Right(&cmt)) + .expect("Proposal tx must have a result") { - Some(Ok(batched_result)) - if batched_result.is_accepted() => - { - state.write_log_mut().commit_batch_and_current_tx(); - Ok(true) + Ok(batched_result) => { + if batched_result.is_accepted() { + state.write_log_mut().commit_batch_and_current_tx(); + Ok(true) + } else { + tracing::warn!( + "Governance proposal rejected by VP(s): {}", + batched_result.vps_result + ); + state.write_log_mut().drop_batch(); + Ok(false) + } } - Some(Err(e)) => { + Err(e) => { tracing::warn!( "Error executing governance proposal {e}", ); state.write_log_mut().drop_batch(); Ok(false) } - _ => { - tracing::warn!("not sure what happen"); - state.write_log_mut().drop_batch(); - Ok(false) - } }, Err(e) => { tracing::warn!( From c6209226992e444d46abae9173baeb09b76f3fcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 30 Aug 2024 12:51:05 +0100 Subject: [PATCH 4/4] changelog: add #3718 --- .changelog/unreleased/improvements/3718-gov-refactor.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/3718-gov-refactor.md diff --git a/.changelog/unreleased/improvements/3718-gov-refactor.md b/.changelog/unreleased/improvements/3718-gov-refactor.md new file mode 100644 index 0000000000..bb7ffc10cb --- /dev/null +++ b/.changelog/unreleased/improvements/3718-gov-refactor.md @@ -0,0 +1,2 @@ +- Moved governance shell sub-module into governance crate using dependency- + injection. ([\#3718](https://github.com/anoma/namada/pull/3718)) \ No newline at end of file