From a89e9f808a4883685d83e89574a24ca656da55e8 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 11 May 2020 18:53:49 +0300 Subject: [PATCH 01/12] copied mock.rs and storage.rs from std to vm as mock/mod.rs and mock/storage.rs --- packages/vm/src/mock/mod.rs | 580 ++++++++++++++++++++++++++++++++ packages/vm/src/mock/storage.rs | 273 +++++++++++++++ 2 files changed, 853 insertions(+) create mode 100644 packages/vm/src/mock/mod.rs create mode 100644 packages/vm/src/mock/storage.rs diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs new file mode 100644 index 0000000000..80adc55e48 --- /dev/null +++ b/packages/vm/src/mock/mod.rs @@ -0,0 +1,580 @@ +use std::collections::HashMap; + +use crate::coins::Coin; +use crate::encoding::Binary; +use crate::errors::{generic_err, invalid_utf8, StdResult, SystemError}; +use crate::query::{AllBalanceResponse, BalanceResponse, BankQuery, QueryRequest, WasmQuery}; +use crate::serde::{from_slice, to_binary}; +use crate::storage::MemoryStorage; +use crate::traits::{Api, Extern, Querier, QuerierResult}; +use crate::types::{BlockInfo, CanonicalAddr, ContractInfo, Env, HumanAddr, MessageInfo, Never}; + +static CONTRACT_ADDR: &str = "cosmos2contract"; + +/// All external requirements that can be injected for unit tests. +/// It sets the given balance for the contract itself, nothing else +pub fn mock_dependencies( + canonical_length: usize, + contract_balance: &[Coin], +) -> Extern { + let contract_addr = HumanAddr::from(CONTRACT_ADDR); + Extern { + storage: MockStorage::default(), + api: MockApi::new(canonical_length), + querier: MockQuerier::new(&[(&contract_addr, contract_balance)]), + } +} + +/// Initializes the querier along with the mock_dependencies. +/// Sets all balances provided (yoy must explicitly set contract balance if desired) +pub fn mock_dependencies_with_balances( + canonical_length: usize, + balances: &[(&HumanAddr, &[Coin])], +) -> Extern { + Extern { + storage: MockStorage::default(), + api: MockApi::new(canonical_length), + querier: MockQuerier::new(balances), + } +} + +// Use MemoryStorage implementation (which is valid in non-testcode) +// We can later make simplifications here if needed +pub type MockStorage = MemoryStorage; + +// MockPrecompiles zero pads all human addresses to make them fit the canonical_length +// it trims off zeros for the reverse operation. +// not really smart, but allows us to see a difference (and consistent length for canonical adddresses) +#[derive(Copy, Clone)] +pub struct MockApi { + canonical_length: usize, +} + +impl MockApi { + pub fn new(canonical_length: usize) -> Self { + MockApi { canonical_length } + } +} + +impl Default for MockApi { + fn default() -> Self { + Self::new(20) + } +} + +impl Api for MockApi { + fn canonical_address(&self, human: &HumanAddr) -> StdResult { + // Dummy input validation. This is more sophisticated for formats like bech32, where format and checksum are validated. + if human.len() < 3 { + return Err(generic_err("Invalid input: human address too short")); + } + if human.len() > self.canonical_length { + return Err(generic_err("Invalid input: human address too long")); + } + + let mut out = Vec::from(human.as_str()); + let append = self.canonical_length - out.len(); + if append > 0 { + out.extend(vec![0u8; append]); + } + Ok(CanonicalAddr(Binary(out))) + } + + fn human_address(&self, canonical: &CanonicalAddr) -> StdResult { + if canonical.len() != self.canonical_length { + return Err(generic_err( + "Invalid input: canonical address length not correct", + )); + } + + // remove trailing 0's (TODO: fix this - but fine for first tests) + let trimmed: Vec = canonical + .as_slice() + .iter() + .cloned() + .filter(|&x| x != 0) + .collect(); + // decode UTF-8 bytes into string + let human = String::from_utf8(trimmed).map_err(invalid_utf8)?; + Ok(HumanAddr(human)) + } +} + +/// Just set sender and sent funds for the message. The rest uses defaults. +/// The sender will be canonicalized internally to allow developers pasing in human readable senders. +/// This is intended for use in test code only. +pub fn mock_env>(api: &T, sender: U, sent: &[Coin]) -> Env { + Env { + block: BlockInfo { + height: 12_345, + time: 1_571_797_419, + chain_id: "cosmos-testnet-14002".to_string(), + }, + message: MessageInfo { + sender: api.canonical_address(&sender.into()).unwrap(), + sent_funds: sent.to_vec(), + }, + contract: ContractInfo { + address: api + .canonical_address(&HumanAddr::from(CONTRACT_ADDR)) + .unwrap(), + }, + } +} + +/// MockQuerier holds an immutable table of bank balances +/// TODO: also allow querying contracts +#[derive(Clone, Default)] +pub struct MockQuerier { + bank: BankQuerier, + #[cfg(feature = "staking")] + staking: staking::StakingQuerier, + // placeholder to add support later + wasm: NoWasmQuerier, +} + +impl MockQuerier { + #[cfg(not(feature = "staking"))] + pub fn new(balances: &[(&HumanAddr, &[Coin])]) -> Self { + MockQuerier { + bank: BankQuerier::new(balances), + wasm: NoWasmQuerier {}, + } + } + + #[cfg(feature = "staking")] + pub fn new(balances: &[(&HumanAddr, &[Coin])]) -> Self { + MockQuerier { + bank: BankQuerier::new(balances), + staking: staking::StakingQuerier::default(), + wasm: NoWasmQuerier {}, + } + } + + #[cfg(feature = "staking")] + pub fn with_staking( + &mut self, + validators: &[crate::query::Validator], + delegations: &[crate::query::Delegation], + ) { + self.staking = staking::StakingQuerier::new(validators, delegations); + } +} + +impl Querier for MockQuerier { + fn raw_query(&self, bin_request: &[u8]) -> QuerierResult { + // MockQuerier doesn't support Custom, so we ignore it completely here + let request: QueryRequest = match from_slice(bin_request) { + Ok(v) => v, + Err(e) => { + return Err(SystemError::InvalidRequest { + error: format!("Parsing QueryRequest: {}", e), + }) + } + }; + self.handle_query(&request) + } +} + +impl MockQuerier { + pub fn handle_query(&self, request: &QueryRequest) -> QuerierResult { + match &request { + QueryRequest::Bank(bank_query) => self.bank.query(bank_query), + QueryRequest::Custom(_) => Err(SystemError::UnsupportedRequest { + kind: "custom".to_string(), + }), + #[cfg(feature = "staking")] + QueryRequest::Staking(staking_query) => self.staking.query(staking_query), + QueryRequest::Wasm(msg) => self.wasm.query(msg), + } + } +} + +#[derive(Clone, Default)] +struct NoWasmQuerier { + // FIXME: actually provide a way to call out +} + +impl NoWasmQuerier { + fn query(&self, request: &WasmQuery) -> QuerierResult { + let addr = match request { + WasmQuery::Smart { contract_addr, .. } => contract_addr, + WasmQuery::Raw { contract_addr, .. } => contract_addr, + } + .clone(); + Err(SystemError::NoSuchContract { addr }) + } +} + +#[derive(Clone, Default)] +struct BankQuerier { + balances: HashMap>, +} + +impl BankQuerier { + fn new(balances: &[(&HumanAddr, &[Coin])]) -> Self { + let mut map = HashMap::new(); + for (addr, coins) in balances.iter() { + map.insert(HumanAddr::from(addr), coins.to_vec()); + } + BankQuerier { balances: map } + } + + fn query(&self, request: &BankQuery) -> QuerierResult { + match request { + BankQuery::Balance { address, denom } => { + // proper error on not found, serialize result on found + let amount = self + .balances + .get(address) + .and_then(|v| v.iter().find(|c| &c.denom == denom).map(|c| c.amount)) + .unwrap_or_default(); + let bank_res = BalanceResponse { + amount: Coin { + amount, + denom: denom.to_string(), + }, + }; + Ok(to_binary(&bank_res)) + } + BankQuery::AllBalances { address } => { + // proper error on not found, serialize result on found + let bank_res = AllBalanceResponse { + amount: self.balances.get(address).cloned().unwrap_or_default(), + }; + Ok(to_binary(&bank_res)) + } + } + } +} + +#[cfg(feature = "staking")] +mod staking { + use crate::query::{ + Delegation, DelegationsResponse, StakingQuery, Validator, ValidatorsResponse, + }; + use crate::serde::to_binary; + use crate::traits::QuerierResult; + + #[derive(Clone, Default)] + pub struct StakingQuerier { + validators: Vec, + delegations: Vec, + } + + impl StakingQuerier { + pub fn new(validators: &[Validator], delegations: &[Delegation]) -> Self { + StakingQuerier { + validators: validators.to_vec(), + delegations: delegations.to_vec(), + } + } + + pub fn query(&self, request: &StakingQuery) -> QuerierResult { + match request { + StakingQuery::Validators {} => { + let val_res = ValidatorsResponse { + validators: self.validators.clone(), + }; + Ok(to_binary(&val_res)) + } + StakingQuery::Delegations { + delegator, + validator, + } => { + let matches = |d: &&Delegation| { + if let Some(val) = validator { + if val != &d.validator { + return false; + } + } + &d.delegator == delegator + }; + let delegations: Vec<_> = + self.delegations.iter().filter(matches).cloned().collect(); + let val_res = DelegationsResponse { delegations }; + Ok(to_binary(&val_res)) + } + } + } + } + + #[cfg(test)] + mod test { + use super::*; + + use crate::{coin, from_binary, HumanAddr}; + + #[test] + fn staking_querier_validators() { + let val1 = Validator { + address: HumanAddr::from("validator-one"), + commission: 1000, + max_commission: 3000, + max_change_rate: 1000, + }; + let val2 = Validator { + address: HumanAddr::from("validator-two"), + commission: 1500, + max_commission: 4000, + max_change_rate: 500, + }; + + let staking = StakingQuerier::new(&[val1.clone(), val2.clone()], &[]); + + // one match + let raw = staking + .query(&StakingQuery::Validators {}) + .unwrap() + .unwrap(); + let vals: ValidatorsResponse = from_binary(&raw).unwrap(); + assert_eq!(vals.validators, vec![val1, val2]); + } + + // gets delegators from query or panic + fn get_delegators( + staking: &StakingQuerier, + delegator: HumanAddr, + validator: Option, + ) -> Vec { + let raw = staking + .query(&StakingQuery::Delegations { + delegator, + validator, + }) + .unwrap() + .unwrap(); + let dels: DelegationsResponse = from_binary(&raw).unwrap(); + dels.delegations + } + + #[test] + fn staking_querier_delegations() { + let val1 = HumanAddr::from("validator-one"); + let val2 = HumanAddr::from("validator-two"); + + let user_a = HumanAddr::from("investor"); + let user_b = HumanAddr::from("speculator"); + let user_c = HumanAddr::from("hodler"); + + // we need multiple validators per delegator, so the queries provide different results + let del1a = Delegation { + delegator: user_a.clone(), + validator: val1.clone(), + amount: coin(100, "stake"), + can_redelegate: true, + accumulated_rewards: coin(5, "stake"), + }; + let del2a = Delegation { + delegator: user_a.clone(), + validator: val2.clone(), + amount: coin(500, "stake"), + can_redelegate: true, + accumulated_rewards: coin(20, "stake"), + }; + + // this is multiple times on the same validator + let del1b = Delegation { + delegator: user_b.clone(), + validator: val1.clone(), + amount: coin(500, "stake"), + can_redelegate: false, + accumulated_rewards: coin(0, "stake"), + }; + let del1bb = Delegation { + delegator: user_b.clone(), + validator: val1.clone(), + amount: coin(700, "stake"), + can_redelegate: true, + accumulated_rewards: coin(70, "stake"), + }; + + // and another one on val2 + let del2c = Delegation { + delegator: user_c.clone(), + validator: val2.clone(), + amount: coin(8888, "stake"), + can_redelegate: true, + accumulated_rewards: coin(900, "stake"), + }; + + let staking = StakingQuerier::new( + &[], + &[ + del1a.clone(), + del1b.clone(), + del1bb.clone(), + del2a.clone(), + del2c.clone(), + ], + ); + + // get all for user a + let dels = get_delegators(&staking, user_a.clone(), None); + assert_eq!(dels, vec![del1a.clone(), del2a.clone()]); + + // get all for user b + let dels = get_delegators(&staking, user_b.clone(), None); + assert_eq!(dels, vec![del1b.clone(), del1bb.clone()]); + + // get all for user c + let dels = get_delegators(&staking, user_c.clone(), None); + assert_eq!(dels, vec![del2c.clone()]); + + // for user with no delegations... + let dels = get_delegators(&staking, HumanAddr::from("no one"), None); + assert_eq!(dels, vec![]); + + // filter a by validator (1 and 1) + let dels = get_delegators(&staking, user_a.clone(), Some(val1.clone())); + assert_eq!(dels, vec![del1a.clone()]); + let dels = get_delegators(&staking, user_a.clone(), Some(val2.clone())); + assert_eq!(dels, vec![del2a.clone()]); + + // filter b by validator (2 and 0) + let dels = get_delegators(&staking, user_b.clone(), Some(val1.clone())); + assert_eq!(dels, vec![del1b.clone(), del1bb.clone()]); + let dels = get_delegators(&staking, user_b.clone(), Some(val2.clone())); + assert_eq!(dels, vec![]); + + // filter c by validator (0 and 1) + let dels = get_delegators(&staking, user_c.clone(), Some(val1.clone())); + assert_eq!(dels, vec![]); + let dels = get_delegators(&staking, user_c.clone(), Some(val2.clone())); + assert_eq!(dels, vec![del2c.clone()]); + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + use crate::{coin, coins, from_binary}; + + #[test] + fn mock_env_arguments() { + let name = HumanAddr("my name".to_string()); + let api = MockApi::new(20); + + // make sure we can generate with &str, &HumanAddr, and HumanAddr + let a = mock_env(&api, "my name", &coins(100, "atom")); + let b = mock_env(&api, &name, &coins(100, "atom")); + let c = mock_env(&api, name, &coins(100, "atom")); + + // and the results are the same + assert_eq!(a, b); + assert_eq!(a, c); + } + + #[test] + fn flip_addresses() { + let api = MockApi::new(20); + let human = HumanAddr("shorty".to_string()); + let canon = api.canonical_address(&human).unwrap(); + assert_eq!(canon.len(), 20); + assert_eq!(&canon.as_slice()[0..6], human.as_str().as_bytes()); + assert_eq!(&canon.as_slice()[6..], &[0u8; 14]); + + let recovered = api.human_address(&canon).unwrap(); + assert_eq!(human, recovered); + } + + #[test] + #[should_panic(expected = "length not correct")] + fn human_address_input_length() { + let api = MockApi::new(10); + let input = CanonicalAddr(Binary(vec![61; 11])); + api.human_address(&input).unwrap(); + } + + #[test] + #[should_panic(expected = "address too short")] + fn canonical_address_min_input_length() { + let api = MockApi::new(10); + let human = HumanAddr("1".to_string()); + let _ = api.canonical_address(&human).unwrap(); + } + + #[test] + #[should_panic(expected = "address too long")] + fn canonical_address_max_input_length() { + let api = MockApi::new(10); + let human = HumanAddr("longer-than-10".to_string()); + let _ = api.canonical_address(&human).unwrap(); + } + + #[test] + fn bank_querier_all_balances() { + let addr = HumanAddr::from("foobar"); + let balance = vec![coin(123, "ELF"), coin(777, "FLY")]; + let bank = BankQuerier::new(&[(&addr, &balance)]); + + // all + let all = bank + .query(&BankQuery::AllBalances { + address: addr.clone(), + }) + .unwrap() + .unwrap(); + let res: AllBalanceResponse = from_binary(&all).unwrap(); + assert_eq!(&res.amount, &balance); + } + + #[test] + fn bank_querier_one_balance() { + let addr = HumanAddr::from("foobar"); + let balance = vec![coin(123, "ELF"), coin(777, "FLY")]; + let bank = BankQuerier::new(&[(&addr, &balance)]); + + // one match + let fly = bank + .query(&BankQuery::Balance { + address: addr.clone(), + denom: "FLY".to_string(), + }) + .unwrap() + .unwrap(); + let res: BalanceResponse = from_binary(&fly).unwrap(); + assert_eq!(res.amount, coin(777, "FLY")); + + // missing denom + let miss = bank + .query(&BankQuery::Balance { + address: addr.clone(), + denom: "MISS".to_string(), + }) + .unwrap() + .unwrap(); + let res: BalanceResponse = from_binary(&miss).unwrap(); + assert_eq!(res.amount, coin(0, "MISS")); + } + + #[test] + fn bank_querier_missing_account() { + let addr = HumanAddr::from("foobar"); + let balance = vec![coin(123, "ELF"), coin(777, "FLY")]; + let bank = BankQuerier::new(&[(&addr, &balance)]); + + // all balances on empty account is empty vec + let all = bank + .query(&BankQuery::AllBalances { + address: HumanAddr::from("elsewhere"), + }) + .unwrap() + .unwrap(); + let res: AllBalanceResponse = from_binary(&all).unwrap(); + assert_eq!(res.amount, vec![]); + + // any denom on balances on empty account is empty coin + let miss = bank + .query(&BankQuery::Balance { + address: HumanAddr::from("elsewhere"), + denom: "ELF".to_string(), + }) + .unwrap() + .unwrap(); + let res: BalanceResponse = from_binary(&miss).unwrap(); + assert_eq!(res.amount, coin(0, "ELF")); + } +} diff --git a/packages/vm/src/mock/storage.rs b/packages/vm/src/mock/storage.rs new file mode 100644 index 0000000000..fe33eebea2 --- /dev/null +++ b/packages/vm/src/mock/storage.rs @@ -0,0 +1,273 @@ +use std::collections::BTreeMap; +#[cfg(feature = "iterator")] +use std::iter; +#[cfg(feature = "iterator")] +use std::ops::{Bound, RangeBounds}; + +use crate::errors::StdResult; +#[cfg(feature = "iterator")] +use crate::iterator::{Order, KV}; +use crate::traits::{ReadonlyStorage, Storage}; + +#[derive(Default)] +pub struct MemoryStorage { + data: BTreeMap, Vec>, +} + +impl MemoryStorage { + pub fn new() -> Self { + MemoryStorage::default() + } +} + +impl ReadonlyStorage for MemoryStorage { + fn get(&self, key: &[u8]) -> StdResult>> { + Ok(self.data.get(key).cloned()) + } + + #[cfg(feature = "iterator")] + /// range allows iteration over a set of keys, either forwards or backwards + /// uses standard rust range notation, and eg db.range(b"foo"..b"bar") also works reverse + fn range<'a>( + &'a self, + start: Option<&[u8]>, + end: Option<&[u8]>, + order: Order, + ) -> StdResult> + 'a>> { + let bounds = range_bounds(start, end); + + // BTreeMap.range panics if range is start > end. + // However, this cases represent just empty range and we treat it as such. + match (bounds.start_bound(), bounds.end_bound()) { + (Bound::Included(start), Bound::Excluded(end)) if start > end => { + return Ok(Box::new(iter::empty())); + } + _ => {} + } + + let iter = self.data.range(bounds); + Ok(match order { + Order::Ascending => Box::new(iter.map(clone_item).map(StdResult::Ok)), + Order::Descending => Box::new(iter.rev().map(clone_item).map(StdResult::Ok)), + }) + } +} + +#[cfg(feature = "iterator")] +fn range_bounds(start: Option<&[u8]>, end: Option<&[u8]>) -> impl RangeBounds> { + ( + start.map_or(Bound::Unbounded, |x| Bound::Included(x.to_vec())), + end.map_or(Bound::Unbounded, |x| Bound::Excluded(x.to_vec())), + ) +} + +#[cfg(feature = "iterator")] +/// The BTreeMap specific key-value pair reference type, as returned by BTreeMap, T>::range. +/// This is internal as it can change any time if the map implementation is swapped out. +type BTreeMapPairRef<'a, T = Vec> = (&'a Vec, &'a T); + +#[cfg(feature = "iterator")] +fn clone_item(item_ref: BTreeMapPairRef) -> KV { + let (key, value) = item_ref; + (key.clone(), value.clone()) +} + +impl Storage for MemoryStorage { + fn set(&mut self, key: &[u8], value: &[u8]) -> StdResult<()> { + self.data.insert(key.to_vec(), value.to_vec()); + Ok(()) + } + + fn remove(&mut self, key: &[u8]) -> StdResult<()> { + self.data.remove(key); + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[cfg(feature = "iterator")] + // iterator_test_suite takes a storage, adds data and runs iterator tests + // the storage must previously have exactly one key: "foo" = "bar" + // (this allows us to test StorageTransaction and other wrapped storage better) + fn iterator_test_suite(store: &mut S) { + // ensure we had previously set "foo" = "bar" + assert_eq!(store.get(b"foo").unwrap(), Some(b"bar".to_vec())); + assert_eq!( + store.range(None, None, Order::Ascending).unwrap().count(), + 1 + ); + + // setup - add some data, and delete part of it as well + store.set(b"ant", b"hill").expect("error setting value"); + store.set(b"ze", b"bra").expect("error setting value"); + + // noise that should be ignored + store.set(b"bye", b"bye").expect("error setting value"); + store.remove(b"bye").expect("error removing key"); + + // unbounded + { + let iter = store.range(None, None, Order::Ascending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"ant".to_vec(), b"hill".to_vec()), + (b"foo".to_vec(), b"bar".to_vec()), + (b"ze".to_vec(), b"bra".to_vec()), + ] + ); + } + + // unbounded (descending) + { + let iter = store.range(None, None, Order::Descending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"ze".to_vec(), b"bra".to_vec()), + (b"foo".to_vec(), b"bar".to_vec()), + (b"ant".to_vec(), b"hill".to_vec()), + ] + ); + } + + // bounded + { + let iter = store + .range(Some(b"f"), Some(b"n"), Order::Ascending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![(b"foo".to_vec(), b"bar".to_vec())]); + } + + // bounded (descending) + { + let iter = store + .range(Some(b"air"), Some(b"loop"), Order::Descending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"foo".to_vec(), b"bar".to_vec()), + (b"ant".to_vec(), b"hill".to_vec()), + ] + ); + } + + // bounded empty [a, a) + { + let iter = store + .range(Some(b"foo"), Some(b"foo"), Order::Ascending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![]); + } + + // bounded empty [a, a) (descending) + { + let iter = store + .range(Some(b"foo"), Some(b"foo"), Order::Descending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![]); + } + + // bounded empty [a, b) with b < a + { + let iter = store + .range(Some(b"z"), Some(b"a"), Order::Ascending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![]); + } + + // bounded empty [a, b) with b < a (descending) + { + let iter = store + .range(Some(b"z"), Some(b"a"), Order::Descending) + .unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![]); + } + + // right unbounded + { + let iter = store.range(Some(b"f"), None, Order::Ascending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"foo".to_vec(), b"bar".to_vec()), + (b"ze".to_vec(), b"bra".to_vec()), + ] + ); + } + + // right unbounded (descending) + { + let iter = store.range(Some(b"f"), None, Order::Descending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"ze".to_vec(), b"bra".to_vec()), + (b"foo".to_vec(), b"bar".to_vec()), + ] + ); + } + + // left unbounded + { + let iter = store.range(None, Some(b"f"), Order::Ascending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!(elements, vec![(b"ant".to_vec(), b"hill".to_vec()),]); + } + + // left unbounded (descending) + { + let iter = store.range(None, Some(b"no"), Order::Descending).unwrap(); + let elements: Vec = iter.filter_map(StdResult::ok).collect(); + assert_eq!( + elements, + vec![ + (b"foo".to_vec(), b"bar".to_vec()), + (b"ant".to_vec(), b"hill".to_vec()), + ] + ); + } + } + + #[test] + fn get_and_set() { + let mut store = MemoryStorage::new(); + assert_eq!(None, store.get(b"foo").unwrap()); + store.set(b"foo", b"bar").unwrap(); + assert_eq!(Some(b"bar".to_vec()), store.get(b"foo").unwrap()); + assert_eq!(None, store.get(b"food").unwrap()); + } + + #[test] + fn delete() { + let mut store = MemoryStorage::new(); + store.set(b"foo", b"bar").unwrap(); + store.set(b"food", b"bank").unwrap(); + store.remove(b"foo").unwrap(); + + assert_eq!(None, store.get(b"foo").unwrap()); + assert_eq!(Some(b"bank".to_vec()), store.get(b"food").unwrap()); + } + + #[test] + #[cfg(feature = "iterator")] + fn iterator() { + let mut store = MemoryStorage::new(); + store.set(b"foo", b"bar").expect("error setting value"); + iterator_test_suite(&mut store); + } +} From 44ca77e28c675937986dd3469e0e836217a7ad00 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Wed, 13 May 2020 20:07:17 +0300 Subject: [PATCH 02/12] implemented propagation of out-of-gas events from the FFI to the wasmer VM instance --- contracts/hackatom/tests/integration.rs | 15 +- contracts/queue/tests/integration.rs | 8 +- contracts/reflect/src/testing.rs | 4 +- contracts/reflect/tests/integration.rs | 67 +++- packages/std/src/errors/system_error.rs | 16 +- packages/std/src/lib.rs | 4 +- packages/std/src/mock.rs | 4 +- packages/vm/src/cache.rs | 4 +- packages/vm/src/calls.rs | 5 +- packages/vm/src/context.rs | 123 +++---- packages/vm/src/errors.rs | 34 +- packages/vm/src/imports.rs | 441 +++++++++++------------- packages/vm/src/instance.rs | 22 +- packages/vm/src/lib.rs | 5 +- packages/vm/src/mock/mod.rs | 102 +++--- packages/vm/src/mock/storage.rs | 48 +-- packages/vm/src/testing.rs | 6 +- packages/vm/src/traits.rs | 90 +++++ 18 files changed, 570 insertions(+), 428 deletions(-) create mode 100644 packages/vm/src/traits.rs diff --git a/contracts/hackatom/tests/integration.rs b/contracts/hackatom/tests/integration.rs index 37bc4d63af..0512550d9c 100644 --- a/contracts/hackatom/tests/integration.rs +++ b/contracts/hackatom/tests/integration.rs @@ -3,7 +3,7 @@ //! Then running `cargo integration-test` will validate we can properly call into that generated Wasm. //! //! You can easily convert unit tests to integration tests. -//! 1. First copy them over verbatum, +//! 1. First copy them over verbatim, //! 2. Then change //! let mut deps = mock_dependencies(20); //! to @@ -17,14 +17,15 @@ //! }); //! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...) -use cosmwasm_std::testing::mock_env; use cosmwasm_std::{ - coins, from_binary, log, AllBalanceResponse, Api, BankMsg, HandleResponse, HandleResult, - HumanAddr, InitResponse, InitResult, ReadonlyStorage, StdError, + coins, from_binary, log, AllBalanceResponse, BankMsg, HandleResponse, HandleResult, HumanAddr, + InitResponse, InitResult, StdError, }; -use cosmwasm_vm::from_slice; -use cosmwasm_vm::testing::{ - handle, init, mock_instance, mock_instance_with_balances, query, test_io, +use cosmwasm_vm::{ + from_slice, + mock::mock_env, + testing::{handle, init, mock_instance, mock_instance_with_balances, query, test_io}, + Api, ReadonlyStorage, }; use hackatom::contract::{HandleMsg, InitMsg, QueryMsg, State, CONFIG_KEY}; diff --git a/contracts/queue/tests/integration.rs b/contracts/queue/tests/integration.rs index 3e59424f3c..c65ddaed72 100644 --- a/contracts/queue/tests/integration.rs +++ b/contracts/queue/tests/integration.rs @@ -17,10 +17,12 @@ //! }); //! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...) -use cosmwasm_std::testing::{mock_env, MockApi, MockQuerier, MockStorage}; use cosmwasm_std::{from_binary, from_slice, Env, HandleResponse, HumanAddr, InitResponse}; -use cosmwasm_vm::testing::{handle, init, mock_instance, query}; -use cosmwasm_vm::Instance; +use cosmwasm_vm::{ + mock::{mock_env, MockApi, MockQuerier, MockStorage}, + testing::{handle, init, mock_instance, query}, + Instance, +}; use queue::contract::{ CountResponse, HandleMsg, InitMsg, Item, QueryMsg, ReducerResponse, SumResponse, diff --git a/contracts/reflect/src/testing.rs b/contracts/reflect/src/testing.rs index 5ae628d738..91258959ed 100644 --- a/contracts/reflect/src/testing.rs +++ b/contracts/reflect/src/testing.rs @@ -22,9 +22,9 @@ impl Querier for CustomQuerier { // parse into our custom query class let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(e) => { + Err(_) => { return Err(SystemError::InvalidRequest { - error: format!("Parsing QueryRequest: {}", e), + msg: bin_request.to_vec(), }) } }; diff --git a/contracts/reflect/tests/integration.rs b/contracts/reflect/tests/integration.rs index 55a2f97c78..092825a242 100644 --- a/contracts/reflect/tests/integration.rs +++ b/contracts/reflect/tests/integration.rs @@ -17,23 +17,78 @@ //! }); //! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...) -use cosmwasm_std::testing::mock_env; use cosmwasm_std::{ - coin, coins, from_binary, Api, BankMsg, Binary, HandleResponse, HandleResult, HumanAddr, + coin, coins, from_binary, BankMsg, Binary, HandleResponse, HandleResult, HumanAddr, InitResponse, StakingMsg, StdError, }; +use cosmwasm_vm::{ + mock::mock_env, + testing::{handle, init, mock_instance, query}, + Api, Instance, +}; -use cosmwasm_vm::testing::{handle, init, mock_instance, query}; -use cosmwasm_vm::Instance; - +use mock::mock_dependencies; use reflect::msg::{CustomMsg, CustomResponse, HandleMsg, InitMsg, OwnerResponse, QueryMsg}; -use reflect::testing::mock_dependencies; // This line will test the output of cargo wasm static WASM: &[u8] = include_bytes!("../target/wasm32-unknown-unknown/release/reflect.wasm"); // You can uncomment this line instead to test productionified build from cosmwasm-opt // static WASM: &[u8] = include_bytes!("../contract.wasm"); +mod mock { + use reflect::msg::{CustomQuery, CustomResponse}; + + use cosmwasm_std::{from_slice, to_binary, Binary, Coin, QueryRequest, StdResult}; + use cosmwasm_vm::{ + mock::{MockApi, MockQuerier, MockStorage}, + Extern, FfiError, Querier, QuerierResult, + }; + + #[derive(Clone)] + pub struct CustomQuerier { + base: MockQuerier, + } + + impl CustomQuerier { + pub fn new(base: MockQuerier) -> Self { + CustomQuerier { base } + } + } + + impl Querier for CustomQuerier { + fn raw_query(&self, bin_request: &[u8]) -> QuerierResult { + // parse into our custom query class + let request: QueryRequest = match from_slice(bin_request) { + Ok(v) => v, + Err(_) => return Err(FfiError::Other), + }; + if let QueryRequest::Custom(custom_query) = &request { + Ok(Ok(execute(&custom_query))) + } else { + self.base.handle_query(&request) + } + } + } + + /// mock_dependencies is a drop-in replacement for cosmwasm_std::testing::mock_dependencies + /// this uses our CustomQuerier. + pub fn mock_dependencies( + canonical_length: usize, + contract_balance: &[Coin], + ) -> Extern { + let base = cosmwasm_vm::mock::mock_dependencies(canonical_length, contract_balance); + base.change_querier(CustomQuerier::new) + } + + fn execute(query: &CustomQuery) -> StdResult { + let msg = match query { + CustomQuery::Ping {} => "pong".to_string(), + CustomQuery::Capital { text } => text.to_uppercase(), + }; + to_binary(&CustomResponse { msg }) + } +} + #[test] fn proper_initialization() { let mut deps = mock_instance(WASM, &[]); diff --git a/packages/std/src/errors/system_error.rs b/packages/std/src/errors/system_error.rs index c8074374cc..5cb1e6d3b9 100644 --- a/packages/std/src/errors/system_error.rs +++ b/packages/std/src/errors/system_error.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use crate::HumanAddr; -/// SystemError is used for errors inside the VM and is API frindly (i.e. serializable). +/// SystemError is used for errors inside the VM and is API friendly (i.e. serializable). /// /// This is used on return values for Querier as a nested result: Result, SystemError> /// The first wrap (SystemError) will trigger if the contract address doesn't exist, @@ -16,9 +16,10 @@ use crate::HumanAddr; #[serde(rename_all = "snake_case")] #[non_exhaustive] pub enum SystemError { - InvalidRequest { error: String }, + InvalidRequest { msg: Vec }, + InvalidResponse { msg: Vec }, NoSuchContract { addr: HumanAddr }, - Unknown {}, + Unknown, UnsupportedRequest { kind: String }, } @@ -27,9 +28,14 @@ impl std::error::Error for SystemError {} impl std::fmt::Display for SystemError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - SystemError::InvalidRequest { error } => write!(f, "Cannot parse request: {}", error), + SystemError::InvalidRequest { msg } => { + write!(f, "Cannot parse request: {}", String::from_utf8_lossy(msg)) + } + SystemError::InvalidResponse { msg } => { + write!(f, "Cannot parse response: {}", String::from_utf8_lossy(msg)) + } SystemError::NoSuchContract { addr } => write!(f, "No such contract: {}", addr), - SystemError::Unknown {} => write!(f, "Unknown system error"), + SystemError::Unknown => write!(f, "Unknown system error"), SystemError::UnsupportedRequest { kind } => write!(f, "Unsupport query type: {}", kind), } } diff --git a/packages/std/src/lib.rs b/packages/std/src/lib.rs index 2dc984a282..3424212557 100644 --- a/packages/std/src/lib.rs +++ b/packages/std/src/lib.rs @@ -31,7 +31,9 @@ pub use crate::query::{ pub use crate::serde::{from_binary, from_slice, to_binary, to_vec}; pub use crate::storage::MemoryStorage; pub use crate::traits::{Api, Extern, Querier, QuerierResult, ReadonlyStorage, Storage}; -pub use crate::types::{CanonicalAddr, Env, HumanAddr, Never}; +pub use crate::types::{ + BlockInfo, CanonicalAddr, ContractInfo, Env, HumanAddr, MessageInfo, Never, +}; #[cfg(feature = "staking")] pub use crate::init_handle::StakingMsg; diff --git a/packages/std/src/mock.rs b/packages/std/src/mock.rs index 80adc55e48..cbbffe8118 100644 --- a/packages/std/src/mock.rs +++ b/packages/std/src/mock.rs @@ -166,9 +166,9 @@ impl Querier for MockQuerier { // MockQuerier doesn't support Custom, so we ignore it completely here let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(e) => { + Err(_) => { return Err(SystemError::InvalidRequest { - error: format!("Parsing QueryRequest: {}", e), + msg: bin_request.to_vec(), }) } }; diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index c03c8bf5c2..ec9b8bcd6b 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use lru::LruCache; use snafu::ResultExt; -use cosmwasm_std::{Api, Extern, Querier, Storage}; +use crate::traits::{Api, Extern, Querier, Storage}; use crate::backends::{backend, compile}; use crate::compatability::check_wasm; @@ -141,7 +141,7 @@ mod test { use super::*; use crate::calls::{call_handle, call_init}; use crate::errors::VmError; - use cosmwasm_std::testing::{mock_dependencies, mock_env, MockApi, MockQuerier, MockStorage}; + use crate::mock::{mock_dependencies, mock_env, MockApi, MockQuerier, MockStorage}; use cosmwasm_std::{coins, Never}; use tempfile::TempDir; diff --git a/packages/vm/src/calls.rs b/packages/vm/src/calls.rs index 5475b85582..93578a09d6 100644 --- a/packages/vm/src/calls.rs +++ b/packages/vm/src/calls.rs @@ -3,13 +3,14 @@ use snafu::ResultExt; use std::fmt; use cosmwasm_std::{ - Api, Env, HandleResponse, HandleResult, InitResponse, InitResult, Querier, QueryResponse, - QueryResult, StdResult, Storage, + Env, HandleResponse, HandleResult, InitResponse, InitResult, QueryResponse, QueryResult, + StdResult, }; use crate::errors::{RuntimeErr, VmResult, WasmerRuntimeErr}; use crate::instance::{Func, Instance}; use crate::serde::{from_slice, to_vec}; +use crate::traits::{Api, Querier, Storage}; use schemars::JsonSchema; static MAX_LENGTH_INIT: usize = 100_000; diff --git a/packages/vm/src/context.rs b/packages/vm/src/context.rs index c4d79baa83..eb23054bad 100644 --- a/packages/vm/src/context.rs +++ b/packages/vm/src/context.rs @@ -4,24 +4,28 @@ use std::collections::HashMap; #[cfg(feature = "iterator")] use std::convert::TryInto; use std::ffi::c_void; +#[cfg(not(feature = "iterator"))] +use std::marker::PhantomData; use wasmer_runtime_core::vm::Ctx; -use cosmwasm_std::{Querier, Storage, SystemError, SystemResult}; #[cfg(feature = "iterator")] -use cosmwasm_std::{StdResult, KV}; +use cosmwasm_std::KV; -#[cfg(feature = "iterator")] -use crate::errors::IteratorDoesNotExist; use crate::errors::{UninitializedContextData, VmResult}; +use crate::traits::{Querier, Storage}; +#[cfg(feature = "iterator")] +use crate::{errors::IteratorDoesNotExist, FfiResult}; /** context data **/ -struct ContextData { +struct ContextData<'a, S: Storage, Q: Querier> { storage: Option, querier: Option, #[cfg(feature = "iterator")] - iterators: HashMap>>>, + iterators: HashMap> + 'a>>, + #[cfg(not(feature = "iterator"))] + iterators: PhantomData<&'a mut ()>, } pub fn setup_context() -> (*mut c_void, fn(*mut c_void)) { @@ -37,6 +41,8 @@ fn create_unmanaged_context_data() -> *mut c_void { querier: None, #[cfg(feature = "iterator")] iterators: HashMap::new(), + #[cfg(not(feature = "iterator"))] + iterators: PhantomData::default(), }; let heap_data = Box::new(data); // move from stack to heap Box::into_raw(heap_data) as *mut c_void // give up ownership @@ -52,7 +58,28 @@ fn destroy_unmanaged_context_data(ptr: *mut c_void) { } /// Get a mutable reference to the context's data. Ownership remains in the Context. -fn get_context_data(ctx: &mut Ctx) -> &mut ContextData { +// NOTE: This is actually not really implemented safely at the moment. I did this as a +// nicer and less-terrible version of the previous solution to the following issue: +// +// +--->> Go pointer +// | +// Ctx -> ContextData --> iterators: Box --+ +// | | +// +-> storage: impl Storage <<------------+ +// | +// +-> querier: impl Querier +// +// -> : Ownership +// ->> : Mutable borrow +// +// As you can see, there's a cyclical reference here... changing this function to return the same lifetime as it +// returns (and adjusting a few other functions to only have one lifetime instead of two) triggers an error +// elsewhere where we try to add iterators to the context. That's not legal according to Rust's rules, and it +// complains that we're trying to borrow ctx mutably twice. This needs a better solution because this function +// probably triggers unsoundness. +fn get_context_data<'a, 'b, S: Storage, Q: Querier>( + ctx: &'a mut Ctx, +) -> &'b mut ContextData<'b, S, Q> { let owned = unsafe { Box::from_raw(ctx.data as *mut ContextData) // obtain ownership }; @@ -75,6 +102,8 @@ pub(crate) fn move_out_of_context( let mut b = get_context_data::(source); // Destroy all existing iterators which are (in contrast to the storage) // not reused between different instances. + // This is also important because the iterators are pointers to Go memory which should not be stored long term + // Paragraphs 5-7: https://golang.org/cmd/cgo/#hdr-Passing_pointers destroy_iterators(&mut b); (b.storage.take(), b.querier.take()) } @@ -91,9 +120,9 @@ pub(crate) fn move_into_context(target: &mut Ctx, storag /// IDs are guaranteed to be in the range [0, 2**31-1], i.e. fit in the non-negative part if type i32. #[cfg(feature = "iterator")] #[must_use = "without the returned iterator ID, the iterator cannot be accessed"] -pub fn add_iterator( +pub fn add_iterator<'a, S: Storage, Q: Querier>( ctx: &mut Ctx, - iter: Box>>, + iter: Box> + 'a>, ) -> u32 { let b = get_context_data::(ctx); let last_id: u32 = b @@ -110,57 +139,52 @@ pub fn add_iterator( new_id } -pub(crate) fn with_storage_from_context(ctx: &mut Ctx, mut func: F) -> VmResult +pub(crate) fn with_storage_from_context<'a, 'b, S, Q, F, T>( + ctx: &'a mut Ctx, + mut func: F, +) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&mut S) -> VmResult, + F: FnMut(&'b mut S) -> VmResult, { let b = get_context_data::(ctx); - let mut storage = b.storage.take(); - let res = match &mut storage { + match b.storage.as_mut() { Some(data) => func(data), None => UninitializedContextData { kind: "storage" }.fail(), - }; - b.storage = storage; - res + } } -pub(crate) fn with_querier_from_context(ctx: &mut Ctx, mut func: F) -> SystemResult +pub(crate) fn with_querier_from_context<'a, 'b, S, Q, F, T>( + ctx: &'a mut Ctx, + mut func: F, +) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&Q) -> SystemResult, + F: FnMut(&'b Q) -> VmResult, { let b = get_context_data::(ctx); - let querier = b.querier.take(); - let res = match &querier { + match b.querier.as_ref() { Some(q) => func(q), - None => Err(SystemError::Unknown {}), - }; - b.querier = querier; - res + None => UninitializedContextData { kind: "storage" }.fail(), + } } #[cfg(feature = "iterator")] -pub(crate) fn with_iterator_from_context( - ctx: &mut Ctx, +pub(crate) fn with_iterator_from_context<'a, 'b, S, Q, F, T>( + ctx: &'a mut Ctx, iterator_id: u32, mut func: F, ) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&mut dyn Iterator>) -> VmResult, + F: FnMut(&'b mut (dyn Iterator>)) -> VmResult, { let b = get_context_data::(ctx); - let iter = b.iterators.remove(&iterator_id); - match iter { - Some(mut data) => { - let res = func(&mut data); - b.iterators.insert(iterator_id, data); - res - } + match b.iterators.get_mut(&iterator_id) { + Some(data) => func(data), None => IteratorDoesNotExist { id: iterator_id }.fail(), } } @@ -171,10 +195,10 @@ mod test { use crate::backends::compile; #[cfg(feature = "iterator")] use crate::errors::VmError; - use cosmwasm_std::testing::{MockQuerier, MockStorage}; + use crate::mock::{MockQuerier, MockStorage}; + use crate::ReadonlyStorage; use cosmwasm_std::{ - coin, coins, from_binary, to_vec, AllBalanceResponse, BankQuery, HumanAddr, Never, - QueryRequest, ReadonlyStorage, + coins, from_binary, to_vec, AllBalanceResponse, BankQuery, HumanAddr, Never, QueryRequest, }; use wasmer_runtime_core::{imports, instance::Instance, typed_func::Func}; @@ -316,37 +340,16 @@ mod test { let req: QueryRequest = QueryRequest::Bank(BankQuery::AllBalances { address: HumanAddr::from(INIT_ADDR), }); - querier.raw_query(&to_vec(&req).unwrap()) + Ok(querier.raw_query(&to_vec(&req).unwrap())?) }) .unwrap() + .unwrap() .unwrap(); let balance: AllBalanceResponse = from_binary(&res).unwrap(); assert_eq!(balance.amount, coins(INIT_AMOUNT, INIT_DENOM)); } - #[test] - fn with_querier_from_context_parse_works() { - let mut instance = make_instance(); - let ctx = instance.context_mut(); - leave_default_data(ctx); - let contract = HumanAddr::from(INIT_ADDR); - - let balance = with_querier_from_context::(ctx, |querier| { - Ok(querier.query_balance(&contract, INIT_DENOM)) - }) - .unwrap() - .unwrap(); - assert_eq!(balance.amount, coin(INIT_AMOUNT, INIT_DENOM)); - - let balance = with_querier_from_context::(ctx, |querier| { - Ok(querier.query_balance(&contract, "foo")) - }) - .unwrap() - .unwrap(); - assert_eq!(balance.amount, coin(0, "foo")); - } - #[test] #[should_panic(expected = "A panic occurred in the callback.")] fn with_querier_from_context_handles_panics() { diff --git a/packages/vm/src/errors.rs b/packages/vm/src/errors.rs index 5b67f90f10..5e92427c25 100644 --- a/packages/vm/src/errors.rs +++ b/packages/vm/src/errors.rs @@ -91,14 +91,40 @@ pub enum VmError { source: core_error::RuntimeError, backtrace: snafu::Backtrace, }, + #[snafu(display("Calling external function through FFI: {}", source))] + FfiErr { + #[snafu(backtrace)] + source: FfiError, + }, + #[snafu(display("Ran out of gas during contract execution"))] + GasDepletion, } pub type VmResult = core::result::Result; -pub fn make_runtime_err(msg: &'static str) -> VmResult { - RuntimeErr { msg }.fail() -} - pub fn make_validation_err(msg: String) -> VmResult { ValidationErr { msg }.fail() } + +#[derive(PartialEq, Debug, Snafu)] +pub enum FfiError { + #[snafu(display("Panic in FFI call"))] + ForeignPanic, + #[snafu(display("bad argument passed to FFI"))] + BadArgument, + #[snafu(display("Ran out of gas during FFI call"))] + OutOfGas, + #[snafu(display("Unspecified Error during FFI call"))] + Other, +} + +impl From for VmError { + fn from(ffi_error: FfiError) -> Self { + match ffi_error { + FfiError::OutOfGas => VmError::GasDepletion, + _ => VmError::FfiErr { source: ffi_error }, + } + } +} + +pub type FfiResult = core::result::Result; diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs index 2504ff6963..bf86f821d2 100644 --- a/packages/vm/src/imports.rs +++ b/packages/vm/src/imports.rs @@ -2,14 +2,10 @@ #[cfg(feature = "iterator")] use std::convert::TryInto; -#[cfg(feature = "iterator")] -use std::mem; #[cfg(feature = "iterator")] -use cosmwasm_std::StdResult; -use cosmwasm_std::{Api, Binary, CanonicalAddr, HumanAddr, Querier, QuerierResult, Storage}; -#[cfg(feature = "iterator")] -use cosmwasm_std::{Order, KV}; +use cosmwasm_std::Order; +use cosmwasm_std::{Binary, CanonicalAddr, HumanAddr}; use wasmer_runtime_core::vm::Ctx; #[cfg(feature = "iterator")] @@ -17,11 +13,12 @@ use crate::context::{add_iterator, with_iterator_from_context}; use crate::context::{with_querier_from_context, with_storage_from_context}; #[cfg(feature = "iterator")] use crate::conversion::to_i32; -use crate::errors::{make_runtime_err, VmError}; +use crate::errors::{VmError, VmResult}; #[cfg(feature = "iterator")] use crate::memory::maybe_read_region; use crate::memory::{read_region, write_region}; use crate::serde::to_vec; +use crate::traits::{Api, Querier, Storage}; /// A kibi (kilo binary) static KI: usize = 1024; @@ -37,6 +34,7 @@ static MAX_LENGTH_CANONICAL_ADDRESS: usize = 32; static MAX_LENGTH_HUMAN_ADDRESS: usize = 90; static MAX_LENGTH_QUERY_CHAIN_REQUEST: usize = 64 * KI; +// TODO convert these numbers to a single enum. mod errors { /// Success pub static NONE: i32 = 0; @@ -53,11 +51,6 @@ mod errors { // unused block (-1_000_3xx) // unused block (-1_000_4xx) - /// Generic error - using context with no Storage attached - pub static NO_CONTEXT_DATA: i32 = -1_000_500; - /// Generic error - An unknown error accessing the DB - pub static DB_UNKNOWN: i32 = -1_000_501; - /// db_read errors (-1_001_0xx) pub mod read { // pub static UNKNOWN: i32 = -1_001_000; @@ -70,22 +63,16 @@ mod errors { /// canonicalize_address errors (-1_002_0xx) pub mod canonicalize { - /// An unknown error when canonicalizing address - pub static UNKNOWN: i32 = -1_002_000; /// The input address (human address) was invalid pub static INVALID_INPUT: i32 = -1_002_001; } - /// humanize_address errors (-1_002_1xx) - pub mod humanize { - /// An unknonw error when humanizing address - pub static UNKNOWN: i32 = -1_002_100; - } + // /// humanize_address errors (-1_002_1xx) + // pub mod humanize { + // } /// query_chain errors (-1_003_0xx) pub mod query_chain { - /// An unknown error in query_chain - // pub static UNKNOWN: i32 = -1_003_000; /// Cannot serialize query response pub static CANNOT_SERIALIZE_RESPONSE: i32 = -1_003_001; } @@ -95,86 +82,103 @@ mod errors { /// db_scan errors (-2_000_0xx) #[cfg(feature = "iterator")] pub mod scan { - /// An unknown error in the db_scan implementation - pub static UNKNOWN: i32 = -2_000_001; /// Invalid Order enum value passed into scan pub static INVALID_ORDER: i32 = -2_000_002; } - /// db_next errors (-2_000_1xx) - #[cfg(feature = "iterator")] - pub mod next { - /// An unknown error in the db_next implementation - pub static UNKNOWN: i32 = -2_000_101; - /// Iterator with the given ID is not registered - pub static ITERATOR_DOES_NOT_EXIST: i32 = -2_000_102; - } + // /// db_next errors (-2_000_1xx) + // #[cfg(feature = "iterator")] + // pub mod next { + // } } -/// Reads a storage entry from the VM's storage into Wasm memory -pub fn do_read(ctx: &mut Ctx, key_ptr: u32, value_ptr: u32) -> i32 { - let key = match read_region(ctx, key_ptr, MAX_LENGTH_DB_KEY) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, +/// This macro wraps the read_region function for the purposes of the functions below which need to report errors +/// in its operation to the caller in the WASM runtime. +/// On success, the read data is returned from the expression. +/// On failure, an error number is wrapped in `Ok` and returned from the function. +macro_rules! read_region { + ($ctx: expr, $ptr: expr, $length: expr) => { + match read_region($ctx, $ptr, $length) { + Ok(data) => data, + Err(err) => { + return Ok(match err { + VmError::RegionLengthTooBigErr { .. } => errors::REGION_READ_LENGTH_TOO_BIG, + _ => errors::REGION_READ_UNKNOWN, + }) + } + } }; - let value: Option> = match with_storage_from_context::(ctx, |store| { - store - .get(&key) - .or_else(|_| make_runtime_err("Error reading from backend")) - }) { - Ok(v) => v, - Err(VmError::UninitializedContextData { .. }) => return errors::NO_CONTEXT_DATA, - Err(_) => return errors::DB_UNKNOWN, +} + +#[cfg(feature = "iterator")] +/// This macro wraps the maybe_read_region function for the purposes of the functions below which need to report errors +/// in its operation to the caller in the WASM runtime. +/// On success, the optionally read data is returned from the expression. +/// On failure, an error number is wrapped in `Ok` and returned from the function. +macro_rules! maybe_read_region { + ($ctx: expr, $ptr: expr, $length: expr) => { + match maybe_read_region($ctx, $ptr, $length) { + Ok(data) => data, + Err(err) => { + return Ok(match err { + VmError::RegionLengthTooBigErr { .. } => errors::REGION_READ_LENGTH_TOO_BIG, + _ => errors::REGION_READ_UNKNOWN, + }) + } + } }; - match value { - Some(buf) => match write_region(ctx, value_ptr, &buf) { +} + +/// This macro wraps the write_region function for the purposes of the functions below which need to report errors +/// in its operation to the caller in the WASM runtime. +/// On success, `errors::NONE` is returned from the expression. +/// On failure, an error number is wrapped in `Ok` and returned from the function. +macro_rules! write_region { + ($ctx: expr, $ptr: expr, $buffer: expr) => { + match write_region($ctx, $ptr, $buffer) { Ok(()) => errors::NONE, - Err(VmError::RegionTooSmallErr { .. }) => errors::REGION_WRITE_TOO_SMALL, - Err(_) => errors::REGION_WRITE_UNKNOWN, - }, + Err(err) => { + return Ok(match err { + VmError::RegionTooSmallErr { .. } => errors::REGION_WRITE_TOO_SMALL, + _ => errors::REGION_WRITE_UNKNOWN, + }) + } + } + }; +} + +/// Reads a storage entry from the VM's storage into Wasm memory +pub fn do_read( + ctx: &mut Ctx, + key_ptr: u32, + value_ptr: u32, +) -> VmResult { + let key = read_region!(ctx, key_ptr, MAX_LENGTH_DB_KEY); + // `Ok(expr?)` used to convert the error variant. + let value: Option> = + with_storage_from_context::(ctx, |store| Ok(store.get(&key)?))?; + Ok(match value { + Some(buf) => write_region!(ctx, value_ptr, &buf), None => errors::read::KEY_DOES_NOT_EXIST, - } + }) } /// Writes a storage entry from Wasm memory into the VM's storage -pub fn do_write(ctx: &mut Ctx, key_ptr: u32, value_ptr: u32) -> i32 { - let key = match read_region(ctx, key_ptr, MAX_LENGTH_DB_KEY) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; - let value = match read_region(ctx, value_ptr, MAX_LENGTH_DB_VALUE) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; - match with_storage_from_context::(ctx, |store| { - store - .set(&key, &value) - .or_else(|_| make_runtime_err("Error setting database value in backend")) - }) { - Ok(_) => errors::NONE, - Err(VmError::UninitializedContextData { .. }) => errors::NO_CONTEXT_DATA, - Err(_) => errors::DB_UNKNOWN, - } +pub fn do_write( + ctx: &mut Ctx, + key_ptr: u32, + value_ptr: u32, +) -> VmResult { + let key = read_region!(ctx, key_ptr, MAX_LENGTH_DB_KEY); + let value = read_region!(ctx, value_ptr, MAX_LENGTH_DB_VALUE); + with_storage_from_context::(ctx, |store| Ok(store.set(&key, &value)?)) + .and(Ok(errors::NONE)) } -pub fn do_remove(ctx: &mut Ctx, key_ptr: u32) -> i32 { - let key = match read_region(ctx, key_ptr, MAX_LENGTH_DB_KEY) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; - match with_storage_from_context::(ctx, |store| { - store - .remove(&key) - .or_else(|_| make_runtime_err("Error removing database key from backend")) - }) { - Ok(_) => errors::NONE, - Err(VmError::UninitializedContextData { .. }) => errors::NO_CONTEXT_DATA, - Err(_) => errors::DB_UNKNOWN, - } +pub fn do_remove(ctx: &mut Ctx, key_ptr: u32) -> VmResult { + let key = read_region!(ctx, key_ptr, MAX_LENGTH_DB_KEY); + with_storage_from_context::(ctx, |store| Ok(store.remove(&key)?)) + .and(Ok(errors::NONE)) } pub fn do_canonicalize_address( @@ -182,24 +186,14 @@ pub fn do_canonicalize_address( ctx: &mut Ctx, human_ptr: u32, canonical_ptr: u32, -) -> i32 { - let human_data = match read_region(ctx, human_ptr, MAX_LENGTH_HUMAN_ADDRESS) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; +) -> VmResult { + let human_data = read_region!(ctx, human_ptr, MAX_LENGTH_HUMAN_ADDRESS); let human = match String::from_utf8(human_data) { Ok(human_str) => HumanAddr(human_str), - Err(_) => return errors::canonicalize::INVALID_INPUT, + Err(_) => return Ok(errors::canonicalize::INVALID_INPUT), }; - match api.canonical_address(&human) { - Ok(canon) => match write_region(ctx, canonical_ptr, canon.as_slice()) { - Ok(()) => errors::NONE, - Err(VmError::RegionTooSmallErr { .. }) => errors::REGION_WRITE_TOO_SMALL, - Err(_) => errors::REGION_WRITE_UNKNOWN, - }, - Err(_) => errors::canonicalize::UNKNOWN, - } + let canon = api.canonical_address(&human)?; + Ok(write_region!(ctx, canonical_ptr, canon.as_slice())) } pub fn do_humanize_address( @@ -207,44 +201,31 @@ pub fn do_humanize_address( ctx: &mut Ctx, canonical_ptr: u32, human_ptr: u32, -) -> i32 { - let canonical = match read_region(ctx, canonical_ptr, MAX_LENGTH_CANONICAL_ADDRESS) { - Ok(data) => Binary(data), - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; - match api.human_address(&CanonicalAddr(canonical)) { - Ok(human) => match write_region(ctx, human_ptr, human.as_str().as_bytes()) { - Ok(()) => errors::NONE, - Err(VmError::RegionTooSmallErr { .. }) => errors::REGION_WRITE_TOO_SMALL, - Err(_) => errors::REGION_WRITE_UNKNOWN, - }, - Err(_) => errors::humanize::UNKNOWN, - } +) -> VmResult { + let canonical = Binary(read_region!( + ctx, + canonical_ptr, + MAX_LENGTH_CANONICAL_ADDRESS + )); + let human = api.human_address(&CanonicalAddr(canonical))?; + Ok(write_region!(ctx, human_ptr, human.as_str().as_bytes())) } pub fn do_query_chain( ctx: &mut Ctx, request_ptr: u32, response_ptr: u32, -) -> i32 { - let request = match read_region(ctx, request_ptr, MAX_LENGTH_QUERY_CHAIN_REQUEST) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; +) -> VmResult { + let request = read_region!(ctx, request_ptr, MAX_LENGTH_QUERY_CHAIN_REQUEST); - let res: QuerierResult = - with_querier_from_context::(ctx, |querier: &Q| querier.raw_query(&request)); + let res = with_querier_from_context::(ctx, |querier: &Q| { + Ok(querier.raw_query(&request)?) + })?; - match to_vec(&res) { - Ok(serialized) => match write_region(ctx, response_ptr, &serialized) { - Ok(()) => errors::NONE, - Err(VmError::RegionTooSmallErr { .. }) => errors::REGION_WRITE_TOO_SMALL, - Err(_) => errors::REGION_WRITE_UNKNOWN, - }, + Ok(match to_vec(&res) { + Ok(serialized) => write_region!(ctx, response_ptr, &serialized), Err(_) => errors::query_chain::CANNOT_SERIALIZE_RESPONSE, - } + }) } #[cfg(feature = "iterator")] @@ -253,45 +234,19 @@ pub fn do_scan( start_ptr: u32, end_ptr: u32, order: i32, -) -> i32 { - let start = match maybe_read_region(ctx, start_ptr, MAX_LENGTH_DB_KEY) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; - let end = match maybe_read_region(ctx, end_ptr, MAX_LENGTH_DB_KEY) { - Ok(data) => data, - Err(VmError::RegionLengthTooBigErr { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, - Err(_) => return errors::REGION_READ_UNKNOWN, - }; +) -> VmResult { + let start = maybe_read_region!(ctx, start_ptr, MAX_LENGTH_DB_KEY); + let end = maybe_read_region!(ctx, end_ptr, MAX_LENGTH_DB_KEY); let order: Order = match order.try_into() { - Ok(o) => o, - Err(_) => return errors::scan::INVALID_ORDER, + Ok(order) => order, + Err(_) => return Ok(errors::scan::INVALID_ORDER), }; - let range_result = with_storage_from_context::(ctx, |store| { - let iter = match store.range(start.as_deref(), end.as_deref(), order) { - Ok(iter) => iter, - Err(_) => return make_runtime_err("An error occurred in range call"), - }; + let iterator = with_storage_from_context::(ctx, |store| { + Ok(store.range(start.as_deref(), end.as_deref(), order)?) + })?; - // Unsafe: I know the iterator will be deallocated before the storage as I control the lifetime below - // But there is no way for the compiler to know. So... let's just lie to the compiler a little bit. - let live_forever: Box> + 'static> = - unsafe { mem::transmute(iter) }; - Ok(live_forever) - }); - - match range_result { - Ok(iterator) => { - let new_id = add_iterator::(ctx, iterator); - match to_i32(new_id) { - Ok(new_id_signed) => new_id_signed, - Err(_) => errors::scan::UNKNOWN, - } - } - Err(VmError::UninitializedContextData { .. }) => errors::NO_CONTEXT_DATA, - Err(_) => errors::scan::UNKNOWN, - } + let new_id = add_iterator::(ctx, iterator); + to_i32(new_id) } #[cfg(feature = "iterator")] @@ -300,49 +255,36 @@ pub fn do_next( iterator_id: u32, key_ptr: u32, value_ptr: u32, -) -> i32 { - let item = match with_iterator_from_context::(ctx, iterator_id, |iter| { - Ok(iter.next()) - }) { - Ok(i) => i, - Err(VmError::IteratorDoesNotExist { .. }) => return errors::next::ITERATOR_DOES_NOT_EXIST, - Err(VmError::UninitializedContextData { .. }) => return errors::NO_CONTEXT_DATA, - Err(_) => return errors::next::UNKNOWN, - }; +) -> VmResult { + // This always succeeds but `?` is cheaper and more future-proof than `unwrap` :D + let item = with_iterator_from_context::(ctx, iterator_id, |iter| Ok(iter.next()))?; // Prepare return values. Both key and value are Options and will be written if set. - let (key, value) = match item { - Some(Ok(item)) => (Some(item.0), Some(item.1)), - Some(Err(_)) => return errors::next::UNKNOWN, - None => (Some(Vec::::new()), None), // Empty key will later be treated as _no more element_. + let (key, value) = if let Some(result) = item { + let item = result?; + (Some(item.0), Some(item.1)) + } else { + (Some(Vec::::new()), None) // Empty key will later be treated as _no more element_. }; if let Some(key) = key { - match write_region(ctx, key_ptr, &key) { - Ok(()) => (), - Err(VmError::RegionTooSmallErr { .. }) => return errors::REGION_WRITE_TOO_SMALL, - Err(_) => return errors::REGION_WRITE_UNKNOWN, - }; + write_region!(ctx, key_ptr, &key); } if let Some(value) = value { - match write_region(ctx, value_ptr, &value) { - Ok(()) => (), - Err(VmError::RegionTooSmallErr { .. }) => return errors::REGION_WRITE_TOO_SMALL, - Err(_) => return errors::REGION_WRITE_UNKNOWN, - }; + write_region!(ctx, value_ptr, &value); } - errors::NONE + Ok(errors::NONE) } #[cfg(test)] mod test { use super::*; - use cosmwasm_std::testing::{MockApi, MockQuerier, MockStorage}; + use crate::mock::{MockApi, MockQuerier, MockStorage}; use cosmwasm_std::{ coins, from_binary, AllBalanceResponse, BankQuery, HumanAddr, Never, QueryRequest, - ReadonlyStorage, SystemError, WasmQuery, + SystemError, WasmQuery, }; use wasmer_runtime_core::{imports, instance::Instance, typed_func::Func}; @@ -350,6 +292,8 @@ mod test { use crate::context::{move_into_context, setup_context}; #[cfg(feature = "iterator")] use crate::conversion::to_u32; + use crate::traits::ReadonlyStorage; + use crate::FfiError; static CONTRACT: &[u8] = include_bytes!("../testdata/contract.wasm"); @@ -433,7 +377,7 @@ mod test { leave_default_data(ctx); let result = do_read::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, value_ptr), VALUE1); } @@ -448,7 +392,7 @@ mod test { leave_default_data(ctx); let result = do_read::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::read::KEY_DOES_NOT_EXIST); + assert_eq!(result.unwrap(), errors::read::KEY_DOES_NOT_EXIST); assert!(force_read(ctx, value_ptr).is_empty()); } @@ -463,7 +407,7 @@ mod test { leave_default_data(ctx); let result = do_read::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); assert!(force_read(ctx, value_ptr).is_empty()); } @@ -478,7 +422,7 @@ mod test { leave_default_data(ctx); let result = do_read::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_WRITE_TOO_SMALL); + assert_eq!(result.unwrap(), errors::REGION_WRITE_TOO_SMALL); assert!(force_read(ctx, value_ptr).is_empty()); } @@ -493,7 +437,7 @@ mod test { leave_default_data(ctx); let result = do_write::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let val = with_storage_from_context::(ctx, |store| { Ok(store.get(b"new storage key").expect("error getting value")) @@ -513,7 +457,7 @@ mod test { leave_default_data(ctx); let result = do_write::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let val = with_storage_from_context::(ctx, |store| { Ok(store.get(KEY1).expect("error getting value")) @@ -533,7 +477,7 @@ mod test { leave_default_data(ctx); let result = do_write::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let val = with_storage_from_context::(ctx, |store| { Ok(store.get(b"new storage key").expect("error getting value")) @@ -553,7 +497,7 @@ mod test { leave_default_data(ctx); let result = do_write::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); } #[test] @@ -567,7 +511,7 @@ mod test { leave_default_data(ctx); let result = do_write::(ctx, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); } #[test] @@ -581,7 +525,7 @@ mod test { leave_default_data(ctx); let result = do_remove::(ctx, key_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let value = with_storage_from_context::(ctx, |store| { Ok(store.get(existing_key).expect("error getting value")) @@ -602,7 +546,7 @@ mod test { let result = do_remove::(ctx, key_ptr); // Note: right now we cannot differnetiate between an existent and a non-existent key - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let value = with_storage_from_context::(ctx, |store| { Ok(store.get(non_existent_key).expect("error getting value")) @@ -621,7 +565,7 @@ mod test { leave_default_data(ctx); let result = do_remove::(ctx, key_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); } #[test] @@ -636,7 +580,7 @@ mod test { let api = MockApi::new(8); let result = do_canonicalize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, dest_ptr), b"foo\0\0\0\0\0"); } @@ -654,15 +598,25 @@ mod test { let api = MockApi::new(8); let result = do_canonicalize_address(api, ctx, source_ptr1, dest_ptr); - assert_eq!(result, errors::canonicalize::INVALID_INPUT); + assert_eq!(result.unwrap(), errors::canonicalize::INVALID_INPUT); // TODO: would be nice if do_canonicalize_address could differentiate between different errors // from Api.canonical_address and return INVALID_INPUT for those cases as well. let result = do_canonicalize_address(api, ctx, source_ptr2, dest_ptr); - assert_eq!(result, errors::canonicalize::UNKNOWN); + match result.unwrap_err() { + VmError::FfiErr { + source: FfiError::Other, + } => {} + err => panic!("Incorrect error returned: {:?}", err), + }; let result = do_canonicalize_address(api, ctx, source_ptr3, dest_ptr); - assert_eq!(result, errors::canonicalize::UNKNOWN); + match result.unwrap_err() { + VmError::FfiErr { + source: FfiError::Other, + } => {} + err => panic!("Incorrect error returned: {:?}", err), + }; } #[test] @@ -677,7 +631,7 @@ mod test { let api = MockApi::new(8); let result = do_canonicalize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); } #[test] @@ -692,7 +646,7 @@ mod test { let api = MockApi::new(8); let result = do_canonicalize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::REGION_WRITE_TOO_SMALL); + assert_eq!(result.unwrap(), errors::REGION_WRITE_TOO_SMALL); } #[test] @@ -707,7 +661,7 @@ mod test { let api = MockApi::new(8); let result = do_humanize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, dest_ptr), b"foo"); } @@ -723,7 +677,12 @@ mod test { let api = MockApi::new(8); let result = do_humanize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::humanize::UNKNOWN); + match result.unwrap_err() { + VmError::FfiErr { + source: FfiError::Other, + } => {} + err => panic!("Incorrect error returned: {:?}", err), + }; } #[test] @@ -738,7 +697,7 @@ mod test { let api = MockApi::new(8); let result = do_humanize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::REGION_READ_LENGTH_TOO_BIG); + assert_eq!(result.unwrap(), errors::REGION_READ_LENGTH_TOO_BIG); } #[test] @@ -753,7 +712,7 @@ mod test { let api = MockApi::new(8); let result = do_humanize_address(api, ctx, source_ptr, dest_ptr); - assert_eq!(result, errors::REGION_WRITE_TOO_SMALL); + assert_eq!(result.unwrap(), errors::REGION_WRITE_TOO_SMALL); } #[test] @@ -771,10 +730,11 @@ mod test { leave_default_data(ctx); let result = do_query_chain::(ctx, request_ptr, response_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let response = force_read(ctx, response_ptr); - let query_result: QuerierResult = cosmwasm_std::from_slice(&response).unwrap(); + let query_result: cosmwasm_std::QuerierResult = + cosmwasm_std::from_slice(&response).unwrap(); let query_result_inner = query_result.unwrap(); let query_result_inner_inner = query_result_inner.unwrap(); let parsed_again: AllBalanceResponse = from_binary(&query_result_inner_inner).unwrap(); @@ -785,22 +745,22 @@ mod test { fn do_query_chain_fails_for_broken_request() { let mut instance = make_instance(); - let request_ptr = write_data(&mut instance, b"Not valid JSON for sure"); + let request = b"Not valid JSON for sure"; + let request_ptr = write_data(&mut instance, request); let response_ptr = create_empty(&mut instance, 1000); let ctx = instance.context_mut(); leave_default_data(ctx); let result = do_query_chain::(ctx, request_ptr, response_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let response = force_read(ctx, response_ptr); - let query_result: QuerierResult = cosmwasm_std::from_slice(&response).unwrap(); + let query_result: cosmwasm_std::QuerierResult = + cosmwasm_std::from_slice(&response).unwrap(); match query_result { Ok(_) => panic!("This must not succeed"), - Err(SystemError::InvalidRequest { error }) => { - assert!(error.starts_with("Parsing QueryRequest"), error) - } + Err(SystemError::InvalidRequest { msg }) => assert_eq!(msg, request), Err(error) => panic!("Unexpeted error: {:?}", error), } } @@ -821,10 +781,11 @@ mod test { leave_default_data(ctx); let result = do_query_chain::(ctx, request_ptr, response_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); let response = force_read(ctx, response_ptr); - let query_result: QuerierResult = cosmwasm_std::from_slice(&response).unwrap(); + let query_result: cosmwasm_std::QuerierResult = + cosmwasm_std::from_slice(&response).unwrap(); match query_result { Ok(_) => panic!("This must not succeed"), Err(SystemError::NoSuchContract { addr }) => { @@ -842,7 +803,7 @@ mod test { leave_default_data(ctx); // set up iterator over all space - let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into())) + let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); assert_eq!(1, id); @@ -867,7 +828,7 @@ mod test { leave_default_data(ctx); // set up iterator over all space - let id = to_u32(do_scan::(ctx, 0, 0, Order::Descending.into())) + let id = to_u32(do_scan::(ctx, 0, 0, Order::Descending.into()).unwrap()) .expect("ID must not be negative"); assert_eq!(1, id); @@ -895,7 +856,7 @@ mod test { let ctx = instance.context_mut(); leave_default_data(ctx); - let id = to_u32(do_scan::(ctx, start, end, Order::Ascending.into())) + let id = to_u32(do_scan::(ctx, start, end, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); let item = @@ -915,9 +876,9 @@ mod test { leave_default_data(ctx); // unbounded, ascending and descending - let id1 = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into())) + let id1 = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); - let id2 = to_u32(do_scan::(ctx, 0, 0, Order::Descending.into())) + let id2 = to_u32(do_scan::(ctx, 0, 0, Order::Descending.into()).unwrap()) .expect("ID must not be negative"); assert_eq!(id1, 1); assert_eq!(id2, 2); @@ -959,24 +920,24 @@ mod test { let ctx = instance.context_mut(); leave_default_data(ctx); - let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into())) + let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); // Entry 1 let result = do_next::(ctx, id, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, key_ptr), KEY1); assert_eq!(force_read(ctx, value_ptr), VALUE1); // Entry 2 let result = do_next::(ctx, id, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, key_ptr), KEY2); assert_eq!(force_read(ctx, value_ptr), VALUE2); // End let result = do_next::(ctx, id, key_ptr, value_ptr); - assert_eq!(result, errors::NONE); + assert_eq!(result.unwrap(), errors::NONE); assert_eq!(force_read(ctx, key_ptr), b""); // API makes no guarantees for value_ptr in this case } @@ -994,7 +955,11 @@ mod test { let non_existent_id = 42u32; let result = do_next::(ctx, non_existent_id, key_ptr, value_ptr); - assert_eq!(result, errors::next::ITERATOR_DOES_NOT_EXIST); + // assert_eq!(result, errors::next::ITERATOR_DOES_NOT_EXIST); + match result { + Err(VmError::IteratorDoesNotExist { id, .. }) if id == non_existent_id => {} + _ => panic!("Got an unexpected error: {:?}", result), + } } #[test] @@ -1008,11 +973,11 @@ mod test { let ctx = instance.context_mut(); leave_default_data(ctx); - let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into())) + let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); let result = do_next::(ctx, id, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_WRITE_TOO_SMALL); + assert_eq!(result.unwrap(), errors::REGION_WRITE_TOO_SMALL); } #[test] @@ -1026,10 +991,10 @@ mod test { let ctx = instance.context_mut(); leave_default_data(ctx); - let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into())) + let id = to_u32(do_scan::(ctx, 0, 0, Order::Ascending.into()).unwrap()) .expect("ID must not be negative"); let result = do_next::(ctx, id, key_ptr, value_ptr); - assert_eq!(result, errors::REGION_WRITE_TOO_SMALL); + assert_eq!(result.unwrap(), errors::REGION_WRITE_TOO_SMALL); } } diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 9a7c3ffc6b..bced7d4454 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -9,7 +9,7 @@ use wasmer_runtime_core::{ vm::Ctx, }; -use cosmwasm_std::{Api, Extern, Querier, Storage}; +use crate::traits::{Api, Extern, Querier, Storage}; use crate::backends::{compile, get_gas, set_gas}; use crate::context::{ @@ -58,13 +58,13 @@ where // value region too small: -1_000_001 // key does not exist: -1_001_001 // Ownership of both input and output pointer is not transferred to the host. - "db_read" => Func::new(move |ctx: &mut Ctx, key_ptr: u32, value_ptr: u32| -> i32 { + "db_read" => Func::new(move |ctx: &mut Ctx, key_ptr: u32, value_ptr: u32| -> VmResult { do_read::(ctx, key_ptr, value_ptr) }), // Writes the given value into the database entry at the given key. // Ownership of both input and output pointer is not transferred to the host. // Returns 0 on success. Returns negative value on error. - "db_write" => Func::new(move |ctx: &mut Ctx, key_ptr: u32, value_ptr: u32| -> i32 { + "db_write" => Func::new(move |ctx: &mut Ctx, key_ptr: u32, value_ptr: u32| -> VmResult { do_write::(ctx, key_ptr, value_ptr) }), // Removes the value at the given key. Different than writing &[] as future @@ -72,24 +72,24 @@ where // At the moment it is not possible to differentiate between a key that existed before and one that did not exist (https://github.com/CosmWasm/cosmwasm/issues/290). // Ownership of both key pointer is not transferred to the host. // Returns 0 on success. Returns negative value on error. - "db_remove" => Func::new(move |ctx: &mut Ctx, key_ptr: u32| -> i32 { + "db_remove" => Func::new(move |ctx: &mut Ctx, key_ptr: u32| -> VmResult { do_remove::(ctx, key_ptr) }), // Reads human address from human_ptr and writes canonicalized representation to canonical_ptr. // A prepared and sufficiently large memory Region is expected at canonical_ptr that points to pre-allocated memory. // Returns 0 on success. Returns negative value on error. // Ownership of both input and output pointer is not transferred to the host. - "canonicalize_address" => Func::new(move |ctx: &mut Ctx, human_ptr: u32, canonical_ptr: u32| -> i32 { + "canonicalize_address" => Func::new(move |ctx: &mut Ctx, human_ptr: u32, canonical_ptr: u32| -> VmResult { do_canonicalize_address(api, ctx, human_ptr, canonical_ptr) }), // Reads canonical address from canonical_ptr and writes humanized representation to human_ptr. // A prepared and sufficiently large memory Region is expected at human_ptr that points to pre-allocated memory. // Returns 0 on success. Returns negative value on error. // Ownership of both input and output pointer is not transferred to the host. - "humanize_address" => Func::new(move |ctx: &mut Ctx, canonical_ptr: u32, human_ptr: u32| -> i32 { + "humanize_address" => Func::new(move |ctx: &mut Ctx, canonical_ptr: u32, human_ptr: u32| -> VmResult { do_humanize_address(api, ctx, canonical_ptr, human_ptr) }), - "query_chain" => Func::new(move |ctx: &mut Ctx, request_ptr: u32, response_ptr: u32| -> i32 { + "query_chain" => Func::new(move |ctx: &mut Ctx, request_ptr: u32, response_ptr: u32| -> VmResult { do_query_chain::(ctx, request_ptr, response_ptr) }), }, @@ -103,14 +103,14 @@ where // Ownership of both start and end pointer is not transferred to the host. // Returns negative code on error. // Returns iterator ID > 0 on success. - "db_scan" => Func::new(move |ctx: &mut Ctx, start_ptr: u32, end_ptr: u32, order: i32| -> i32 { + "db_scan" => Func::new(move |ctx: &mut Ctx, start_ptr: u32, end_ptr: u32, order: i32| -> VmResult { do_scan::(ctx, start_ptr, end_ptr, order) }), // Get next element of iterator with ID `iterator_id`. - // Expectes Regions in key_ptr and value_ptr, in which the result is written. + // Expects Regions in key_ptr and value_ptr, in which the result is written. // An empty key value (Region of length 0) means no more element. // Ownership of both key and value pointer is not transferred to the host. - "db_next" => Func::new(move |ctx: &mut Ctx, iterator_id: u32, key_ptr: u32, value_ptr: u32| -> i32 { + "db_next" => Func::new(move |ctx: &mut Ctx, iterator_id: u32, key_ptr: u32, value_ptr: u32| -> VmResult { do_next::(ctx, iterator_id, key_ptr, value_ptr) }), }, @@ -364,7 +364,7 @@ mod test { #[cfg(test)] #[cfg(feature = "default-singlepass")] mod singlepass_test { - use cosmwasm_std::testing::mock_env; + use crate::mock::mock_env; use cosmwasm_std::{coins, Never}; use crate::calls::{call_handle, call_init, call_query}; diff --git a/packages/vm/src/lib.rs b/packages/vm/src/lib.rs index f6502f28eb..9177a899f0 100644 --- a/packages/vm/src/lib.rs +++ b/packages/vm/src/lib.rs @@ -9,16 +9,19 @@ mod imports; mod instance; mod memory; mod middleware; +pub mod mock; mod modules; mod serde; pub mod testing; +mod traits; mod wasm_store; pub use crate::cache::CosmCache; pub use crate::calls::{ call_handle, call_handle_raw, call_init, call_init_raw, call_query, call_query_raw, }; -pub use crate::errors::{VmError, VmResult}; +pub use crate::errors::{FfiError, FfiResult, VmError, VmResult}; pub use crate::instance::Instance; pub use crate::modules::FileSystemCache; pub use crate::serde::{from_slice, to_vec}; +pub use crate::traits::{Api, Extern, Querier, QuerierResult, ReadonlyStorage, Storage}; diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs index 80adc55e48..7b393561a4 100644 --- a/packages/vm/src/mock/mod.rs +++ b/packages/vm/src/mock/mod.rs @@ -1,13 +1,15 @@ use std::collections::HashMap; -use crate::coins::Coin; -use crate::encoding::Binary; -use crate::errors::{generic_err, invalid_utf8, StdResult, SystemError}; -use crate::query::{AllBalanceResponse, BalanceResponse, BankQuery, QueryRequest, WasmQuery}; -use crate::serde::{from_slice, to_binary}; -use crate::storage::MemoryStorage; -use crate::traits::{Api, Extern, Querier, QuerierResult}; -use crate::types::{BlockInfo, CanonicalAddr, ContractInfo, Env, HumanAddr, MessageInfo, Never}; +use crate::{Api, Extern, FfiError, FfiResult, Querier, QuerierResult}; +use cosmwasm_std::{ + from_slice, to_binary, AllBalanceResponse, BalanceResponse, BankQuery, Binary, BlockInfo, + CanonicalAddr, Coin, ContractInfo, Delegation, Env, HumanAddr, MessageInfo, Never, + QueryRequest, SystemError, Validator, WasmQuery, +}; + +mod storage; + +use storage::MemoryStorage; static CONTRACT_ADDR: &str = "cosmos2contract"; @@ -63,13 +65,13 @@ impl Default for MockApi { } impl Api for MockApi { - fn canonical_address(&self, human: &HumanAddr) -> StdResult { + fn canonical_address(&self, human: &HumanAddr) -> FfiResult { // Dummy input validation. This is more sophisticated for formats like bech32, where format and checksum are validated. if human.len() < 3 { - return Err(generic_err("Invalid input: human address too short")); + return Err(FfiError::Other); } if human.len() > self.canonical_length { - return Err(generic_err("Invalid input: human address too long")); + return Err(FfiError::Other); } let mut out = Vec::from(human.as_str()); @@ -80,11 +82,9 @@ impl Api for MockApi { Ok(CanonicalAddr(Binary(out))) } - fn human_address(&self, canonical: &CanonicalAddr) -> StdResult { + fn human_address(&self, canonical: &CanonicalAddr) -> FfiResult { if canonical.len() != self.canonical_length { - return Err(generic_err( - "Invalid input: canonical address length not correct", - )); + return Err(FfiError::Other); } // remove trailing 0's (TODO: fix this - but fine for first tests) @@ -95,7 +95,7 @@ impl Api for MockApi { .filter(|&x| x != 0) .collect(); // decode UTF-8 bytes into string - let human = String::from_utf8(trimmed).map_err(invalid_utf8)?; + let human = String::from_utf8(trimmed).or(Err(FfiError::Other))?; Ok(HumanAddr(human)) } } @@ -127,22 +127,12 @@ pub fn mock_env>(api: &T, sender: U, sent: &[Coin]) - #[derive(Clone, Default)] pub struct MockQuerier { bank: BankQuerier, - #[cfg(feature = "staking")] staking: staking::StakingQuerier, // placeholder to add support later wasm: NoWasmQuerier, } impl MockQuerier { - #[cfg(not(feature = "staking"))] - pub fn new(balances: &[(&HumanAddr, &[Coin])]) -> Self { - MockQuerier { - bank: BankQuerier::new(balances), - wasm: NoWasmQuerier {}, - } - } - - #[cfg(feature = "staking")] pub fn new(balances: &[(&HumanAddr, &[Coin])]) -> Self { MockQuerier { bank: BankQuerier::new(balances), @@ -151,12 +141,7 @@ impl MockQuerier { } } - #[cfg(feature = "staking")] - pub fn with_staking( - &mut self, - validators: &[crate::query::Validator], - delegations: &[crate::query::Delegation], - ) { + pub fn with_staking(&mut self, validators: &[Validator], delegations: &[Delegation]) { self.staking = staking::StakingQuerier::new(validators, delegations); } } @@ -166,10 +151,10 @@ impl Querier for MockQuerier { // MockQuerier doesn't support Custom, so we ignore it completely here let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(e) => { - return Err(SystemError::InvalidRequest { - error: format!("Parsing QueryRequest: {}", e), - }) + Err(_) => { + return Ok(Err(SystemError::InvalidRequest { + msg: bin_request.to_vec(), + })) } }; self.handle_query(&request) @@ -180,10 +165,9 @@ impl MockQuerier { pub fn handle_query(&self, request: &QueryRequest) -> QuerierResult { match &request { QueryRequest::Bank(bank_query) => self.bank.query(bank_query), - QueryRequest::Custom(_) => Err(SystemError::UnsupportedRequest { + QueryRequest::Custom(_) => Ok(Err(SystemError::UnsupportedRequest { kind: "custom".to_string(), - }), - #[cfg(feature = "staking")] + })), QueryRequest::Staking(staking_query) => self.staking.query(staking_query), QueryRequest::Wasm(msg) => self.wasm.query(msg), } @@ -202,7 +186,7 @@ impl NoWasmQuerier { WasmQuery::Raw { contract_addr, .. } => contract_addr, } .clone(); - Err(SystemError::NoSuchContract { addr }) + Ok(Err(SystemError::NoSuchContract { addr })) } } @@ -235,26 +219,24 @@ impl BankQuerier { denom: denom.to_string(), }, }; - Ok(to_binary(&bank_res)) + Ok(Ok(to_binary(&bank_res))) } BankQuery::AllBalances { address } => { // proper error on not found, serialize result on found let bank_res = AllBalanceResponse { amount: self.balances.get(address).cloned().unwrap_or_default(), }; - Ok(to_binary(&bank_res)) + Ok(Ok(to_binary(&bank_res))) } } } } -#[cfg(feature = "staking")] mod staking { - use crate::query::{ - Delegation, DelegationsResponse, StakingQuery, Validator, ValidatorsResponse, - }; - use crate::serde::to_binary; use crate::traits::QuerierResult; + use cosmwasm_std::{ + to_binary, Delegation, DelegationsResponse, StakingQuery, Validator, ValidatorsResponse, + }; #[derive(Clone, Default)] pub struct StakingQuerier { @@ -276,7 +258,7 @@ mod staking { let val_res = ValidatorsResponse { validators: self.validators.clone(), }; - Ok(to_binary(&val_res)) + Ok(Ok(to_binary(&val_res))) } StakingQuery::Delegations { delegator, @@ -293,7 +275,7 @@ mod staking { let delegations: Vec<_> = self.delegations.iter().filter(matches).cloned().collect(); let val_res = DelegationsResponse { delegations }; - Ok(to_binary(&val_res)) + Ok(Ok(to_binary(&val_res))) } } } @@ -303,7 +285,7 @@ mod staking { mod test { use super::*; - use crate::{coin, from_binary, HumanAddr}; + use cosmwasm_std::{coin, from_binary, HumanAddr}; #[test] fn staking_querier_validators() { @@ -326,6 +308,7 @@ mod staking { let raw = staking .query(&StakingQuery::Validators {}) .unwrap() + .unwrap() .unwrap(); let vals: ValidatorsResponse = from_binary(&raw).unwrap(); assert_eq!(vals.validators, vec![val1, val2]); @@ -343,6 +326,7 @@ mod staking { validator, }) .unwrap() + .unwrap() .unwrap(); let dels: DelegationsResponse = from_binary(&raw).unwrap(); dels.delegations @@ -450,7 +434,7 @@ mod staking { mod test { use super::*; - use crate::{coin, coins, from_binary}; + use cosmwasm_std::{coin, coins, from_binary}; #[test] fn mock_env_arguments() { @@ -481,27 +465,26 @@ mod test { } #[test] - #[should_panic(expected = "length not correct")] + // #[should_panic(expected = "length not correct")] fn human_address_input_length() { let api = MockApi::new(10); let input = CanonicalAddr(Binary(vec![61; 11])); - api.human_address(&input).unwrap(); + // api.human_address(&input).unwrap(); + assert_eq!(api.human_address(&input).unwrap_err(), FfiError::Other); } #[test] - #[should_panic(expected = "address too short")] fn canonical_address_min_input_length() { let api = MockApi::new(10); let human = HumanAddr("1".to_string()); - let _ = api.canonical_address(&human).unwrap(); + assert_eq!(api.canonical_address(&human).unwrap_err(), FfiError::Other); } #[test] - #[should_panic(expected = "address too long")] fn canonical_address_max_input_length() { let api = MockApi::new(10); let human = HumanAddr("longer-than-10".to_string()); - let _ = api.canonical_address(&human).unwrap(); + assert_eq!(api.canonical_address(&human).unwrap_err(), FfiError::Other); } #[test] @@ -516,6 +499,7 @@ mod test { address: addr.clone(), }) .unwrap() + .unwrap() .unwrap(); let res: AllBalanceResponse = from_binary(&all).unwrap(); assert_eq!(&res.amount, &balance); @@ -534,6 +518,7 @@ mod test { denom: "FLY".to_string(), }) .unwrap() + .unwrap() .unwrap(); let res: BalanceResponse = from_binary(&fly).unwrap(); assert_eq!(res.amount, coin(777, "FLY")); @@ -545,6 +530,7 @@ mod test { denom: "MISS".to_string(), }) .unwrap() + .unwrap() .unwrap(); let res: BalanceResponse = from_binary(&miss).unwrap(); assert_eq!(res.amount, coin(0, "MISS")); @@ -562,6 +548,7 @@ mod test { address: HumanAddr::from("elsewhere"), }) .unwrap() + .unwrap() .unwrap(); let res: AllBalanceResponse = from_binary(&all).unwrap(); assert_eq!(res.amount, vec![]); @@ -573,6 +560,7 @@ mod test { denom: "ELF".to_string(), }) .unwrap() + .unwrap() .unwrap(); let res: BalanceResponse = from_binary(&miss).unwrap(); assert_eq!(res.amount, coin(0, "ELF")); diff --git a/packages/vm/src/mock/storage.rs b/packages/vm/src/mock/storage.rs index fe33eebea2..932f54e0a9 100644 --- a/packages/vm/src/mock/storage.rs +++ b/packages/vm/src/mock/storage.rs @@ -1,13 +1,13 @@ use std::collections::BTreeMap; #[cfg(feature = "iterator")] -use std::iter; -#[cfg(feature = "iterator")] -use std::ops::{Bound, RangeBounds}; +use std::{ + iter, + ops::{Bound, RangeBounds}, +}; -use crate::errors::StdResult; +use crate::{FfiResult, ReadonlyStorage, Storage}; #[cfg(feature = "iterator")] -use crate::iterator::{Order, KV}; -use crate::traits::{ReadonlyStorage, Storage}; +use cosmwasm_std::{Order, KV}; #[derive(Default)] pub struct MemoryStorage { @@ -21,7 +21,7 @@ impl MemoryStorage { } impl ReadonlyStorage for MemoryStorage { - fn get(&self, key: &[u8]) -> StdResult>> { + fn get(&self, key: &[u8]) -> FfiResult>> { Ok(self.data.get(key).cloned()) } @@ -33,7 +33,7 @@ impl ReadonlyStorage for MemoryStorage { start: Option<&[u8]>, end: Option<&[u8]>, order: Order, - ) -> StdResult> + 'a>> { + ) -> FfiResult> + 'a>> { let bounds = range_bounds(start, end); // BTreeMap.range panics if range is start > end. @@ -47,8 +47,8 @@ impl ReadonlyStorage for MemoryStorage { let iter = self.data.range(bounds); Ok(match order { - Order::Ascending => Box::new(iter.map(clone_item).map(StdResult::Ok)), - Order::Descending => Box::new(iter.rev().map(clone_item).map(StdResult::Ok)), + Order::Ascending => Box::new(iter.map(clone_item).map(FfiResult::Ok)), + Order::Descending => Box::new(iter.rev().map(clone_item).map(FfiResult::Ok)), }) } } @@ -73,12 +73,12 @@ fn clone_item(item_ref: BTreeMapPairRef) -> KV { } impl Storage for MemoryStorage { - fn set(&mut self, key: &[u8], value: &[u8]) -> StdResult<()> { + fn set(&mut self, key: &[u8], value: &[u8]) -> FfiResult<()> { self.data.insert(key.to_vec(), value.to_vec()); Ok(()) } - fn remove(&mut self, key: &[u8]) -> StdResult<()> { + fn remove(&mut self, key: &[u8]) -> FfiResult<()> { self.data.remove(key); Ok(()) } @@ -111,7 +111,7 @@ mod test { // unbounded { let iter = store.range(None, None, Order::Ascending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ @@ -125,7 +125,7 @@ mod test { // unbounded (descending) { let iter = store.range(None, None, Order::Descending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ @@ -141,7 +141,7 @@ mod test { let iter = store .range(Some(b"f"), Some(b"n"), Order::Ascending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![(b"foo".to_vec(), b"bar".to_vec())]); } @@ -150,7 +150,7 @@ mod test { let iter = store .range(Some(b"air"), Some(b"loop"), Order::Descending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ @@ -165,7 +165,7 @@ mod test { let iter = store .range(Some(b"foo"), Some(b"foo"), Order::Ascending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![]); } @@ -174,7 +174,7 @@ mod test { let iter = store .range(Some(b"foo"), Some(b"foo"), Order::Descending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![]); } @@ -183,7 +183,7 @@ mod test { let iter = store .range(Some(b"z"), Some(b"a"), Order::Ascending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![]); } @@ -192,14 +192,14 @@ mod test { let iter = store .range(Some(b"z"), Some(b"a"), Order::Descending) .unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![]); } // right unbounded { let iter = store.range(Some(b"f"), None, Order::Ascending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ @@ -212,7 +212,7 @@ mod test { // right unbounded (descending) { let iter = store.range(Some(b"f"), None, Order::Descending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ @@ -225,14 +225,14 @@ mod test { // left unbounded { let iter = store.range(None, Some(b"f"), Order::Ascending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!(elements, vec![(b"ant".to_vec(), b"hill".to_vec()),]); } // left unbounded (descending) { let iter = store.range(None, Some(b"no"), Order::Descending).unwrap(); - let elements: Vec = iter.filter_map(StdResult::ok).collect(); + let elements: Vec = iter.filter_map(FfiResult::ok).collect(); assert_eq!( elements, vec![ diff --git a/packages/vm/src/testing.rs b/packages/vm/src/testing.rs index 1bf2726a9b..0024c92664 100644 --- a/packages/vm/src/testing.rs +++ b/packages/vm/src/testing.rs @@ -5,12 +5,12 @@ use schemars::JsonSchema; use serde::{de::DeserializeOwned, Serialize}; use std::fmt; -use cosmwasm_std::testing::{ +use crate::mock::{ mock_dependencies, mock_dependencies_with_balances, MockApi, MockQuerier, MockStorage, }; +use crate::{Api, Querier, Storage}; use cosmwasm_std::{ - to_vec, Api, Coin, Env, HandleResult, HumanAddr, InitResult, Querier, QueryResponse, StdResult, - Storage, + to_vec, Coin, Env, HandleResult, HumanAddr, InitResult, QueryResponse, StdResult, }; use crate::calls::{call_handle, call_init, call_query}; diff --git a/packages/vm/src/traits.rs b/packages/vm/src/traits.rs new file mode 100644 index 0000000000..afb0905c0b --- /dev/null +++ b/packages/vm/src/traits.rs @@ -0,0 +1,90 @@ +use crate::errors::FfiResult; +use cosmwasm_std::{Binary, CanonicalAddr, HumanAddr, StdResult, SystemResult}; +#[cfg(feature = "iterator")] +use cosmwasm_std::{Order, KV}; + +/// Holds all external dependencies of the contract. +/// Designed to allow easy dependency injection at runtime. +/// This cannot be copied or cloned since it would behave differently +/// for mock storages and a bridge storage in the VM. +pub struct Extern { + pub storage: S, + pub api: A, + pub querier: Q, +} + +impl Extern { + /// change_querier is a helper mainly for test code when swapping out the Querier + /// from the auto-generated one from mock_dependencies. This changes the type of + /// Extern so replaces requires some boilerplate. + pub fn change_querier T>(self, transform: F) -> Extern { + Extern { + storage: self.storage, + api: self.api, + querier: transform(self.querier), + } + } +} + +/// ReadonlyStorage is access to the contracts persistent data store +pub trait ReadonlyStorage +where + Self: 'static, +{ + /// Returns Err on error. + /// Returns Ok(None) when key does not exist. + /// Returns Ok(Some(Vec)) when key exists. + /// + /// Note: Support for differentiating between a non-existent key and a key with empty value + /// is not great yet and might not be possible in all backends. But we're trying to get there. + fn get(&self, key: &[u8]) -> FfiResult>>; + + #[cfg(feature = "iterator")] + /// Allows iteration over a set of key/value pairs, either forwards or backwards. + /// + /// The bound `start` is inclusive and `end` is exclusive. + /// + /// If `start` is lexicographically greater than or equal to `end`, an empty range is described, mo matter of the order. + fn range<'a>( + &'a self, + start: Option<&[u8]>, + end: Option<&[u8]>, + order: Order, + ) -> FfiResult> + 'a>>; +} + +// Storage extends ReadonlyStorage to give mutable access +pub trait Storage: ReadonlyStorage { + fn set(&mut self, key: &[u8], value: &[u8]) -> FfiResult<()>; + /// Removes a database entry at `key`. + /// + /// The current interface does not allow to differentiate between a key that existed + /// before and one that didn't exist. See https://github.com/CosmWasm/cosmwasm/issues/290 + fn remove(&mut self, key: &[u8]) -> FfiResult<()>; +} + +/// Api are callbacks to system functions defined outside of the wasm modules. +/// This is a trait to allow Mocks in the test code. +/// +/// Currently it just supports address conversion, we could add eg. crypto functions here. +/// These should all be pure (stateless) functions. If you need state, you probably want +/// to use the Querier. +/// +/// We can use feature flags to opt-in to non-essential methods +/// for backwards compatibility in systems that don't have them all. +pub trait Api: Copy + Clone + Send { + fn canonical_address(&self, human: &HumanAddr) -> FfiResult; + fn human_address(&self, canonical: &CanonicalAddr) -> FfiResult; +} + +/// A short-hand alias for the two-level query result (1. accessing the contract, 2. executing query in the contract) +pub type QuerierResult = FfiResult>>; + +pub trait Querier: Clone + Send + 'static { + /// raw_query is all that must be implemented for the Querier. + /// This allows us to pass through binary queries from one level to another without + /// knowing the custom format, or we can decode it, with the knowledge of the allowed + /// types. People using the querier probably want one of the simpler auto-generated + /// helper methods + fn raw_query(&self, bin_request: &[u8]) -> QuerierResult; +} From 41be7bab4a09ae8ccc7c4e6df5c946aef1b12352 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Sun, 17 May 2020 20:06:12 +0300 Subject: [PATCH 03/12] PR code review fixes --- contracts/reflect/src/testing.rs | 5 +-- contracts/reflect/tests/integration.rs | 7 ++-- packages/std/src/errors/system_error.rs | 26 +++++++++----- packages/std/src/mock.rs | 5 +-- packages/vm/src/context.rs | 18 +++++----- packages/vm/src/errors.rs | 47 ++++++++++++++++++++++--- packages/vm/src/imports.rs | 31 ++++++++-------- packages/vm/src/lib.rs | 5 ++- packages/vm/src/mock/mod.rs | 41 ++++++++++++++------- 9 files changed, 128 insertions(+), 57 deletions(-) diff --git a/contracts/reflect/src/testing.rs b/contracts/reflect/src/testing.rs index 91258959ed..fae2300041 100644 --- a/contracts/reflect/src/testing.rs +++ b/contracts/reflect/src/testing.rs @@ -22,9 +22,10 @@ impl Querier for CustomQuerier { // parse into our custom query class let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(_) => { + Err(e) => { return Err(SystemError::InvalidRequest { - msg: bin_request.to_vec(), + error: format!("Parsing QueryRequest: {}", e), + request: bin_request.to_vec(), }) } }; diff --git a/contracts/reflect/tests/integration.rs b/contracts/reflect/tests/integration.rs index 092825a242..3d04fc5b41 100644 --- a/contracts/reflect/tests/integration.rs +++ b/contracts/reflect/tests/integration.rs @@ -40,8 +40,9 @@ mod mock { use cosmwasm_std::{from_slice, to_binary, Binary, Coin, QueryRequest, StdResult}; use cosmwasm_vm::{ + make_ffi_other, mock::{MockApi, MockQuerier, MockStorage}, - Extern, FfiError, Querier, QuerierResult, + Extern, Querier, QuerierResult, }; #[derive(Clone)] @@ -60,7 +61,9 @@ mod mock { // parse into our custom query class let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(_) => return Err(FfiError::Other), + Err(e) => { + return Err(make_ffi_other(format!("Parsing QueryRequest: {}", e))); + } }; if let QueryRequest::Custom(custom_query) = &request { Ok(Ok(execute(&custom_query))) diff --git a/packages/std/src/errors/system_error.rs b/packages/std/src/errors/system_error.rs index 5cb1e6d3b9..11cad4758a 100644 --- a/packages/std/src/errors/system_error.rs +++ b/packages/std/src/errors/system_error.rs @@ -16,8 +16,8 @@ use crate::HumanAddr; #[serde(rename_all = "snake_case")] #[non_exhaustive] pub enum SystemError { - InvalidRequest { msg: Vec }, - InvalidResponse { msg: Vec }, + InvalidRequest { error: String, request: Vec }, + InvalidResponse { error: String, response: Vec }, NoSuchContract { addr: HumanAddr }, Unknown, UnsupportedRequest { kind: String }, @@ -28,15 +28,23 @@ impl std::error::Error for SystemError {} impl std::fmt::Display for SystemError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - SystemError::InvalidRequest { msg } => { - write!(f, "Cannot parse request: {}", String::from_utf8_lossy(msg)) - } - SystemError::InvalidResponse { msg } => { - write!(f, "Cannot parse response: {}", String::from_utf8_lossy(msg)) - } + SystemError::InvalidRequest { error, request } => write!( + f, + "Cannot parse request: {} in: {}", + error, + String::from_utf8_lossy(request) + ), + SystemError::InvalidResponse { error, response } => write!( + f, + "Cannot parse response: {} in: {}", + error, + String::from_utf8_lossy(response) + ), SystemError::NoSuchContract { addr } => write!(f, "No such contract: {}", addr), SystemError::Unknown => write!(f, "Unknown system error"), - SystemError::UnsupportedRequest { kind } => write!(f, "Unsupport query type: {}", kind), + SystemError::UnsupportedRequest { kind } => { + write!(f, "Unsupported query type: {}", kind) + } } } } diff --git a/packages/std/src/mock.rs b/packages/std/src/mock.rs index cbbffe8118..c8d365f125 100644 --- a/packages/std/src/mock.rs +++ b/packages/std/src/mock.rs @@ -166,9 +166,10 @@ impl Querier for MockQuerier { // MockQuerier doesn't support Custom, so we ignore it completely here let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(_) => { + Err(e) => { return Err(SystemError::InvalidRequest { - msg: bin_request.to_vec(), + error: format!("Parsing QueryRequest: {}", e), + request: bin_request.to_vec(), }) } }; diff --git a/packages/vm/src/context.rs b/packages/vm/src/context.rs index eb23054bad..f5df6bf290 100644 --- a/packages/vm/src/context.rs +++ b/packages/vm/src/context.rs @@ -61,13 +61,13 @@ fn destroy_unmanaged_context_data(ptr: *mut c_void) { // NOTE: This is actually not really implemented safely at the moment. I did this as a // nicer and less-terrible version of the previous solution to the following issue: // -// +--->> Go pointer -// | -// Ctx -> ContextData --> iterators: Box --+ -// | | -// +-> storage: impl Storage <<------------+ -// | -// +-> querier: impl Querier +// +--->> Go pointer +// | +// Ctx ->> ContextData +-> iterators: Box --+ +// | | +// +-> storage: impl Storage <<------------+ +// | +// +-> querier: impl Querier // // -> : Ownership // ->> : Mutable borrow @@ -167,7 +167,7 @@ where let b = get_context_data::(ctx); match b.querier.as_ref() { Some(q) => func(q), - None => UninitializedContextData { kind: "storage" }.fail(), + None => UninitializedContextData { kind: "querier" }.fail(), } } @@ -184,7 +184,7 @@ where { let b = get_context_data::(ctx); match b.iterators.get_mut(&iterator_id) { - Some(data) => func(data), + Some(iterator) => func(iterator), None => IteratorDoesNotExist { id: iterator_id }.fail(), } } diff --git a/packages/vm/src/errors.rs b/packages/vm/src/errors.rs index 5e92427c25..fbc9f0edec 100644 --- a/packages/vm/src/errors.rs +++ b/packages/vm/src/errors.rs @@ -106,16 +106,31 @@ pub fn make_validation_err(msg: String) -> VmResult { ValidationErr { msg }.fail() } -#[derive(PartialEq, Debug, Snafu)] +#[derive(Debug, Snafu)] pub enum FfiError { #[snafu(display("Panic in FFI call"))] - ForeignPanic, + ForeignPanic { backtrace: snafu::Backtrace }, #[snafu(display("bad argument passed to FFI"))] - BadArgument, + BadArgument { backtrace: snafu::Backtrace }, #[snafu(display("Ran out of gas during FFI call"))] OutOfGas, - #[snafu(display("Unspecified Error during FFI call"))] - Other, + #[snafu(display("Error during FFI call: {}", error))] + Other { + error: String, + backtrace: snafu::Backtrace, + }, +} + +impl FfiError { + pub fn set_message(&mut self, message: S) -> &mut Self + where + S: Into, + { + if let FfiError::Other { error, .. } = self { + *error = message.into() + } + self + } } impl From for VmError { @@ -128,3 +143,25 @@ impl From for VmError { } pub type FfiResult = core::result::Result; + +pub fn make_ffi_foreign_panic() -> FfiError { + ForeignPanic {}.build() +} + +pub fn make_ffi_bad_argument() -> FfiError { + BadArgument {}.build() +} + +pub fn make_ffi_out_of_gas() -> FfiError { + FfiError::OutOfGas +} + +pub fn make_ffi_other(error: S) -> FfiError +where + S: Into, +{ + Other { + error: error.into(), + } + .build() +} diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs index bf86f821d2..4742033111 100644 --- a/packages/vm/src/imports.rs +++ b/packages/vm/src/imports.rs @@ -86,10 +86,12 @@ mod errors { pub static INVALID_ORDER: i32 = -2_000_002; } - // /// db_next errors (-2_000_1xx) - // #[cfg(feature = "iterator")] - // pub mod next { - // } + /// db_next errors (-2_000_1xx) + #[cfg(feature = "iterator")] + pub mod next { + /// Iterator with the given ID is not registered + pub static ITERATOR_DOES_NOT_EXIST: i32 = -2_000_102; + } } /// This macro wraps the read_region function for the purposes of the functions below which need to report errors @@ -257,7 +259,12 @@ pub fn do_next( value_ptr: u32, ) -> VmResult { // This always succeeds but `?` is cheaper and more future-proof than `unwrap` :D - let item = with_iterator_from_context::(ctx, iterator_id, |iter| Ok(iter.next()))?; + let result = with_iterator_from_context::(ctx, iterator_id, |iter| Ok(iter.next())); + // This error variant is caused by user input, so we let the user know about it using an explicit error code. + if let Err(VmError::IteratorDoesNotExist { .. }) = result { + return Ok(errors::next::ITERATOR_DOES_NOT_EXIST); + } + let item = result?; // Prepare return values. Both key and value are Options and will be written if set. let (key, value) = if let Some(result) = item { @@ -605,7 +612,7 @@ mod test { let result = do_canonicalize_address(api, ctx, source_ptr2, dest_ptr); match result.unwrap_err() { VmError::FfiErr { - source: FfiError::Other, + source: FfiError::Other { .. }, } => {} err => panic!("Incorrect error returned: {:?}", err), }; @@ -613,7 +620,7 @@ mod test { let result = do_canonicalize_address(api, ctx, source_ptr3, dest_ptr); match result.unwrap_err() { VmError::FfiErr { - source: FfiError::Other, + source: FfiError::Other { .. }, } => {} err => panic!("Incorrect error returned: {:?}", err), }; @@ -679,7 +686,7 @@ mod test { let result = do_humanize_address(api, ctx, source_ptr, dest_ptr); match result.unwrap_err() { VmError::FfiErr { - source: FfiError::Other, + source: FfiError::Other { .. }, } => {} err => panic!("Incorrect error returned: {:?}", err), }; @@ -760,7 +767,7 @@ mod test { cosmwasm_std::from_slice(&response).unwrap(); match query_result { Ok(_) => panic!("This must not succeed"), - Err(SystemError::InvalidRequest { msg }) => assert_eq!(msg, request), + Err(SystemError::InvalidRequest { request: err, .. }) => assert_eq!(err, request), Err(error) => panic!("Unexpeted error: {:?}", error), } } @@ -955,11 +962,7 @@ mod test { let non_existent_id = 42u32; let result = do_next::(ctx, non_existent_id, key_ptr, value_ptr); - // assert_eq!(result, errors::next::ITERATOR_DOES_NOT_EXIST); - match result { - Err(VmError::IteratorDoesNotExist { id, .. }) if id == non_existent_id => {} - _ => panic!("Got an unexpected error: {:?}", result), - } + assert_eq!(result.unwrap(), errors::next::ITERATOR_DOES_NOT_EXIST); } #[test] diff --git a/packages/vm/src/lib.rs b/packages/vm/src/lib.rs index 9177a899f0..b0212fadf2 100644 --- a/packages/vm/src/lib.rs +++ b/packages/vm/src/lib.rs @@ -20,7 +20,10 @@ pub use crate::cache::CosmCache; pub use crate::calls::{ call_handle, call_handle_raw, call_init, call_init_raw, call_query, call_query_raw, }; -pub use crate::errors::{FfiError, FfiResult, VmError, VmResult}; +pub use crate::errors::{ + make_ffi_bad_argument, make_ffi_foreign_panic, make_ffi_other, make_ffi_out_of_gas, FfiError, + FfiResult, VmError, VmResult, +}; pub use crate::instance::Instance; pub use crate::modules::FileSystemCache; pub use crate::serde::{from_slice, to_vec}; diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs index 7b393561a4..917d06bc55 100644 --- a/packages/vm/src/mock/mod.rs +++ b/packages/vm/src/mock/mod.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use crate::{Api, Extern, FfiError, FfiResult, Querier, QuerierResult}; +use crate::{Api, Extern, FfiResult, Querier, QuerierResult}; use cosmwasm_std::{ from_slice, to_binary, AllBalanceResponse, BalanceResponse, BankQuery, Binary, BlockInfo, CanonicalAddr, Coin, ContractInfo, Delegation, Env, HumanAddr, MessageInfo, Never, @@ -68,10 +68,14 @@ impl Api for MockApi { fn canonical_address(&self, human: &HumanAddr) -> FfiResult { // Dummy input validation. This is more sophisticated for formats like bech32, where format and checksum are validated. if human.len() < 3 { - return Err(FfiError::Other); + return Err(crate::make_ffi_other( + "Invalid input: human address too short", + )); } if human.len() > self.canonical_length { - return Err(FfiError::Other); + return Err(crate::make_ffi_other( + "Invalid input: human address too long", + )); } let mut out = Vec::from(human.as_str()); @@ -84,7 +88,9 @@ impl Api for MockApi { fn human_address(&self, canonical: &CanonicalAddr) -> FfiResult { if canonical.len() != self.canonical_length { - return Err(FfiError::Other); + return Err(crate::make_ffi_other( + "Invalid input: canonical address length not correct", + )); } // remove trailing 0's (TODO: fix this - but fine for first tests) @@ -95,7 +101,8 @@ impl Api for MockApi { .filter(|&x| x != 0) .collect(); // decode UTF-8 bytes into string - let human = String::from_utf8(trimmed).or(Err(FfiError::Other))?; + let human = String::from_utf8(trimmed) + .map_err(|_| crate::make_ffi_other("Could not parse human address result as utf-8"))?; Ok(HumanAddr(human)) } } @@ -151,9 +158,10 @@ impl Querier for MockQuerier { // MockQuerier doesn't support Custom, so we ignore it completely here let request: QueryRequest = match from_slice(bin_request) { Ok(v) => v, - Err(_) => { + Err(e) => { return Ok(Err(SystemError::InvalidRequest { - msg: bin_request.to_vec(), + error: format!("Parsing QueryRequest: {}", e), + request: bin_request.to_vec(), })) } }; @@ -433,7 +441,7 @@ mod staking { #[cfg(test)] mod test { use super::*; - + use crate::FfiError; use cosmwasm_std::{coin, coins, from_binary}; #[test] @@ -465,26 +473,33 @@ mod test { } #[test] - // #[should_panic(expected = "length not correct")] fn human_address_input_length() { let api = MockApi::new(10); let input = CanonicalAddr(Binary(vec![61; 11])); - // api.human_address(&input).unwrap(); - assert_eq!(api.human_address(&input).unwrap_err(), FfiError::Other); + match api.human_address(&input).unwrap_err() { + FfiError::Other { .. } => {} + err => panic!("Unexpected error: {}", err), + } } #[test] fn canonical_address_min_input_length() { let api = MockApi::new(10); let human = HumanAddr("1".to_string()); - assert_eq!(api.canonical_address(&human).unwrap_err(), FfiError::Other); + match api.canonical_address(&human).unwrap_err() { + FfiError::Other { .. } => {} + err => panic!("Unexpected error: {}", err), + } } #[test] fn canonical_address_max_input_length() { let api = MockApi::new(10); let human = HumanAddr("longer-than-10".to_string()); - assert_eq!(api.canonical_address(&human).unwrap_err(), FfiError::Other); + match api.canonical_address(&human).unwrap_err() { + FfiError::Other { .. } => {} + err => panic!("Unexpected error: {}", err), + } } #[test] From 95ea5f1ba157f95d31d9707c99382c8ed5fddfa3 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Sun, 17 May 2020 23:19:57 +0300 Subject: [PATCH 04/12] fixed error when using the iterator feature --- packages/vm/src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vm/src/context.rs b/packages/vm/src/context.rs index aaabd665b9..0682b9e1b4 100644 --- a/packages/vm/src/context.rs +++ b/packages/vm/src/context.rs @@ -13,7 +13,7 @@ use wasmer_runtime_core::vm::Ctx; use cosmwasm_std::KV; #[cfg(feature = "iterator")] -use crate::errors::{make_iterator_does_not_exist, FfiResult, IteratorDoesNotExist}; +use crate::errors::{make_iterator_does_not_exist, FfiResult}; use crate::errors::{make_uninitialized_context_data, VmResult}; use crate::traits::{Querier, Storage}; From 87adf872d1a8d096f0ad41ebe7cb67e63d1e39bb Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Sun, 17 May 2020 23:45:23 +0300 Subject: [PATCH 05/12] fixed tests --- packages/vm/src/instance.rs | 2 +- packages/vm/src/mock/mod.rs | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 539bba2d96..7d31608b34 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -225,8 +225,8 @@ where mod test { use super::*; use crate::errors::VmError; + use crate::mock::mock_dependencies; use crate::testing::mock_instance; - use cosmwasm_std::testing::mock_dependencies; use wabt::wat2wasm; static KIB: usize = 1024; diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs index ffffffc98c..db76b139ef 100644 --- a/packages/vm/src/mock/mod.rs +++ b/packages/vm/src/mock/mod.rs @@ -302,24 +302,24 @@ mod staking { mod test { use super::*; - use cosmwasm_std::{coin, from_binary, HumanAddr}; + use cosmwasm_std::{coin, from_binary, Decimal, HumanAddr}; #[test] fn staking_querier_validators() { let val1 = Validator { address: HumanAddr::from("validator-one"), - commission: 1000, - max_commission: 3000, - max_change_rate: 1000, + commission: Decimal::percent(1), + max_commission: Decimal::percent(3), + max_change_rate: Decimal::percent(1), }; let val2 = Validator { address: HumanAddr::from("validator-two"), - commission: 1500, - max_commission: 4000, - max_change_rate: 500, + commission: Decimal::permille(15), + max_commission: Decimal::permille(40), + max_change_rate: Decimal::permille(5), }; - let staking = StakingQuerier::new(&[val1.clone(), val2.clone()], &[]); + let staking = StakingQuerier::new("stake", &[val1.clone(), val2.clone()], &[]); // one match let raw = staking @@ -400,6 +400,7 @@ mod staking { }; let staking = StakingQuerier::new( + "stake", &[], &[ del1a.clone(), From 235994fdecdf4a0641b1d5b9f9fe9102ea1e8e02 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Sun, 17 May 2020 23:52:41 +0300 Subject: [PATCH 06/12] integrated all the changes from #333 --- packages/vm/src/context.rs | 12 ++++++------ packages/vm/src/instance.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/vm/src/context.rs b/packages/vm/src/context.rs index 0682b9e1b4..7caeccad97 100644 --- a/packages/vm/src/context.rs +++ b/packages/vm/src/context.rs @@ -141,12 +141,12 @@ pub fn add_iterator<'a, S: Storage, Q: Querier>( pub(crate) fn with_storage_from_context<'a, 'b, S, Q, F, T>( ctx: &'a mut Ctx, - mut func: F, + func: F, ) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&'b mut S) -> VmResult, + F: FnOnce(&'b mut S) -> VmResult, { let b = get_context_data::(ctx); match b.storage.as_mut() { @@ -157,12 +157,12 @@ where pub(crate) fn with_querier_from_context<'a, 'b, S, Q, F, T>( ctx: &'a mut Ctx, - mut func: F, + func: F, ) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&'b Q) -> VmResult, + F: FnOnce(&'b Q) -> VmResult, { let b = get_context_data::(ctx); match b.querier.as_ref() { @@ -175,12 +175,12 @@ where pub(crate) fn with_iterator_from_context<'a, 'b, S, Q, F, T>( ctx: &'a mut Ctx, iterator_id: u32, - mut func: F, + func: F, ) -> VmResult where S: Storage, Q: Querier, - F: FnMut(&'b mut (dyn Iterator>)) -> VmResult, + F: FnOnce(&'b mut (dyn Iterator>)) -> VmResult, { let b = get_context_data::(ctx); match b.iterators.get_mut(&iterator_id) { diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs index 7d31608b34..f17d495da9 100644 --- a/packages/vm/src/instance.rs +++ b/packages/vm/src/instance.rs @@ -179,7 +179,7 @@ where get_gas(&self.wasmer_instance) } - pub fn with_storage VmResult, T>(&mut self, func: F) -> VmResult { + pub fn with_storage VmResult, T>(&mut self, func: F) -> VmResult { with_storage_from_context::(self.wasmer_instance.context_mut(), func) } From 77f7b7775951cc3bf557b5f75c4c836b5f7a28e1 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 12:02:06 +0300 Subject: [PATCH 07/12] removed commented code fragment from last merge --- packages/vm/src/imports.rs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs index c264fb92b6..21ae5c2ce6 100644 --- a/packages/vm/src/imports.rs +++ b/packages/vm/src/imports.rs @@ -128,23 +128,6 @@ macro_rules! maybe_read_region { }) } } -// ======= -// /// Reads a storage entry from the VM's storage into Wasm memory -// pub fn do_read(ctx: &mut Ctx, key_ptr: u32, value_ptr: u32) -> i32 { -// let key = match read_region(ctx, key_ptr, MAX_LENGTH_DB_KEY) { -// Ok(data) => data, -// Err(VmError::RegionLengthTooBig { .. }) => return errors::REGION_READ_LENGTH_TOO_BIG, -// Err(_) => return errors::REGION_READ_UNKNOWN, -// }; -// let value: Option> = match with_storage_from_context::(ctx, |store| { -// store -// .get(&key) -// .or_else(|_| Err(make_backend_err("Error reading from backend"))) -// }) { -// Ok(v) => v, -// Err(VmError::UninitializedContextData { .. }) => return errors::NO_CONTEXT_DATA, -// Err(_) => return errors::DB_UNKNOWN, -// >>>>>>> origin/master }; } From a9ee6ed15ec666b57ff6faf3684154e553485c99 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 12:12:49 +0300 Subject: [PATCH 08/12] removed VmError::BackendErr as it is now deprecated --- packages/vm/src/errors.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/packages/vm/src/errors.rs b/packages/vm/src/errors.rs index c6bc72bf2b..7d29ea2573 100644 --- a/packages/vm/src/errors.rs +++ b/packages/vm/src/errors.rs @@ -5,12 +5,6 @@ use snafu::Snafu; #[derive(Debug, Snafu)] #[non_exhaustive] pub enum VmError { - /// An error coming from the backend (i.e. the chain) - #[snafu(display("Backend error: {}", msg))] - BackendErr { - msg: String, - backtrace: snafu::Backtrace, - }, #[snafu(display("Cache error: {}", msg))] CacheErr { msg: String, @@ -129,10 +123,6 @@ impl From for VmError { pub type VmResult = core::result::Result; -pub fn make_backend_err>(msg: S) -> VmError { - BackendErr { msg: msg.into() }.build() -} - pub fn make_cache_err>(msg: S) -> VmError { CacheErr { msg: msg.into() }.build() } @@ -275,15 +265,6 @@ where mod test { use super::*; - #[test] - fn make_backend_err_works() { - let err = make_backend_err("something went wrong"); - match err { - VmError::BackendErr { msg, .. } => assert_eq!(msg, "something went wrong"), - _ => panic!("Unexpected error"), - } - } - #[test] fn make_cache_err_works() { let err = make_cache_err("something went wrong"); From 52b8d7f4af248f952a370eac8d4bef3a4ce3ef43 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 12:54:09 +0300 Subject: [PATCH 09/12] changed SystemError fields to contain Binary instead of Vec --- contracts/reflect/src/testing.rs | 2 +- packages/std/src/errors/system_error.rs | 10 +++++----- packages/std/src/mock.rs | 2 +- packages/vm/src/imports.rs | 4 +++- packages/vm/src/mock/mod.rs | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/contracts/reflect/src/testing.rs b/contracts/reflect/src/testing.rs index fae2300041..4ca8b242b7 100644 --- a/contracts/reflect/src/testing.rs +++ b/contracts/reflect/src/testing.rs @@ -25,7 +25,7 @@ impl Querier for CustomQuerier { Err(e) => { return Err(SystemError::InvalidRequest { error: format!("Parsing QueryRequest: {}", e), - request: bin_request.to_vec(), + request: Binary(bin_request.to_vec()), }) } }; diff --git a/packages/std/src/errors/system_error.rs b/packages/std/src/errors/system_error.rs index 11cad4758a..f55094560b 100644 --- a/packages/std/src/errors/system_error.rs +++ b/packages/std/src/errors/system_error.rs @@ -1,7 +1,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use crate::HumanAddr; +use crate::{Binary, HumanAddr}; /// SystemError is used for errors inside the VM and is API friendly (i.e. serializable). /// @@ -16,8 +16,8 @@ use crate::HumanAddr; #[serde(rename_all = "snake_case")] #[non_exhaustive] pub enum SystemError { - InvalidRequest { error: String, request: Vec }, - InvalidResponse { error: String, response: Vec }, + InvalidRequest { error: String, request: Binary }, + InvalidResponse { error: String, response: Binary }, NoSuchContract { addr: HumanAddr }, Unknown, UnsupportedRequest { kind: String }, @@ -32,13 +32,13 @@ impl std::fmt::Display for SystemError { f, "Cannot parse request: {} in: {}", error, - String::from_utf8_lossy(request) + String::from_utf8_lossy(request.as_slice()) ), SystemError::InvalidResponse { error, response } => write!( f, "Cannot parse response: {} in: {}", error, - String::from_utf8_lossy(response) + String::from_utf8_lossy(response.as_slice()) ), SystemError::NoSuchContract { addr } => write!(f, "No such contract: {}", addr), SystemError::Unknown => write!(f, "Unknown system error"), diff --git a/packages/std/src/mock.rs b/packages/std/src/mock.rs index e17363e05f..278a01ef18 100644 --- a/packages/std/src/mock.rs +++ b/packages/std/src/mock.rs @@ -172,7 +172,7 @@ impl Querier for MockQuerier { Err(e) => { return Err(SystemError::InvalidRequest { error: format!("Parsing QueryRequest: {}", e), - request: bin_request.to_vec(), + request: Binary(bin_request.to_vec()), }) } }; diff --git a/packages/vm/src/imports.rs b/packages/vm/src/imports.rs index 21ae5c2ce6..bfa7c861b2 100644 --- a/packages/vm/src/imports.rs +++ b/packages/vm/src/imports.rs @@ -769,7 +769,9 @@ mod test { cosmwasm_std::from_slice(&response).unwrap(); match query_result { Ok(_) => panic!("This must not succeed"), - Err(SystemError::InvalidRequest { request: err, .. }) => assert_eq!(err, request), + Err(SystemError::InvalidRequest { request: err, .. }) => { + assert_eq!(err.as_slice(), request) + } Err(error) => panic!("Unexpeted error: {:?}", error), } } diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs index db76b139ef..d6fcc35006 100644 --- a/packages/vm/src/mock/mod.rs +++ b/packages/vm/src/mock/mod.rs @@ -161,7 +161,7 @@ impl Querier for MockQuerier { Err(e) => { return Ok(Err(SystemError::InvalidRequest { error: format!("Parsing QueryRequest: {}", e), - request: bin_request.to_vec(), + request: Binary(bin_request.to_vec()), })) } }; From bb24665c96e543a6d0f500c43ea9de70a89ac9f4 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 15:51:16 +0300 Subject: [PATCH 10/12] added {} to the definition of FfiError::OutOfGas --- packages/vm/src/errors.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/vm/src/errors.rs b/packages/vm/src/errors.rs index 7d29ea2573..b31576e9e4 100644 --- a/packages/vm/src/errors.rs +++ b/packages/vm/src/errors.rs @@ -208,7 +208,7 @@ pub enum FfiError { #[snafu(display("bad argument passed to FFI"))] BadArgument { backtrace: snafu::Backtrace }, #[snafu(display("Ran out of gas during FFI call"))] - OutOfGas, + OutOfGas {}, #[snafu(display("Error during FFI call: {}", error))] Other { error: String, @@ -231,7 +231,7 @@ impl FfiError { impl From for VmError { fn from(ffi_error: FfiError) -> Self { match ffi_error { - FfiError::OutOfGas => VmError::GasDepletion, + FfiError::OutOfGas {} => VmError::GasDepletion, _ => VmError::FfiErr { source: ffi_error }, } } @@ -248,7 +248,7 @@ pub fn make_ffi_bad_argument() -> FfiError { } pub fn make_ffi_out_of_gas() -> FfiError { - FfiError::OutOfGas + FfiError::OutOfGas {} } pub fn make_ffi_other(error: S) -> FfiError From daf71dc18f18e33e16aaa6b2c8e3be479686b4d9 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 15:54:52 +0300 Subject: [PATCH 11/12] added {} to the definition of SystemError::Unknown --- packages/std/src/errors/system_error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/std/src/errors/system_error.rs b/packages/std/src/errors/system_error.rs index f55094560b..6e317bb582 100644 --- a/packages/std/src/errors/system_error.rs +++ b/packages/std/src/errors/system_error.rs @@ -19,7 +19,7 @@ pub enum SystemError { InvalidRequest { error: String, request: Binary }, InvalidResponse { error: String, response: Binary }, NoSuchContract { addr: HumanAddr }, - Unknown, + Unknown {}, UnsupportedRequest { kind: String }, } @@ -41,7 +41,7 @@ impl std::fmt::Display for SystemError { String::from_utf8_lossy(response.as_slice()) ), SystemError::NoSuchContract { addr } => write!(f, "No such contract: {}", addr), - SystemError::Unknown => write!(f, "Unknown system error"), + SystemError::Unknown {} => write!(f, "Unknown system error"), SystemError::UnsupportedRequest { kind } => { write!(f, "Unsupported query type: {}", kind) } From fe40d41bfce50119801ee498425c1c5a31bb3468 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Mon, 18 May 2020 16:38:17 +0300 Subject: [PATCH 12/12] fixed staking/tests/integration.rs --- contracts/staking/tests/integration.rs | 2 +- packages/vm/src/mock/mod.rs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contracts/staking/tests/integration.rs b/contracts/staking/tests/integration.rs index cd6d538f41..a86975925d 100644 --- a/contracts/staking/tests/integration.rs +++ b/contracts/staking/tests/integration.rs @@ -17,10 +17,10 @@ //! }); //! 4. Anywhere you see query(&deps, ...) you must replace it with query(&mut deps, ...) -use cosmwasm_std::testing::{mock_dependencies, mock_env}; use cosmwasm_std::{ coin, from_binary, Decimal, HumanAddr, InitResponse, StdError, StdResult, Uint128, Validator, }; +use cosmwasm_vm::mock::{mock_dependencies, mock_env}; use cosmwasm_vm::testing::{init, query}; use cosmwasm_vm::Instance; diff --git a/packages/vm/src/mock/mod.rs b/packages/vm/src/mock/mod.rs index d6fcc35006..7b31b74f1a 100644 --- a/packages/vm/src/mock/mod.rs +++ b/packages/vm/src/mock/mod.rs @@ -148,8 +148,13 @@ impl MockQuerier { } } - pub fn with_staking(&mut self, validators: &[Validator], delegations: &[Delegation]) { - self.staking = staking::StakingQuerier::new("stake", validators, delegations); + pub fn with_staking( + &mut self, + denom: &str, + validators: &[Validator], + delegations: &[Delegation], + ) { + self.staking = staking::StakingQuerier::new(denom, validators, delegations); } }