From 4fc3d95ba8d356044aff8544e504bcc89900de8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Thu, 12 Sep 2024 10:22:09 +0200 Subject: [PATCH] parameters: env agnostic VP --- Cargo.lock | 2 +- crates/benches/native_vps.rs | 19 +++---- crates/node/src/protocol.rs | 15 +++--- crates/node/src/shell/finalize_block.rs | 15 +++--- crates/parameters/Cargo.toml | 2 +- crates/parameters/src/vp.rs | 57 ++++++--------------- crates/sdk/src/validation.rs | 10 ++-- crates/vp_env/src/lib.rs | 66 ++++++++----------------- wasm/Cargo.lock | 2 +- wasm_for_tests/Cargo.lock | 2 +- 10 files changed, 70 insertions(+), 120 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 171f8e4c26..7fbd5780e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5233,7 +5233,7 @@ dependencies = [ "namada_state", "namada_systems", "namada_tx", - "namada_vp", + "namada_vp_env", "smooth-operator", "thiserror", ] diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index 4f4ab800c3..58806363e9 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -62,6 +62,7 @@ use namada_node::bench_utils::{ TX_BRIDGE_POOL_WASM, TX_IBC_WASM, TX_INIT_PROPOSAL_WASM, TX_RESIGN_STEWARD, TX_TRANSFER_WASM, TX_UPDATE_STEWARD_COMMISSION, TX_VOTE_PROPOSAL_WASM, }; +use namada_vm::wasm::run::VpEvalWasm; use namada_vp::native_vp::{Ctx, NativeVp}; use rand_core::OsRng; @@ -1557,7 +1558,7 @@ fn parameters(c: &mut Criterion) { let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( &TxGasMeter::new(u64::MAX), )); - let parameters = ParametersVp::new(Ctx::new( + let ctx = Ctx::<_, _, VpEvalWasm<_, _, _>>::new( &vp_address, &shell.state, &signed_tx.tx, @@ -1567,18 +1568,18 @@ fn parameters(c: &mut Criterion) { &keys_changed, &verifiers, shell.vp_wasm_cache.clone(), - )); + ); group.bench_function(bench_name, |b| { b.iter(|| { assert!( - parameters - .validate_tx( - &signed_tx.to_ref(), - parameters.ctx.keys_changed, - parameters.ctx.verifiers, - ) - .is_ok() + ParametersVp::validate_tx( + &ctx, + &signed_tx.to_ref(), + ctx.keys_changed, + ctx.verifiers, + ) + .is_ok() ) }) }); diff --git a/crates/node/src/protocol.rs b/crates/node/src/protocol.rs index f9bcde8fa8..1ace8b45bf 100644 --- a/crates/node/src/protocol.rs +++ b/crates/node/src/protocol.rs @@ -1238,14 +1238,13 @@ where .map_err(Error::NativeVpError) } InternalAddress::Parameters => { - let parameters = ParametersVp::new(ctx); - parameters - .validate_tx( - batched_tx, - &keys_changed, - &verifiers, - ) - .map_err(Error::NativeVpError) + ParametersVp::validate_tx( + &ctx, + batched_tx, + &keys_changed, + &verifiers, + ) + .map_err(Error::NativeVpError) } InternalAddress::PosSlashPool => { Err(Error::AccessForbidden( diff --git a/crates/node/src/shell/finalize_block.rs b/crates/node/src/shell/finalize_block.rs index 04c816470e..8acc54e240 100644 --- a/crates/node/src/shell/finalize_block.rs +++ b/crates/node/src/shell/finalize_block.rs @@ -1339,8 +1339,8 @@ mod test_finalize_block { use namada_sdk::validation::ParametersVp; use namada_test_utils::tx_data::TxWriteData; use namada_test_utils::TestWasms; + use namada_vm::wasm::run::VpEvalWasm; use namada_vote_ext::ethereum_events; - use namada_vp::native_vp::NativeVp; use proof_of_stake::{bond_tokens, PosParams}; use test_log::test; @@ -5650,7 +5650,7 @@ mod test_finalize_block { let keys_changed = BTreeSet::from([min_confirmations_key()]); let verifiers = BTreeSet::default(); let batched_tx = tx.batch_ref_first_tx().unwrap(); - let ctx = namada_vp::native_vp::Ctx::new( + let ctx = namada_vp::native_vp::Ctx::<_, _, VpEvalWasm<_, _, _>>::new( shell.mode.get_validator_address().expect("Test failed"), shell.state.read_only(), batched_tx.tx, @@ -5661,11 +5661,14 @@ mod test_finalize_block { &verifiers, shell.vp_wasm_cache.clone(), ); - let parameters = ParametersVp::new(ctx); assert!( - parameters - .validate_tx(&batched_tx, &keys_changed, &verifiers) - .is_ok() + ParametersVp::validate_tx( + &ctx, + &batched_tx, + &keys_changed, + &verifiers + ) + .is_ok() ); // we advance forward to the next epoch diff --git a/crates/parameters/Cargo.toml b/crates/parameters/Cargo.toml index de2ad221d6..214a5fab6f 100644 --- a/crates/parameters/Cargo.toml +++ b/crates/parameters/Cargo.toml @@ -25,7 +25,7 @@ namada_macros = { path = "../macros" } namada_state = { path = "../state" } namada_systems = { path = "../systems" } namada_tx = { path = "../tx" } -namada_vp = { path = "../vp" } +namada_vp_env = { path = "../vp_env" } smooth-operator.workspace = true thiserror.workspace = true diff --git a/crates/parameters/src/vp.rs b/crates/parameters/src/vp.rs index 3d34467a30..50f3b462b9 100644 --- a/crates/parameters/src/vp.rs +++ b/crates/parameters/src/vp.rs @@ -1,40 +1,30 @@ -//! Native VP for protocol parameters +//! VP for protocol parameters use std::collections::BTreeSet; use std::marker::PhantomData; use namada_core::address::Address; use namada_core::booleans::BoolResultUnitExt; -use namada_state::{Key, StateRead}; use namada_systems::governance; use namada_tx::BatchedTxRef; -use namada_vp::native_vp::{ - Ctx, CtxPreStorageRead, Error, NativeVp, Result, VpEvaluator, -}; +use namada_vp_env::{Error, Key, Result, VpEnv}; use crate::storage; /// Parameters VP -pub struct ParametersVp<'ctx, S, CA, EVAL, Gov> -where - S: 'static + StateRead, -{ - /// Context to interact with the host structures. - pub ctx: Ctx<'ctx, S, CA, EVAL>, - /// Generic types for DI - pub _marker: PhantomData, +pub struct ParametersVp<'ctx, CTX, Gov> { + /// Generic types for VP context and DI + pub _marker: PhantomData<(&'ctx CTX, Gov)>, } -impl<'view, 'ctx: 'view, S, CA, EVAL, Gov> NativeVp<'view> - for ParametersVp<'ctx, S, CA, EVAL, Gov> +impl<'ctx, CTX, Gov> ParametersVp<'ctx, CTX, Gov> where - S: 'static + StateRead, - CA: 'static + Clone, - EVAL: 'static + VpEvaluator<'ctx, S, CA, EVAL>, - Gov: governance::Read>, + CTX: VpEnv<'ctx>, + Gov: governance::Read<>::Pre>, { - fn validate_tx( - &'view self, + /// Run the validity predicate + pub fn validate_tx( + ctx: &'ctx CTX, batched_tx: &BatchedTxRef<'_>, keys_changed: &BTreeSet, _verifiers: &BTreeSet
, @@ -45,20 +35,21 @@ where data } else { return Err(Error::new_const( - "Token parameter changes require tx data to be present", + "Parameter changes require tx data to be present", )); }; match key_type { KeyType::PARAMETER | KeyType::UNKNOWN_PARAMETER => { - Gov::is_proposal_accepted(&self.ctx.pre(), &data)? - .ok_or_else(|| { + Gov::is_proposal_accepted(&ctx.pre(), &data)?.ok_or_else( + || { Error::new_alloc(format!( "Attempted to change a protocol parameter \ from outside of a governance proposal, or \ from a non-accepted governance proposal: \ {key}", )) - }) + }, + ) } KeyType::UNKNOWN => Ok(()), } @@ -66,22 +57,6 @@ where } } -impl<'ctx, S, CA, EVAL, Gov> ParametersVp<'ctx, S, CA, EVAL, Gov> -where - S: 'static + StateRead, - CA: 'static + Clone, - EVAL: 'static + VpEvaluator<'ctx, S, CA, EVAL>, - Gov: governance::Read>, -{ - /// Instantiate parameters VP - pub fn new(ctx: Ctx<'ctx, S, CA, EVAL>) -> Self { - Self { - ctx, - _marker: PhantomData, - } - } -} - #[allow(clippy::upper_case_acronyms)] enum KeyType { #[allow(clippy::upper_case_acronyms)] diff --git a/crates/sdk/src/validation.rs b/crates/sdk/src/validation.rs index c1c8d8ab13..fa56983fab 100644 --- a/crates/sdk/src/validation.rs +++ b/crates/sdk/src/validation.rs @@ -51,12 +51,10 @@ pub type IbcVpContext<'view, 'a, S, CA, EVAL> = >; /// Native parameters VP -pub type ParametersVp<'a, S, CA> = parameters::vp::ParametersVp< - 'a, - S, - VpCache, - Eval, - GovPreStore<'a, S, CA>, +pub type ParametersVp<'ctx, CTX> = parameters::vp::ParametersVp< + 'ctx, + CTX, + governance::Store<>::Pre>, >; /// Native governance VP diff --git a/crates/vp_env/src/lib.rs b/crates/vp_env/src/lib.rs index 7b1dab928c..7f3a1abbec 100644 --- a/crates/vp_env/src/lib.rs +++ b/crates/vp_env/src/lib.rs @@ -25,10 +25,9 @@ use namada_core::borsh::BorshDeserialize; use namada_core::chain::ChainId; pub use namada_core::chain::{BlockHeader, BlockHeight, Epoch, Epochs}; use namada_core::hash::Hash; -pub use namada_core::storage::{Key, TxIndex}; use namada_events::{Event, EventType}; use namada_gas::Gas; -pub use namada_storage::StorageRead; +pub use namada_storage::{Error, Key, Result, StorageRead, TxIndex}; use namada_tx::BatchedTxRef; /// Validity predicate's environment is available for native VPs and WASM VPs @@ -56,56 +55,47 @@ where /// Storage read temporary state Borsh encoded value (after tx execution). /// It will try to read from only the write log and then decode it if /// found. - fn read_temp( - &self, - key: &Key, - ) -> Result, namada_storage::Error>; + fn read_temp(&self, key: &Key) -> Result>; /// Storage read temporary state raw bytes (after tx execution). It will try /// to read from only the write log. - fn read_bytes_temp( - &self, - key: &Key, - ) -> Result>, namada_storage::Error>; + fn read_bytes_temp(&self, key: &Key) -> Result>>; /// Getting the chain ID. - fn get_chain_id(&self) -> Result; + fn get_chain_id(&self) -> Result; /// Getting the block height. The height is that of the block to which the /// current transaction is being applied. - fn get_block_height(&self) -> Result; + fn get_block_height(&self) -> Result; /// Getting the block header. fn get_block_header( &self, height: BlockHeight, - ) -> Result, namada_storage::Error>; + ) -> Result>; /// Getting the block epoch. The epoch is that of the block to which the /// current transaction is being applied. - fn get_block_epoch(&self) -> Result; + fn get_block_epoch(&self) -> Result; /// Get the shielded transaction index. - fn get_tx_index(&self) -> Result; + fn get_tx_index(&self) -> Result; /// Get the address of the native token. - fn get_native_token(&self) -> Result; + fn get_native_token(&self) -> Result
; /// Given the information about predecessor block epochs fn get_pred_epochs(&self) -> namada_storage::Result; /// Get the events emitted by the current tx. - fn get_events( - &self, - event_type: &EventType, - ) -> Result, namada_storage::Error>; + fn get_events(&self, event_type: &EventType) -> Result>; /// Storage prefix iterator, ordered by storage keys. It will try to get an /// iterator from the storage. fn iter_prefix<'iter>( &'iter self, prefix: &Key, - ) -> Result, namada_storage::Error>; + ) -> Result>; /// Evaluate a validity predicate with given data. The address, changed /// storage keys and verifiers will have the same values as the input to @@ -113,17 +103,13 @@ where /// /// If the execution fails for whatever reason, this will return `false`. /// Otherwise returns the result of evaluation. - fn eval( - &self, - vp_code: Hash, - input_data: BatchedTxRef<'_>, - ) -> Result<(), namada_storage::Error>; + fn eval(&self, vp_code: Hash, input_data: BatchedTxRef<'_>) -> Result<()>; /// Get a tx hash - fn get_tx_code_hash(&self) -> Result, namada_storage::Error>; + fn get_tx_code_hash(&self) -> Result>; /// Charge the provided gas for the current vp - fn charge_gas(&self, used_gas: Gas) -> Result<(), namada_storage::Error>; + fn charge_gas(&self, used_gas: Gas) -> Result<()>; // ---- Methods below have default implementation via `pre/post` ---- @@ -132,16 +118,13 @@ where fn read_pre( &'view self, key: &Key, - ) -> Result, namada_storage::Error> { + ) -> Result> { self.pre().read(key) } /// Storage read prior state raw bytes (before tx execution). It /// will try to read from the storage. - fn read_bytes_pre( - &'view self, - key: &Key, - ) -> Result>, namada_storage::Error> { + fn read_bytes_pre(&'view self, key: &Key) -> Result>> { self.pre().read_bytes(key) } @@ -151,35 +134,26 @@ where fn read_post( &'view self, key: &Key, - ) -> Result, namada_storage::Error> { + ) -> Result> { self.post().read(key) } /// Storage read posterior state raw bytes (after tx execution). It will try /// to read from the write log first and if no entry found then from the /// storage. - fn read_bytes_post( - &'view self, - key: &Key, - ) -> Result>, namada_storage::Error> { + fn read_bytes_post(&'view self, key: &Key) -> Result>> { self.post().read_bytes(key) } /// Storage `has_key` in prior state (before tx execution). It will try to /// read from the storage. - fn has_key_pre( - &'view self, - key: &Key, - ) -> Result { + fn has_key_pre(&'view self, key: &Key) -> Result { self.pre().has_key(key) } /// Storage `has_key` in posterior state (after tx execution). It will try /// to check the write log first and if no entry found then the storage. - fn has_key_post( - &'view self, - key: &Key, - ) -> Result { + fn has_key_post(&'view self, key: &Key) -> Result { self.post().has_key(key) } } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index a9c4964664..34c76b18dd 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3849,7 +3849,7 @@ dependencies = [ "namada_state", "namada_systems", "namada_tx", - "namada_vp", + "namada_vp_env", "smooth-operator", "thiserror", ] diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 9ecc644451..468a222e42 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2122,7 +2122,7 @@ dependencies = [ "namada_state", "namada_systems", "namada_tx", - "namada_vp", + "namada_vp_env", "smooth-operator", "thiserror", ]