diff --git a/.changelog/unreleased/improvements/331-common-write-storage-trait.md b/.changelog/unreleased/improvements/331-common-write-storage-trait.md new file mode 100644 index 0000000000..2e3e605197 --- /dev/null +++ b/.changelog/unreleased/improvements/331-common-write-storage-trait.md @@ -0,0 +1,2 @@ +- Added a StorageWrite trait for a common interface for transactions and direct + storage access for protocol ([#331](https://github.com/anoma/namada/pull/331)) \ No newline at end of file diff --git a/shared/src/ledger/native_vp.rs b/shared/src/ledger/native_vp.rs index fa30efb7ea..5cbab7d639 100644 --- a/shared/src/ledger/native_vp.rs +++ b/shared/src/ledger/native_vp.rs @@ -150,7 +150,7 @@ where } } -impl<'f, 'a, DB, H, CA> StorageRead for CtxPreStorageRead<'f, 'a, DB, H, CA> +impl<'f, 'a, DB, H, CA> StorageRead<'f> for CtxPreStorageRead<'f, 'a, DB, H, CA> where DB: 'static + storage::DB + for<'iter> storage::DBIter<'iter>, H: 'static + StorageHasher, @@ -210,7 +210,8 @@ where } } -impl<'f, 'a, DB, H, CA> StorageRead for CtxPostStorageRead<'f, 'a, DB, H, CA> +impl<'f, 'a, DB, H, CA> StorageRead<'f> + for CtxPostStorageRead<'f, 'a, DB, H, CA> where DB: 'static + storage::DB + for<'iter> storage::DBIter<'iter>, H: 'static + StorageHasher, diff --git a/shared/src/ledger/storage/mod.rs b/shared/src/ledger/storage/mod.rs index 1f106bcf69..d8c8e28091 100644 --- a/shared/src/ledger/storage/mod.rs +++ b/shared/src/ledger/storage/mod.rs @@ -12,7 +12,7 @@ use tendermint::merkle::proof::Proof; use thiserror::Error; use super::parameters::Parameters; -use super::storage_api::{ResultExt, StorageRead}; +use super::storage_api::{ResultExt, StorageRead, StorageWrite}; use super::{parameters, storage_api}; use crate::ledger::gas::MIN_STORAGE_GAS; use crate::ledger::parameters::EpochDuration; @@ -424,12 +424,15 @@ where pub fn write( &mut self, key: &Key, - value: impl AsRef<[u8]> + Clone, + value: impl AsRef<[u8]>, ) -> Result<(u64, i64)> { + // Note that this method is the same as `StorageWrite::write_bytes`, + // but with gas and storage bytes len diff accounting tracing::debug!("storage write key {}", key,); - self.block.tree.update(key, value.clone())?; + let value = value.as_ref(); + self.block.tree.update(key, &value)?; - let len = value.as_ref().len(); + let len = value.len(); let gas = key.len() + len; let size_diff = self.db.write_subspace_val(self.last_height, key, value)?; @@ -439,6 +442,8 @@ where /// Delete the specified subspace and returns the gas cost and the size /// difference pub fn delete(&mut self, key: &Key) -> Result<(u64, i64)> { + // Note that this method is the same as `StorageWrite::delete`, + // but with gas and storage bytes len diff accounting let mut deleted_bytes_len = 0; if self.has_key(key)?.0 { self.block.tree.delete(key)?; @@ -685,10 +690,7 @@ where } } -// The `'iter` lifetime is needed for the associated type `PrefixIter`. -// Note that the `D: DBIter<'iter>` bound uses another higher-rank lifetime -// (see https://doc.rust-lang.org/nomicon/hrtb.html). -impl<'iter, D, H> StorageRead for &'iter Storage +impl<'iter, D, H> StorageRead<'iter> for Storage where D: DB + for<'iter_> DBIter<'iter_>, H: StorageHasher, @@ -721,7 +723,7 @@ where } fn iter_prefix( - &self, + &'iter self, prefix: &crate::types::storage::Key, ) -> std::result::Result { Ok(self.db.iter_prefix(prefix)) @@ -758,6 +760,53 @@ where } } +impl StorageWrite for Storage +where + D: DB + for<'iter> DBIter<'iter>, + H: StorageHasher, +{ + fn write( + &mut self, + key: &crate::types::storage::Key, + val: T, + ) -> storage_api::Result<()> { + let val = val.try_to_vec().into_storage_result()?; + self.write_bytes(key, val) + } + + fn write_bytes( + &mut self, + key: &crate::types::storage::Key, + val: impl AsRef<[u8]>, + ) -> storage_api::Result<()> { + // Note that this method is the same as `Storage::write`, but without + // gas and storage bytes len diff accounting, because it can only be + // used by the protocol that has a direct mutable access to storage + let val = val.as_ref(); + self.block.tree.update(key, &val).into_storage_result()?; + let _ = self + .db + .write_subspace_val(self.block.height, key, val) + .into_storage_result()?; + Ok(()) + } + + fn delete( + &mut self, + key: &crate::types::storage::Key, + ) -> storage_api::Result<()> { + // Note that this method is the same as `Storage::delete`, but without + // gas and storage bytes len diff accounting, because it can only be + // used by the protocol that has a direct mutable access to storage + self.block.tree.delete(key).into_storage_result()?; + let _ = self + .db + .delete_subspace_val(self.block.height, key) + .into_storage_result()?; + Ok(()) + } +} + impl From for Error { fn from(error: MerkleTreeError) -> Self { Self::MerkleTreeError(error) diff --git a/shared/src/ledger/storage_api/mod.rs b/shared/src/ledger/storage_api/mod.rs index 0c0e4f4083..0e7e299970 100644 --- a/shared/src/ledger/storage_api/mod.rs +++ b/shared/src/ledger/storage_api/mod.rs @@ -3,13 +3,32 @@ mod error; -use borsh::BorshDeserialize; +use borsh::{BorshDeserialize, BorshSerialize}; pub use error::{CustomError, Error, Result, ResultExt}; use crate::types::storage::{self, BlockHash, BlockHeight, Epoch}; /// Common storage read interface -pub trait StorageRead { +/// +/// If you're using this trait and having compiler complaining about needing an +/// explicit lifetime parameter, simply use trait bounds with the following +/// syntax: +/// +/// ```rust,ignore +/// where +/// S: for<'iter> StorageRead<'iter> +/// ``` +/// +/// If you want to know why this is needed, see the to-do task below. The +/// syntax for this relies on higher-rank lifetimes, see e.g. +/// . +/// +/// TODO: once GATs are stabilized, we should be able to remove the `'iter` +/// lifetime param that is currently the only way to make the prefix iterator +/// typecheck in the `>::PrefixIter` associated type used in +/// `impl StorageRead for Storage` (shared/src/ledger/storage/mod.rs). +/// See +pub trait StorageRead<'iter> { /// Storage read prefix iterator type PrefixIter; @@ -28,7 +47,13 @@ pub trait StorageRead { /// Storage prefix iterator. It will try to get an iterator from the /// storage. - fn iter_prefix(&self, prefix: &storage::Key) -> Result; + /// + /// For a more user-friendly iterator API, use [`fn@iter_prefix`] or + /// [`fn@iter_prefix_bytes`] instead. + fn iter_prefix( + &'iter self, + prefix: &storage::Key, + ) -> Result; /// Storage prefix iterator for. It will try to read from the storage. fn iter_next( @@ -51,3 +76,23 @@ pub trait StorageRead { /// current transaction is being applied. fn get_block_epoch(&self) -> Result; } + +/// Common storage write interface +pub trait StorageWrite { + /// Write a value to be encoded with Borsh at the given key to storage. + fn write( + &mut self, + key: &storage::Key, + val: T, + ) -> Result<()>; + + /// Write a value as bytes at the given key to storage. + fn write_bytes( + &mut self, + key: &storage::Key, + val: impl AsRef<[u8]>, + ) -> Result<()>; + + /// Delete a value at the given key from storage. + fn delete(&mut self, key: &storage::Key) -> Result<()>; +} diff --git a/shared/src/ledger/tx_env.rs b/shared/src/ledger/tx_env.rs index 1fc7fa788c..1db8fad09b 100644 --- a/shared/src/ledger/tx_env.rs +++ b/shared/src/ledger/tx_env.rs @@ -3,34 +3,17 @@ use borsh::BorshSerialize; -use crate::ledger::storage_api::{self, StorageRead}; +use crate::ledger::storage_api::{StorageRead, StorageWrite}; use crate::types::address::Address; use crate::types::ibc::IbcEvent; use crate::types::storage; use crate::types::time::Rfc3339String; /// Transaction host functions -pub trait TxEnv: StorageRead { +pub trait TxEnv<'iter>: StorageRead<'iter> + StorageWrite { /// Host env functions possible errors type Error; - /// Write a value to be encoded with Borsh at the given key to storage. - fn write( - &mut self, - key: &storage::Key, - val: T, - ) -> Result<(), storage_api::Error>; - - /// Write a value as bytes at the given key to storage. - fn write_bytes( - &mut self, - key: &storage::Key, - val: impl AsRef<[u8]>, - ) -> Result<(), storage_api::Error>; - - /// Delete a value at the given key from storage. - fn delete(&mut self, key: &storage::Key) -> Result<(), storage_api::Error>; - /// Write a temporary value to be encoded with Borsh at the given key to /// storage. fn write_temp( diff --git a/tests/src/native_vp/pos.rs b/tests/src/native_vp/pos.rs index 7f8d3a15db..ec6f75a15f 100644 --- a/tests/src/native_vp/pos.rs +++ b/tests/src/native_vp/pos.rs @@ -575,7 +575,6 @@ pub mod testing { use derivative::Derivative; use itertools::Either; use namada::ledger::pos::namada_proof_of_stake::btree_set::BTreeSetShims; - use namada::ledger::tx_env::TxEnv; use namada::types::key::common::PublicKey; use namada::types::key::RefTo; use namada::types::storage::Epoch; @@ -590,7 +589,7 @@ pub mod testing { use namada_tx_prelude::proof_of_stake::{ staking_token_address, BondId, Bonds, PosParams, Unbonds, }; - use namada_tx_prelude::{Address, StorageRead}; + use namada_tx_prelude::{Address, StorageRead, StorageWrite}; use proptest::prelude::*; use crate::tx::{self, tx_host_env}; diff --git a/tests/src/vm_host_env/ibc.rs b/tests/src/vm_host_env/ibc.rs index a838f37819..72b33f9e5e 100644 --- a/tests/src/vm_host_env/ibc.rs +++ b/tests/src/vm_host_env/ibc.rs @@ -68,6 +68,7 @@ use namada::types::ibc::data::FungibleTokenPacketData; use namada::types::storage::{self, BlockHash, BlockHeight}; use namada::types::token::{self, Amount}; use namada::vm::{wasm, WasmCacheRwAccess}; +use namada_tx_prelude::StorageWrite; use crate::tx::{self, *}; diff --git a/tests/src/vm_host_env/mod.rs b/tests/src/vm_host_env/mod.rs index cf9dccfb56..37d1afb790 100644 --- a/tests/src/vm_host_env/mod.rs +++ b/tests/src/vm_host_env/mod.rs @@ -34,7 +34,9 @@ mod tests { use namada::types::time::DateTimeUtc; use namada::types::token::{self, Amount}; use namada::types::{address, key}; - use namada_tx_prelude::{BorshDeserialize, BorshSerialize, StorageRead}; + use namada_tx_prelude::{ + BorshDeserialize, BorshSerialize, StorageRead, StorageWrite, + }; use namada_vp_prelude::VpEnv; use prost::Message; use test_log::test; diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index 7be4e4c064..4a8a3ef3a9 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -1,7 +1,7 @@ //! IBC lower-level functions for transactions. pub use namada::ledger::ibc::handler::{Error, IbcActions, Result}; -use namada::ledger::storage_api::StorageRead; +use namada::ledger::storage_api::{StorageRead, StorageWrite}; use namada::ledger::tx_env::TxEnv; use namada::types::address::Address; pub use namada::types::ibc::IbcEvent; diff --git a/tx_prelude/src/lib.rs b/tx_prelude/src/lib.rs index 42b63a892a..02341e70e5 100644 --- a/tx_prelude/src/lib.rs +++ b/tx_prelude/src/lib.rs @@ -23,7 +23,7 @@ pub use namada::ledger::governance::storage as gov_storage; pub use namada::ledger::parameters::storage as parameters_storage; pub use namada::ledger::storage::types::encode; use namada::ledger::storage_api; -pub use namada::ledger::storage_api::StorageRead; +pub use namada::ledger::storage_api::{StorageRead, StorageWrite}; pub use namada::ledger::treasury::storage as treasury_storage; pub use namada::ledger::tx_env::TxEnv; pub use namada::proto::{Signed, SignedTxData}; @@ -95,7 +95,7 @@ pub type TxResult = EnvResult<()>; #[derive(Debug)] pub struct KeyValIterator(pub u64, pub PhantomData); -impl StorageRead for Ctx { +impl StorageRead<'_> for Ctx { type PrefixIter = KeyValIterator<(String, Vec)>; fn read( @@ -188,19 +188,7 @@ impl StorageRead for Ctx { } } -impl TxEnv for Ctx { - type Error = Error; - - fn get_block_time(&self) -> Result { - let read_result = unsafe { anoma_tx_get_block_time() }; - let time_value = read_from_buffer(read_result, anoma_tx_result_buffer) - .expect("The block time should exist"); - Ok(Rfc3339String( - String::try_from_slice(&time_value[..]) - .expect("The conversion shouldn't fail"), - )) - } - +impl StorageWrite for Ctx { fn write( &mut self, key: &namada::types::storage::Key, @@ -227,6 +215,29 @@ impl TxEnv for Ctx { Ok(()) } + fn delete( + &mut self, + key: &namada::types::storage::Key, + ) -> storage_api::Result<()> { + let key = key.to_string(); + unsafe { anoma_tx_delete(key.as_ptr() as _, key.len() as _) }; + Ok(()) + } +} + +impl TxEnv<'_> for Ctx { + type Error = Error; + + fn get_block_time(&self) -> Result { + let read_result = unsafe { anoma_tx_get_block_time() }; + let time_value = read_from_buffer(read_result, anoma_tx_result_buffer) + .expect("The block time should exist"); + Ok(Rfc3339String( + String::try_from_slice(&time_value[..]) + .expect("The conversion shouldn't fail"), + )) + } + fn write_temp( &mut self, key: &namada::types::storage::Key, @@ -253,15 +264,6 @@ impl TxEnv for Ctx { Ok(()) } - fn delete( - &mut self, - key: &namada::types::storage::Key, - ) -> storage_api::Result<()> { - let key = key.to_string(); - unsafe { anoma_tx_delete(key.as_ptr() as _, key.len() as _) }; - Ok(()) - } - fn insert_verifier(&mut self, addr: &Address) -> Result<(), Error> { let addr = addr.encode(); unsafe { anoma_tx_insert_verifier(addr.as_ptr() as _, addr.len() as _) } diff --git a/vp_prelude/src/lib.rs b/vp_prelude/src/lib.rs index 37d1958aa9..24d702b6b5 100644 --- a/vp_prelude/src/lib.rs +++ b/vp_prelude/src/lib.rs @@ -295,7 +295,7 @@ impl VpEnv for Ctx { } } -impl StorageRead for CtxPreStorageRead<'_> { +impl StorageRead<'_> for CtxPreStorageRead<'_> { type PrefixIter = KeyValIterator<(String, Vec)>; fn read( @@ -372,7 +372,7 @@ impl StorageRead for CtxPreStorageRead<'_> { } } -impl StorageRead for CtxPostStorageRead<'_> { +impl StorageRead<'_> for CtxPostStorageRead<'_> { type PrefixIter = KeyValIterator<(String, Vec)>; fn read( diff --git a/wasm/checksums.json b/wasm/checksums.json index 4a72c13aae..78740b3592 100644 --- a/wasm/checksums.json +++ b/wasm/checksums.json @@ -1,17 +1,17 @@ { - "tx_bond.wasm": "tx_bond.08dbf08605594443dac2fcd4d2a78eaf9d83d063ac51eafe6c30b9bc44cda1e1.wasm", - "tx_from_intent.wasm": "tx_from_intent.c70e4d41a695f5a2f86b06b10ac5ec5812e4f5ce83e385180e2b7f90cd0a17eb.wasm", - "tx_ibc.wasm": "tx_ibc.33e3bc19674a732d400a2d971e498297b076593ed1aa2533b94f45f55e877069.wasm", - "tx_init_account.wasm": "tx_init_account.2dbc104e1721d85e22f22b5453665098078d6b3f3018ce078ec611a4b7dd24a7.wasm", - "tx_init_nft.wasm": "tx_init_nft.3a2e8501a53ff569b682d0eca04db9127e2279b60a1d28c82431bf4794e710a9.wasm", - "tx_init_proposal.wasm": "tx_init_proposal.92b4d1d3cb7f419e8def71402f3b0f66b37f09446f9e6d0ff9de92f0942b53ec.wasm", - "tx_init_validator.wasm": "tx_init_validator.ee5e145bc5074906ea04a4054184273222b8c8568e4000e9949800744dc2cf34.wasm", - "tx_mint_nft.wasm": "tx_mint_nft.e21ef7f68862681e46c9725d59ef106aae2eeacd5fc4df8554dabcc5eb861647.wasm", - "tx_transfer.wasm": "tx_transfer.246530cf1176e234ca7d6867007e48b4787e0a6ae58d8ca6ab2ead1056f24b46.wasm", - "tx_unbond.wasm": "tx_unbond.30c35eacb98d4c5bcbfbb283ea94aa29b287ceb40d46779af634a71691909bda.wasm", + "tx_bond.wasm": "tx_bond.37dda241d2041c7534a3e65199979af8de1c3128927048ebe5a69586ff7fe7a0.wasm", + "tx_from_intent.wasm": "tx_from_intent.b74d3c5b49c79ae3237b83fa896e04dc839194b5a3f74f5099b03663d06c81c8.wasm", + "tx_ibc.wasm": "tx_ibc.2642651c00baf6f8a9f078ac049cf6ba4fcd98d2a9ca8d4ec81d9d54b6d9ca8a.wasm", + "tx_init_account.wasm": "tx_init_account.4761be38a6fc5c731a310993d897f569f71701791105bbaac64070b3bab82905.wasm", + "tx_init_nft.wasm": "tx_init_nft.d3989943620ffd568800c44f47dbd8db00080fb5fc18e566df22e7a41d754c7a.wasm", + "tx_init_proposal.wasm": "tx_init_proposal.b9ae52416d25ed51b312b0dccb893a3de2feb1e1d1f73306d0ca7941f295904b.wasm", + "tx_init_validator.wasm": "tx_init_validator.3b0f95a2225510fc17cb28d971ac020767ecd5f1ebf6a9c2652f8338ed9ae2b0.wasm", + "tx_mint_nft.wasm": "tx_mint_nft.e53a544f131500db263abb4172039a1bdae035493b662782a7d4b0fbb77a228e.wasm", + "tx_transfer.wasm": "tx_transfer.3f092cc2b18c1031e0be662dca8a414de5d043082408f5989bb14b2aea5f5b3f.wasm", + "tx_unbond.wasm": "tx_unbond.4d937a43fc32a9e849abf801988b73298c96c652a4a72b887c9e7936488206cd.wasm", "tx_update_vp.wasm": "tx_update_vp.f32dd6d4225d426d60bf82cda4e6d6432f869a201b65ff135a9fd589679a9f0a.wasm", "tx_vote_proposal.wasm": "tx_vote_proposal.829b383d2db6b077b5a6112cacb75af63cc152bf2fb66e2df45f42d55c251b41.wasm", - "tx_withdraw.wasm": "tx_withdraw.664a908326d0ccd6ac2b17d9b27fef9e569361c8d87b5a591102972ab21b0a9a.wasm", + "tx_withdraw.wasm": "tx_withdraw.846c26a11c331652c80ed42f1b90caa90346b5e93bbb6761e5cae8f948f73851.wasm", "vp_nft.wasm": "vp_nft.ded411ba44c11b9fc6a3626c6858c760b0fae6d3cdef4664ec5ce0a938149edc.wasm", "vp_testnet_faucet.wasm": "vp_testnet_faucet.1ba83fcbf94b596c12313f405103baa6c10ef64f1c013134730c1926aed6503f.wasm", "vp_token.wasm": "vp_token.9799a1d3e276c19b58ad6e78f0f1c6f8d4b17da72ff7fee582658d33bbd67583.wasm", diff --git a/wasm/wasm_source/src/vp_nft.rs b/wasm/wasm_source/src/vp_nft.rs index 4ab7f232bd..956a0a5d42 100644 --- a/wasm/wasm_source/src/vp_nft.rs +++ b/wasm/wasm_source/src/vp_nft.rs @@ -43,7 +43,7 @@ mod tests { use namada_tests::log::test; use namada_tests::tx::{self, tx_host_env, TestTxEnv}; use namada_tests::vp::*; - use namada_tx_prelude::TxEnv; + use namada_tx_prelude::{StorageWrite, TxEnv}; use super::*; diff --git a/wasm/wasm_source/src/vp_testnet_faucet.rs b/wasm/wasm_source/src/vp_testnet_faucet.rs index a3a5630ab7..b599f82251 100644 --- a/wasm/wasm_source/src/vp_testnet_faucet.rs +++ b/wasm/wasm_source/src/vp_testnet_faucet.rs @@ -97,7 +97,7 @@ mod tests { use namada_tests::tx::{self, tx_host_env, TestTxEnv}; use namada_tests::vp::vp_host_env::storage::Key; use namada_tests::vp::*; - use namada_tx_prelude::TxEnv; + use namada_tx_prelude::{StorageWrite, TxEnv}; use namada_vp_prelude::key::RefTo; use proptest::prelude::*; use storage::testing::arb_account_storage_key_no_vp; diff --git a/wasm/wasm_source/src/vp_user.rs b/wasm/wasm_source/src/vp_user.rs index 3b6ddcf65f..d20472d9ec 100644 --- a/wasm/wasm_source/src/vp_user.rs +++ b/wasm/wasm_source/src/vp_user.rs @@ -380,7 +380,7 @@ mod tests { use namada_tests::tx::{self, tx_host_env, TestTxEnv}; use namada_tests::vp::vp_host_env::storage::Key; use namada_tests::vp::*; - use namada_tx_prelude::TxEnv; + use namada_tx_prelude::{StorageWrite, TxEnv}; use namada_vp_prelude::key::RefTo; use proptest::prelude::*; use storage::testing::arb_account_storage_key_no_vp;