From 4abb87ce1bb693ce75ca18605caa59e3889a61fc Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 17:03:18 +0200 Subject: [PATCH 01/13] Add Pk1Owned for flexible keys, such as u64_key --- packages/storage-plus/Cargo.toml | 2 +- packages/storage-plus/src/keys.rs | 50 ++++++++++++++++++++++++++++--- packages/storage-plus/src/lib.rs | 2 +- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/storage-plus/Cargo.toml b/packages/storage-plus/Cargo.toml index fa982d681..bcad85a6b 100644 --- a/packages/storage-plus/Cargo.toml +++ b/packages/storage-plus/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cw-storage-plus" -version = "0.2.2" +version = "0.2.3" authors = ["Ethan Frey "] edition = "2018" description = "Enhanced/experimental storage engines" diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 03f2d6a4b..b2f1616b8 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -2,13 +2,18 @@ pub trait PrimaryKey<'a> { type Prefix: Prefixer<'a>; /// returns a slice of key steps, which can be optionally combined - fn key(&self) -> Vec<&'a [u8]>; + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b; } impl<'a> PrimaryKey<'a> for &'a [u8] { type Prefix = (); - fn key(&self) -> Vec<&'a [u8]> { + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b, + { // this is simple, we don't add more prefixes vec![self] } @@ -17,7 +22,10 @@ impl<'a> PrimaryKey<'a> for &'a [u8] { impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8]) { type Prefix = &'a [u8]; - fn key(&self) -> Vec<&'a [u8]> { + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b, + { vec![self.0, self.1] } } @@ -25,7 +33,10 @@ impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8]) { impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8], &'a [u8]) { type Prefix = (&'a [u8], &'a [u8]); - fn key(&self) -> Vec<&'a [u8]> { + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b, + { vec![self.0, self.1, self.2] } } @@ -52,3 +63,34 @@ impl<'a> Prefixer<'a> for (&'a [u8], &'a [u8]) { vec![self.0, self.1] } } + +// Add support for an dynamic keys - constructor functions below +pub struct Pk1Owned(pub Vec); + +pub fn u64_key(val: u64) -> Pk1Owned { + Pk1Owned(val.to_be_bytes().into()) +} + +impl<'a> PrimaryKey<'a> for Pk1Owned { + type Prefix = (); + + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b, + { + vec![&self.0] + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn u64key_works() { + let k = u64_key(134); + let path = k.key(); + assert_eq!(1, path.len()); + assert_eq!(134u64.to_be_bytes().to_vec(), path[0].to_vec()); + } +} diff --git a/packages/storage-plus/src/lib.rs b/packages/storage-plus/src/lib.rs index c032464ea..4e8066128 100644 --- a/packages/storage-plus/src/lib.rs +++ b/packages/storage-plus/src/lib.rs @@ -7,7 +7,7 @@ mod path; mod prefix; pub use item::Item; -pub use keys::{Prefixer, PrimaryKey}; +pub use keys::{u64_key, Pk1Owned, Prefixer, PrimaryKey}; pub use map::Map; pub use path::Path; #[cfg(feature = "iterator")] From 96c112e86f69a63ac55353203ad83537ee83a432 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 17:31:25 +0200 Subject: [PATCH 02/13] cw2 uses cw-storage-plus --- Cargo.lock | 4 ++-- packages/cw2/Cargo.toml | 2 +- packages/cw2/src/lib.rs | 21 ++++++++++----------- packages/storage-plus/src/item.rs | 5 +++++ 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3e83f89e..0ce6abdfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -115,7 +115,7 @@ dependencies = [ [[package]] name = "cw-storage-plus" -version = "0.2.2" +version = "0.2.3" dependencies = [ "cosmwasm-std", "schemars", @@ -177,7 +177,7 @@ name = "cw2" version = "0.2.3" dependencies = [ "cosmwasm-std", - "cosmwasm-storage", + "cw-storage-plus", "schemars", "serde", ] diff --git a/packages/cw2/Cargo.toml b/packages/cw2/Cargo.toml index a98d764a0..7b35da2d9 100644 --- a/packages/cw2/Cargo.toml +++ b/packages/cw2/Cargo.toml @@ -11,6 +11,6 @@ documentation = "https://docs.cosmwasm.com" [dependencies] cosmwasm-std = { version = "0.11.0" } -cosmwasm-storage = { version = "0.11.0" } +cw-storage-plus = { path = "../../packages/storage-plus", version = "0.2.3" } schemars = "0.7" serde = { version = "1.0.103", default-features = false, features = ["derive"] } diff --git a/packages/cw2/src/lib.rs b/packages/cw2/src/lib.rs index eadc54477..ccd140bec 100644 --- a/packages/cw2/src/lib.rs +++ b/packages/cw2/src/lib.rs @@ -1,12 +1,10 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use cosmwasm_std::{ - HumanAddr, Querier, QueryRequest, ReadonlyStorage, StdResult, Storage, WasmQuery, -}; -use cosmwasm_storage::{to_length_prefixed, ReadonlySingleton, Singleton}; +use cosmwasm_std::{HumanAddr, Querier, QueryRequest, StdResult, Storage, WasmQuery}; +use cw_storage_plus::Item; -pub const PREFIX_INFO: &[u8] = b"contract_info"; +pub const CONTRACT: Item = Item::new(b"contract_info"); #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub struct ContractVersion { @@ -21,21 +19,22 @@ pub struct ContractVersion { } /// get_contract_version can be use in migrate to read the previous version of this contract -pub fn get_contract_version(storage: &S) -> StdResult { - ReadonlySingleton::new(storage, PREFIX_INFO).load() +pub fn get_contract_version(store: &S) -> StdResult { + CONTRACT.load(store) } /// set_contract_version should be used in init to store the original version, and after a successful /// migrate to update it pub fn set_contract_version, U: Into>( - storage: &mut S, + store: &mut S, name: T, version: U, ) -> StdResult<()> { - Singleton::new(storage, PREFIX_INFO).save(&ContractVersion { + let val = ContractVersion { contract: name.into(), version: version.into(), - }) + }; + CONTRACT.save(store, &val) } /// This will make a raw_query to another contract to determine the current version it @@ -49,7 +48,7 @@ pub fn query_contract_info>( ) -> StdResult { let req = QueryRequest::Wasm(WasmQuery::Raw { contract_addr: contract_addr.into(), - key: to_length_prefixed(PREFIX_INFO).into(), + key: CONTRACT.as_slice().into(), }); querier.query(&req) } diff --git a/packages/storage-plus/src/item.rs b/packages/storage-plus/src/item.rs index 5d55c1285..187cb7c99 100644 --- a/packages/storage-plus/src/item.rs +++ b/packages/storage-plus/src/item.rs @@ -29,6 +29,11 @@ impl<'a, T> Item<'a, T> where T: Serialize + DeserializeOwned, { + // this gets the path of the data to use elsewhere + pub fn as_slice(&self) -> &[u8] { + self.storage_key + } + /// save will serialize the model and store, returns an error on serialization issues pub fn save(&self, store: &mut S, data: &T) -> StdResult<()> { store.set(self.storage_key, &to_vec(data)?); From 6a3f074d219e3a3503ea985eefc39e342138d42b Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 18:12:12 +0200 Subject: [PATCH 03/13] Start porting cw3-fixed --- Cargo.lock | 2 +- contracts/cw3-fixed-multisig/Cargo.toml | 2 +- contracts/cw3-fixed-multisig/src/contract.rs | 68 ++++++++++---------- contracts/cw3-fixed-multisig/src/state.rs | 62 ++++-------------- packages/storage-plus/src/keys.rs | 36 +++++++++-- packages/storage-plus/src/lib.rs | 2 +- 6 files changed, 83 insertions(+), 89 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ce6abdfa..8c48cc4bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -273,7 +273,7 @@ version = "0.2.3" dependencies = [ "cosmwasm-schema", "cosmwasm-std", - "cosmwasm-storage", + "cw-storage-plus", "cw0", "cw2", "cw3", diff --git a/contracts/cw3-fixed-multisig/Cargo.toml b/contracts/cw3-fixed-multisig/Cargo.toml index 5a956448b..81eec7756 100644 --- a/contracts/cw3-fixed-multisig/Cargo.toml +++ b/contracts/cw3-fixed-multisig/Cargo.toml @@ -21,8 +21,8 @@ library = [] cw0 = { path = "../../packages/cw0", version = "0.2.3" } cw2 = { path = "../../packages/cw2", version = "0.2.3" } cw3 = { path = "../../packages/cw3", version = "0.2.3" } +cw-storage-plus = { path = "../../packages/storage-plus", version = "0.2.3", features = ["iterator"] } cosmwasm-std = { version = "0.11.0", features = ["iterator"] } -cosmwasm-storage = { version = "0.11.0", features = ["iterator"] } schemars = "0.7" serde = { version = "1.0.103", default-features = false, features = ["derive"] } thiserror = { version = "1.0.20" } diff --git a/contracts/cw3-fixed-multisig/src/contract.rs b/contracts/cw3-fixed-multisig/src/contract.rs index 2434bf237..81a30d1be 100644 --- a/contracts/cw3-fixed-multisig/src/contract.rs +++ b/contracts/cw3-fixed-multisig/src/contract.rs @@ -15,8 +15,7 @@ use cw3::{ use crate::error::ContractError; use crate::msg::{HandleMsg, InitMsg, QueryMsg}; use crate::state::{ - ballots, ballots_read, config, config_read, next_id, parse_id, proposal, proposal_read, voters, - voters_read, Ballot, Config, Proposal, + next_id, parse_id, Ballot, Config, Proposal, BALLOTS, CONFIG, PROPOSALS, VOTERS, }; // version info for migration info @@ -48,13 +47,12 @@ pub fn init( total_weight, max_voting_period: msg.max_voting_period, }; - config(&mut deps.storage).save(&cfg)?; + CONFIG.save(&mut deps.storage, &cfg)?; // add all voters - let mut bucket = voters(&mut deps.storage); for voter in msg.voters.iter() { let key = deps.api.canonical_address(&voter.addr)?; - bucket.save(key.as_slice(), &voter.weight)?; + VOTERS.save(&mut deps.storage, &key, &voter.weight)?; } Ok(InitResponse::default()) } @@ -90,11 +88,11 @@ pub fn handle_propose( ) -> Result, ContractError> { // only members of the multisig can create a proposal let raw_sender = deps.api.canonical_address(&info.sender)?; - let vote_power = voters_read(&deps.storage) - .may_load(raw_sender.as_slice())? + let vote_power = VOTERS + .may_load(&deps.storage, &raw_sender)? .ok_or_else(|| ContractError::Unauthorized {})?; - let cfg = config_read(&deps.storage).load()?; + let cfg = CONFIG.load(&deps.storage)?; // max expires also used as default let max_expires = cfg.max_voting_period.after(&env.block); @@ -123,14 +121,14 @@ pub fn handle_propose( required_weight: cfg.required_weight, }; let id = next_id(&mut deps.storage)?; - proposal(&mut deps.storage).save(&id.to_be_bytes(), &prop)?; + PROPOSALS.save(&mut deps.storage, id.into(), &prop)?; // add the first yes vote from voter let ballot = Ballot { weight: vote_power, vote: Vote::Yes, }; - ballots(&mut deps.storage, id).save(raw_sender.as_slice(), &ballot)?; + BALLOTS.save(&mut deps.storage, (id.into(), &raw_sender), &ballot)?; Ok(HandleResponse { messages: vec![], @@ -153,12 +151,12 @@ pub fn handle_vote( ) -> Result, ContractError> { // only members of the multisig can vote let raw_sender = deps.api.canonical_address(&info.sender)?; - let vote_power = voters_read(&deps.storage) - .may_load(raw_sender.as_slice())? + let vote_power = VOTERS + .may_load(&deps.storage, &raw_sender)? .ok_or_else(|| ContractError::Unauthorized {})?; // ensure proposal exists and can be voted on - let mut prop = proposal_read(&deps.storage).load(&proposal_id.to_be_bytes())?; + let mut prop = PROPOSALS.load(&deps.storage, proposal_id.into())?; if prop.status != Status::Open { return Err(ContractError::NotOpen {}); } @@ -167,13 +165,17 @@ pub fn handle_vote( } // cast vote if no vote previously cast - ballots(&mut deps.storage, proposal_id).update(raw_sender.as_slice(), |bal| match bal { - Some(_) => Err(ContractError::AlreadyVoted {}), - None => Ok(Ballot { - weight: vote_power, - vote, - }), - })?; + BALLOTS.update( + &mut deps.storage, + (proposal_id.into(), &raw_sender), + |bal| match bal { + Some(_) => Err(ContractError::AlreadyVoted {}), + None => Ok(Ballot { + weight: vote_power, + vote, + }), + }, + )?; // if yes vote, update tally if vote == Vote::Yes { @@ -182,7 +184,7 @@ pub fn handle_vote( if prop.yes_weight >= prop.required_weight { prop.status = Status::Passed; } - proposal(&mut deps.storage).save(&proposal_id.to_be_bytes(), &prop)?; + PROPOSALS.save(&mut deps.storage, proposal_id.into(), &prop)?; } Ok(HandleResponse { @@ -205,7 +207,7 @@ pub fn handle_execute( ) -> Result { // anyone can trigger this if the vote passed - let mut prop = proposal_read(&deps.storage).load(&proposal_id.to_be_bytes())?; + let mut prop = PROPOSALS.load(&deps.storage, proposal_id.into())?; // we allow execution even after the proposal "expiration" as long as all vote come in before // that point. If it was approved on time, it can be executed any time. if prop.status != Status::Passed { @@ -214,7 +216,7 @@ pub fn handle_execute( // set it to executed prop.status = Status::Executed; - proposal(&mut deps.storage).save(&proposal_id.to_be_bytes(), &prop)?; + PROPOSALS.save(&mut deps.storage, proposal_id.into(), &prop)?; // dispatch all proposed messages Ok(HandleResponse { @@ -236,7 +238,7 @@ pub fn handle_close( ) -> Result, ContractError> { // anyone can trigger this if the vote passed - let mut prop = proposal_read(&deps.storage).load(&proposal_id.to_be_bytes())?; + let mut prop = PROPOSALS.load(&deps.storage, proposal_id.into())?; if [Status::Executed, Status::Rejected, Status::Passed] .iter() .any(|x| *x == prop.status) @@ -249,7 +251,7 @@ pub fn handle_close( // set it to failed prop.status = Status::Rejected; - proposal(&mut deps.storage).save(&proposal_id.to_be_bytes(), &prop)?; + PROPOSALS.save(&mut deps.storage, proposal_id.into(), &prop)?; Ok(HandleResponse { messages: vec![], @@ -293,7 +295,7 @@ pub fn query( fn query_threshold( deps: &Extern, ) -> StdResult { - let cfg = config_read(&deps.storage).load()?; + let cfg = CONFIG.load(&deps.storage)?; Ok(ThresholdResponse::AbsoluteCount { weight_needed: cfg.required_weight, total_weight: cfg.total_weight, @@ -305,7 +307,7 @@ fn query_proposal( env: Env, id: u64, ) -> StdResult { - let prop = proposal_read(&deps.storage).load(&id.to_be_bytes())?; + let prop = PROPOSALS.load(&deps.storage, id.into())?; let status = prop.current_status(&env.block); Ok(ProposalResponse { id, @@ -329,9 +331,9 @@ fn list_proposals( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = start_after.map(|id| (id + 1).to_be_bytes().to_vec()); - let props: StdResult> = proposal_read(&deps.storage) - .range(start.as_deref(), None, Order::Ascending) + let start = start_after.map(|id| id.to_be_bytes().to_vec()); + let props: StdResult> = PROPOSALS + .range(&deps.storage, start.as_deref(), None, Order::Ascending) .take(limit) .map(|p| map_proposal(&env.block, p)) .collect(); @@ -378,7 +380,7 @@ fn query_vote( voter: HumanAddr, ) -> StdResult { let voter_raw = deps.api.canonical_address(&voter)?; - let prop = ballots_read(&deps.storage, proposal_id).may_load(voter_raw.as_slice())?; + let prop = BALLOTS.may_load(&deps.storage, (proposal_id.into(), &voter_raw))?; let vote = prop.map(|b| b.vote); Ok(VoteResponse { vote }) } @@ -414,8 +416,8 @@ fn query_voter( voter: HumanAddr, ) -> StdResult { let voter_raw = deps.api.canonical_address(&voter)?; - let weight = voters_read(&deps.storage) - .may_load(voter_raw.as_slice())? + let weight = VOTERS + .may_load(&deps.storage, &voter_raw)? .unwrap_or_default(); Ok(VoterResponse { addr: voter, diff --git a/contracts/cw3-fixed-multisig/src/state.rs b/contracts/cw3-fixed-multisig/src/state.rs index 6d4e948d6..aa484b2e2 100644 --- a/contracts/cw3-fixed-multisig/src/state.rs +++ b/contracts/cw3-fixed-multisig/src/state.rs @@ -2,13 +2,11 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::convert::TryInto; -use cosmwasm_std::{BlockInfo, CosmosMsg, Empty, ReadonlyStorage, StdError, StdResult, Storage}; -use cosmwasm_storage::{ - bucket, bucket_read, singleton, singleton_read, Bucket, ReadonlyBucket, ReadonlySingleton, - Singleton, -}; +use cosmwasm_std::{BlockInfo, CosmosMsg, Empty, StdError, StdResult, Storage}; + use cw0::{Duration, Expiration}; use cw3::{Status, Vote}; +use cw_storage_plus::{Item, Map, U64Key}; #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)] pub struct Config { @@ -55,41 +53,18 @@ pub struct Ballot { pub vote: Vote, } -pub const CONFIG_KEY: &[u8] = b"config"; -pub const PROPOSAL_COUNTER: &[u8] = b"proposal_count"; - -pub const PREFIX_PROPOSAL: &[u8] = b"proposals"; -pub const PREFIX_VOTERS: &[u8] = b"voters"; -pub const PREFIX_VOTES: &[u8] = b"votes"; +// unique items +pub const CONFIG: Item = Item::new(b"config"); +pub const PROPOSAL_COUNT: Item = Item::new(b"proposal_count"); -pub fn config(storage: &mut S) -> Singleton { - singleton(storage, CONFIG_KEY) -} - -pub fn config_read(storage: &S) -> ReadonlySingleton { - singleton_read(storage, CONFIG_KEY) -} - -pub fn voters(storage: &mut S) -> Bucket { - bucket(storage, PREFIX_VOTERS) -} - -pub fn voters_read(storage: &S) -> ReadonlyBucket { - bucket_read(storage, PREFIX_VOTERS) -} +// multiple-item maps +pub const VOTERS: Map<&[u8], u64> = Map::new(b"voters"); +pub const PROPOSALS: Map = Map::new(b"proposals"); +pub const BALLOTS: Map<(U64Key, &[u8]), Ballot> = Map::new(b"votes"); -pub fn proposal(storage: &mut S) -> Bucket { - bucket(storage, PREFIX_PROPOSAL) -} - -pub fn proposal_read(storage: &S) -> ReadonlyBucket { - bucket_read(storage, PREFIX_PROPOSAL) -} - -pub fn next_id(storage: &mut S) -> StdResult { - let mut s = singleton(storage, PROPOSAL_COUNTER); - let id: u64 = s.may_load()?.unwrap_or_default() + 1; - s.save(&id)?; +pub fn next_id(store: &mut S) -> StdResult { + let id: u64 = PROPOSAL_COUNT.may_load(store)?.unwrap_or_default() + 1; + PROPOSAL_COUNT.save(store, &id)?; Ok(id) } @@ -101,14 +76,3 @@ pub fn parse_id(data: &[u8]) -> StdResult { )), } } - -pub fn ballots(storage: &mut S, proposal_id: u64) -> Bucket { - Bucket::multilevel(storage, &[PREFIX_VOTES, &proposal_id.to_be_bytes()]) -} - -pub fn ballots_read( - storage: &S, - proposal_id: u64, -) -> ReadonlyBucket { - ReadonlyBucket::multilevel(storage, &[PREFIX_VOTES, &proposal_id.to_be_bytes()]) -} diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index b2f1616b8..6cab8a97f 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -67,10 +67,6 @@ impl<'a> Prefixer<'a> for (&'a [u8], &'a [u8]) { // Add support for an dynamic keys - constructor functions below pub struct Pk1Owned(pub Vec); -pub fn u64_key(val: u64) -> Pk1Owned { - Pk1Owned(val.to_be_bytes().into()) -} - impl<'a> PrimaryKey<'a> for Pk1Owned { type Prefix = (); @@ -82,6 +78,38 @@ impl<'a> PrimaryKey<'a> for Pk1Owned { } } +impl<'a, T: AsRef> PrimaryKey<'a> for T { + type Prefix = (); + + fn key<'b>(&'b self) -> Vec<&'b [u8]> + where + 'a: 'b, + { + self.as_ref().key() + } +} + +// this reuses Pk1Owned logic with a particular type +pub struct U64Key(pub Pk1Owned); + +impl U64Key { + pub fn new(val: u64) -> Self { + U64Key(Pk1Owned(val.to_be_bytes().to_vec())) + } +} + +impl From for U64Key { + fn from(val: u64) -> Self { + U64Key::new(val) + } +} + +impl AsRef for U64Key { + fn as_ref(&self) -> &Pk1Owned { + &self.0 + } +} + #[cfg(test)] mod test { use super::*; diff --git a/packages/storage-plus/src/lib.rs b/packages/storage-plus/src/lib.rs index 4e8066128..0334c48a6 100644 --- a/packages/storage-plus/src/lib.rs +++ b/packages/storage-plus/src/lib.rs @@ -7,7 +7,7 @@ mod path; mod prefix; pub use item::Item; -pub use keys::{u64_key, Pk1Owned, Prefixer, PrimaryKey}; +pub use keys::{Pk1Owned, Prefixer, PrimaryKey, U64Key}; pub use map::Map; pub use path::Path; #[cfg(feature = "iterator")] From 0133cda301aff966c77177437769de0564530fee Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 19:11:16 +0200 Subject: [PATCH 04/13] Macros to generate multiple IntKey classes --- packages/storage-plus/src/endian.rs | 55 +++++++++++++++++++++++++++++ packages/storage-plus/src/keys.rs | 43 ++++++++++++++++------ packages/storage-plus/src/lib.rs | 4 ++- 3 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 packages/storage-plus/src/endian.rs diff --git a/packages/storage-plus/src/endian.rs b/packages/storage-plus/src/endian.rs new file mode 100644 index 000000000..0990ecee0 --- /dev/null +++ b/packages/storage-plus/src/endian.rs @@ -0,0 +1,55 @@ +//! This code is inspired by (and partially borrowed from) +//! https://docs.rs/endiannezz/0.5.2/endiannezz/trait.Primitive.html +//! but there was a lot in that crate I did not want, the name did not inspire +//! confidence, and I wanted a different return value, so I just took the code +//! to modify slightly. + +use std::mem; + +pub trait Endian: Sized + Copy { + type Buf: AsRef<[u8]> + AsMut<[u8]> + Default; + + fn to_le_bytes(self) -> Self::Buf; + fn to_be_bytes(self) -> Self::Buf; + + fn from_le_bytes(bytes: Self::Buf) -> Self; + fn from_be_bytes(bytes: Self::Buf) -> Self; +} + +macro_rules! delegate { + ($ty:ty, [$($method:ident),* $(,)?], ($param:ident : $param_ty:ty) -> $ret:ty) => { + delegate!(@inner $ty, [$($method),*], $param, $param_ty, $ret); + }; + (@inner $ty:ty, [$($method:ident),*], $param:ident, $param_ty:ty, $ret:ty) => { + $( + #[inline] + fn $method ($param: $param_ty) -> $ret { <$ty>::$method($param) } + )* + }; +} + +macro_rules! impl_primitives { + ($($ty:ty),* $(,)?) => { + $( + impl Endian for $ty { + type Buf = [u8; mem::size_of::<$ty>()]; + + delegate!($ty, [ + to_le_bytes, + to_be_bytes, + ], (self: Self) -> Self::Buf); + + delegate!($ty, [ + from_le_bytes, + from_be_bytes, + ], (bytes: Self::Buf) -> Self); + } + )* + }; +} + +#[rustfmt::skip] +impl_primitives![ + i8, i16, i32, i64, i128, + u8, u16, u32, u64, u128, +]; diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 6cab8a97f..b534ba1f9 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -1,3 +1,6 @@ +use crate::Endian; +use std::marker::PhantomData; + pub trait PrimaryKey<'a> { type Prefix: Prefixer<'a>; @@ -78,6 +81,7 @@ impl<'a> PrimaryKey<'a> for Pk1Owned { } } +// this auto-implements PrimaryKey for all the IntKey types (and more!) impl<'a, T: AsRef> PrimaryKey<'a> for T { type Prefix = (); @@ -89,24 +93,35 @@ impl<'a, T: AsRef> PrimaryKey<'a> for T { } } +pub type U16Key = IntKey; +pub type U32Key = IntKey; +pub type U64Key = IntKey; +pub type U128Key = IntKey; + // this reuses Pk1Owned logic with a particular type -pub struct U64Key(pub Pk1Owned); +pub struct IntKey { + pub wrapped: Pk1Owned, + pub data: PhantomData, +} -impl U64Key { - pub fn new(val: u64) -> Self { - U64Key(Pk1Owned(val.to_be_bytes().to_vec())) +impl IntKey { + pub fn new(val: T) -> Self { + IntKey { + wrapped: Pk1Owned(val.to_be_bytes().as_ref().to_vec()), + data: PhantomData, + } } } -impl From for U64Key { - fn from(val: u64) -> Self { - U64Key::new(val) +impl From for IntKey { + fn from(val: T) -> Self { + IntKey::new(val) } } -impl AsRef for U64Key { +impl AsRef for IntKey { fn as_ref(&self) -> &Pk1Owned { - &self.0 + &self.wrapped } } @@ -116,9 +131,17 @@ mod test { #[test] fn u64key_works() { - let k = u64_key(134); + let k: U64Key = 134u64.into(); let path = k.key(); assert_eq!(1, path.len()); assert_eq!(134u64.to_be_bytes().to_vec(), path[0].to_vec()); } + + #[test] + fn u32key_works() { + let k: U32Key = 4242u32.into(); + let path = k.key(); + assert_eq!(1, path.len()); + assert_eq!(4242u32.to_be_bytes().to_vec(), path[0].to_vec()); + } } diff --git a/packages/storage-plus/src/lib.rs b/packages/storage-plus/src/lib.rs index 0334c48a6..316c949fd 100644 --- a/packages/storage-plus/src/lib.rs +++ b/packages/storage-plus/src/lib.rs @@ -1,3 +1,4 @@ +mod endian; mod helpers; mod item; mod iter_helpers; @@ -6,8 +7,9 @@ mod map; mod path; mod prefix; +pub use endian::Endian; pub use item::Item; -pub use keys::{Pk1Owned, Prefixer, PrimaryKey, U64Key}; +pub use keys::{Pk1Owned, Prefixer, PrimaryKey, U128Key, U16Key, U32Key, U64Key}; pub use map::Map; pub use path::Path; #[cfg(feature = "iterator")] From 6d78e98ade463ac56d532a4ca9fbc64c456f0e99 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 19:12:48 +0200 Subject: [PATCH 05/13] Simplify some lifetime stuff --- packages/storage-plus/src/keys.rs | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index b534ba1f9..9f75ddef6 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -5,18 +5,13 @@ pub trait PrimaryKey<'a> { type Prefix: Prefixer<'a>; /// returns a slice of key steps, which can be optionally combined - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b; + fn key<'b>(&'b self) -> Vec<&'b [u8]>; } impl<'a> PrimaryKey<'a> for &'a [u8] { type Prefix = (); - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b, - { + fn key<'b>(&'b self) -> Vec<&'b [u8]> { // this is simple, we don't add more prefixes vec![self] } @@ -25,10 +20,7 @@ impl<'a> PrimaryKey<'a> for &'a [u8] { impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8]) { type Prefix = &'a [u8]; - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b, - { + fn key<'b>(&'b self) -> Vec<&'b [u8]> { vec![self.0, self.1] } } @@ -36,10 +28,7 @@ impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8]) { impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8], &'a [u8]) { type Prefix = (&'a [u8], &'a [u8]); - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b, - { + fn key<'b>(&'b self) -> Vec<&'b [u8]> { vec![self.0, self.1, self.2] } } @@ -73,10 +62,7 @@ pub struct Pk1Owned(pub Vec); impl<'a> PrimaryKey<'a> for Pk1Owned { type Prefix = (); - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b, - { + fn key<'b>(&'b self) -> Vec<&'b [u8]> { vec![&self.0] } } @@ -85,10 +71,7 @@ impl<'a> PrimaryKey<'a> for Pk1Owned { impl<'a, T: AsRef> PrimaryKey<'a> for T { type Prefix = (); - fn key<'b>(&'b self) -> Vec<&'b [u8]> - where - 'a: 'b, - { + fn key<'b>(&'b self) -> Vec<&'b [u8]> { self.as_ref().key() } } From c151690b3ddce0f2096d622378ceb2cc909560cf Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 19:27:14 +0200 Subject: [PATCH 06/13] Allow composite keys with int keys - eg (U32Key, U64Key) --- packages/storage-plus/src/keys.rs | 64 ++++++++++++++++++++++++------- packages/storage-plus/src/lib.rs | 2 +- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 9f75ddef6..6d01436aa 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -17,14 +17,18 @@ impl<'a> PrimaryKey<'a> for &'a [u8] { } } -impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8]) { - type Prefix = &'a [u8]; +// use generics for combining there - so we can use &[u8], PkOwned, or IntKey +impl<'a, T: PrimaryKey<'a> + Prefixer<'a>, U: PrimaryKey<'a>> PrimaryKey<'a> for (T, U) { + type Prefix = T; fn key<'b>(&'b self) -> Vec<&'b [u8]> { - vec![self.0, self.1] + let mut keys = self.0.key(); + keys.extend(&self.1.key()); + keys } } +// TODO: make this fully generic as above - this involved more work on prefix traits as well impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8], &'a [u8]) { type Prefix = (&'a [u8], &'a [u8]); @@ -35,31 +39,31 @@ impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8], &'a [u8]) { pub trait Prefixer<'a> { /// returns 0 or more namespaces that should length-prefixed and concatenated for range searches - fn prefix(&self) -> Vec<&'a [u8]>; + fn prefix<'b>(&'b self) -> Vec<&'b [u8]>; } impl<'a> Prefixer<'a> for () { - fn prefix(&self) -> Vec<&'a [u8]> { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { vec![] } } impl<'a> Prefixer<'a> for &'a [u8] { - fn prefix(&self) -> Vec<&'a [u8]> { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { vec![self] } } impl<'a> Prefixer<'a> for (&'a [u8], &'a [u8]) { - fn prefix(&self) -> Vec<&'a [u8]> { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { vec![self.0, self.1] } } // Add support for an dynamic keys - constructor functions below -pub struct Pk1Owned(pub Vec); +pub struct PkOwned(pub Vec); -impl<'a> PrimaryKey<'a> for Pk1Owned { +impl<'a> PrimaryKey<'a> for PkOwned { type Prefix = (); fn key<'b>(&'b self) -> Vec<&'b [u8]> { @@ -68,7 +72,7 @@ impl<'a> PrimaryKey<'a> for Pk1Owned { } // this auto-implements PrimaryKey for all the IntKey types (and more!) -impl<'a, T: AsRef> PrimaryKey<'a> for T { +impl<'a, T: AsRef> PrimaryKey<'a> for T { type Prefix = (); fn key<'b>(&'b self) -> Vec<&'b [u8]> { @@ -76,6 +80,19 @@ impl<'a, T: AsRef> PrimaryKey<'a> for T { } } +impl<'a> Prefixer<'a> for PkOwned { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { + vec![&self.0] + } +} + +// this auto-implements Prefixer for all the IntKey types (and more!) +impl<'a, T: AsRef> Prefixer<'a> for T { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { + self.as_ref().prefix() + } +} + pub type U16Key = IntKey; pub type U32Key = IntKey; pub type U64Key = IntKey; @@ -83,14 +100,14 @@ pub type U128Key = IntKey; // this reuses Pk1Owned logic with a particular type pub struct IntKey { - pub wrapped: Pk1Owned, + pub wrapped: PkOwned, pub data: PhantomData, } impl IntKey { pub fn new(val: T) -> Self { IntKey { - wrapped: Pk1Owned(val.to_be_bytes().as_ref().to_vec()), + wrapped: PkOwned(val.to_be_bytes().as_ref().to_vec()), data: PhantomData, } } @@ -102,8 +119,8 @@ impl From for IntKey { } } -impl AsRef for IntKey { - fn as_ref(&self) -> &Pk1Owned { +impl AsRef for IntKey { + fn as_ref(&self) -> &PkOwned { &self.wrapped } } @@ -127,4 +144,23 @@ mod test { assert_eq!(1, path.len()); assert_eq!(4242u32.to_be_bytes().to_vec(), path[0].to_vec()); } + + #[test] + fn composite_byte_key() { + let k: (&[u8], &[u8]) = (b"foo", b"bar"); + let path = k.key(); + assert_eq!(2, path.len()); + assert_eq!(path, vec![b"foo", b"bar"]); + } + + #[test] + fn composite_int_key() { + let k: (U32Key, U64Key) = (123.into(), 87654.into()); + let path = k.key(); + assert_eq!(2, path.len()); + assert_eq!(4, path[0].len()); + assert_eq!(8, path[1].len()); + assert_eq!(path[0].to_vec(), 123u32.to_be_bytes().to_vec()); + assert_eq!(path[1].to_vec(), 87654u64.to_be_bytes().to_vec()); + } } diff --git a/packages/storage-plus/src/lib.rs b/packages/storage-plus/src/lib.rs index 316c949fd..58e2dbca2 100644 --- a/packages/storage-plus/src/lib.rs +++ b/packages/storage-plus/src/lib.rs @@ -9,7 +9,7 @@ mod prefix; pub use endian::Endian; pub use item::Item; -pub use keys::{Pk1Owned, Prefixer, PrimaryKey, U128Key, U16Key, U32Key, U64Key}; +pub use keys::{PkOwned, Prefixer, PrimaryKey, U128Key, U16Key, U32Key, U64Key}; pub use map::Map; pub use path::Path; #[cfg(feature = "iterator")] From 3dc02219bba5b3f42b3bac6108baee1d024973d5 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 19:32:39 +0200 Subject: [PATCH 07/13] Expose Map.range() on IntKey as well as &[u8] --- packages/storage-plus/src/keys.rs | 11 +++++++++++ packages/storage-plus/src/map.rs | 8 +++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 6d01436aa..8ee497eb2 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -60,6 +60,17 @@ impl<'a> Prefixer<'a> for (&'a [u8], &'a [u8]) { } } +// this is a marker for the Map.range() helper, so we can detect () in Generic bounds +pub trait EmptyPrefix { + fn new() -> Self; +} + +impl EmptyPrefix for () { + fn new() -> Self { + () + } +} + // Add support for an dynamic keys - constructor functions below pub struct PkOwned(pub Vec); diff --git a/packages/storage-plus/src/map.rs b/packages/storage-plus/src/map.rs index d7b57cd2a..22a6e71f1 100644 --- a/packages/storage-plus/src/map.rs +++ b/packages/storage-plus/src/map.rs @@ -4,7 +4,7 @@ use std::marker::PhantomData; #[cfg(feature = "iterator")] use crate::keys::Prefixer; -use crate::keys::PrimaryKey; +use crate::keys::{EmptyPrefix, PrimaryKey}; use crate::path::Path; #[cfg(feature = "iterator")] use crate::prefix::{Bound, Prefix}; @@ -76,9 +76,11 @@ where // short-cut for simple keys, rather than .prefix(()).range(...) #[cfg(feature = "iterator")] -impl<'a, T> Map<'a, &'a [u8], T> +impl<'a, K, T> Map<'a, K, T> where T: Serialize + DeserializeOwned, + K: PrimaryKey<'a>, + K::Prefix: EmptyPrefix, { // I would prefer not to copy code from Prefix, but no other way // with lifetimes (create Prefix inside function and return ref = no no) @@ -92,7 +94,7 @@ where where T: 'c, { - self.prefix(()).range(store, min, max, order) + self.prefix(K::Prefix::new()).range(store, min, max, order) } } From 4df8d7632674405090d3f053e2bedeef0671c9e1 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 19:56:16 +0200 Subject: [PATCH 08/13] Add OwnedBound --- packages/storage-plus/src/endian.rs | 2 +- packages/storage-plus/src/keys.rs | 5 +++- packages/storage-plus/src/lib.rs | 2 +- packages/storage-plus/src/map.rs | 4 +-- packages/storage-plus/src/prefix.rs | 42 +++++++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/packages/storage-plus/src/endian.rs b/packages/storage-plus/src/endian.rs index 0990ecee0..719b62af3 100644 --- a/packages/storage-plus/src/endian.rs +++ b/packages/storage-plus/src/endian.rs @@ -7,7 +7,7 @@ use std::mem; pub trait Endian: Sized + Copy { - type Buf: AsRef<[u8]> + AsMut<[u8]> + Default; + type Buf: AsRef<[u8]> + AsMut<[u8]> + Into> + Default; fn to_le_bytes(self) -> Self::Buf; fn to_be_bytes(self) -> Self::Buf; diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 8ee497eb2..9a3871bfd 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -74,6 +74,9 @@ impl EmptyPrefix for () { // Add support for an dynamic keys - constructor functions below pub struct PkOwned(pub Vec); +// FIXME: simplify all this - I think we can just implement generic for AsRef<&[u8]> and most of the rest falls awayy? +// But then I hit lifetime issues.... + impl<'a> PrimaryKey<'a> for PkOwned { type Prefix = (); @@ -118,7 +121,7 @@ pub struct IntKey { impl IntKey { pub fn new(val: T) -> Self { IntKey { - wrapped: PkOwned(val.to_be_bytes().as_ref().to_vec()), + wrapped: PkOwned(val.to_be_bytes().into()), data: PhantomData, } } diff --git a/packages/storage-plus/src/lib.rs b/packages/storage-plus/src/lib.rs index 58e2dbca2..29d286443 100644 --- a/packages/storage-plus/src/lib.rs +++ b/packages/storage-plus/src/lib.rs @@ -13,4 +13,4 @@ pub use keys::{PkOwned, Prefixer, PrimaryKey, U128Key, U16Key, U32Key, U64Key}; pub use map::Map; pub use path::Path; #[cfg(feature = "iterator")] -pub use prefix::Prefix; +pub use prefix::{Bound, OwnedBound, Prefix}; diff --git a/packages/storage-plus/src/map.rs b/packages/storage-plus/src/map.rs index 22a6e71f1..923d27352 100644 --- a/packages/storage-plus/src/map.rs +++ b/packages/storage-plus/src/map.rs @@ -2,9 +2,9 @@ use serde::de::DeserializeOwned; use serde::Serialize; use std::marker::PhantomData; +use crate::keys::PrimaryKey; #[cfg(feature = "iterator")] -use crate::keys::Prefixer; -use crate::keys::{EmptyPrefix, PrimaryKey}; +use crate::keys::{EmptyPrefix, Prefixer}; use crate::path::Path; #[cfg(feature = "iterator")] use crate::prefix::{Bound, Prefix}; diff --git a/packages/storage-plus/src/prefix.rs b/packages/storage-plus/src/prefix.rs index 2d8516b86..52508cbd3 100644 --- a/packages/storage-plus/src/prefix.rs +++ b/packages/storage-plus/src/prefix.rs @@ -7,6 +7,7 @@ use cosmwasm_std::{Order, StdResult, Storage, KV}; use crate::helpers::nested_namespaces_with_key; use crate::iter_helpers::{concat, deserialize_kv, trim}; +use crate::Endian; /// Bound is used to defines the two ends of a range, more explicit than Option /// None means that we don't limit that side of the range at all. @@ -19,6 +20,47 @@ pub enum Bound<'a> { None, } +/// OwnedBound is like bound, but owns the data (as a Vec) inside. +/// It is much easier to use if you dynamically construct the content, and can be passed into range as well. +#[derive(Clone, Debug)] +pub enum OwnedBound { + Inclusive(Vec), + Exclusive(Vec), + None, +} + +impl OwnedBound { + pub fn bound<'a>(&'a self) -> Bound<'a> { + match self { + OwnedBound::Inclusive(limit) => Bound::Inclusive(&limit), + OwnedBound::Exclusive(limit) => Bound::Exclusive(&limit), + OwnedBound::None => Bound::None, + } + } + + pub fn inclusive(limit: T) -> Self { + OwnedBound::Inclusive(limit.to_be_bytes().into()) + } + + pub fn exclusive(limit: T) -> Self { + OwnedBound::Exclusive(limit.to_be_bytes().into()) + } + + pub fn inclusive_or_none(limit: Option) -> Self { + match limit { + Some(t) => Self::inclusive(t), + None => OwnedBound::None, + } + } + + pub fn exclusive_or_none(limit: Option) -> Self { + match limit { + Some(t) => Self::exclusive(t), + None => OwnedBound::None, + } + } +} + pub struct Prefix where T: Serialize + DeserializeOwned, From 3a88cbb7953763122185a019144c464e8996ca2a Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 20:12:37 +0200 Subject: [PATCH 09/13] Contract compiles --- contracts/cw3-fixed-multisig/src/contract.rs | 39 +++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/contracts/cw3-fixed-multisig/src/contract.rs b/contracts/cw3-fixed-multisig/src/contract.rs index 81a30d1be..23e9b4770 100644 --- a/contracts/cw3-fixed-multisig/src/contract.rs +++ b/contracts/cw3-fixed-multisig/src/contract.rs @@ -5,12 +5,13 @@ use cosmwasm_std::{ HandleResponse, HumanAddr, InitResponse, MessageInfo, Order, Querier, StdResult, Storage, }; -use cw0::{calc_range_start_human, Expiration}; +use cw0::Expiration; use cw2::set_contract_version; use cw3::{ ProposalListResponse, ProposalResponse, Status, ThresholdResponse, Vote, VoteInfo, VoteListResponse, VoteResponse, VoterListResponse, VoterResponse, }; +use cw_storage_plus::{Bound, OwnedBound}; use crate::error::ContractError; use crate::msg::{HandleMsg, InitMsg, QueryMsg}; @@ -331,9 +332,9 @@ fn list_proposals( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = start_after.map(|id| id.to_be_bytes().to_vec()); + let start = OwnedBound::exclusive_or_none(start_after); let props: StdResult> = PROPOSALS - .range(&deps.storage, start.as_deref(), None, Order::Ascending) + .range(&deps.storage, start.bound(), Bound::None, Order::Ascending) .take(limit) .map(|p| map_proposal(&env.block, p)) .collect(); @@ -348,9 +349,9 @@ fn reverse_proposals( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let end = start_before.map(|id| id.to_be_bytes().to_vec()); - let props: StdResult> = proposal_read(&deps.storage) - .range(None, end.as_deref(), Order::Descending) + let end = OwnedBound::exclusive_or_none(start_before); + let props: StdResult> = PROPOSALS + .range(&deps.storage, Bound::None, end.bound(), Order::Descending) .take(limit) .map(|p| map_proposal(&env.block, p)) .collect(); @@ -385,6 +386,15 @@ fn query_vote( Ok(VoteResponse { vote }) } +fn make_exclusive_bound(api: A, limit: Option) -> StdResult { + let canon: Option = limit.map(|x| api.canonical_address(&x)).transpose()?; + let start = match canon { + Some(x) => OwnedBound::Exclusive(x.into()), + None => OwnedBound::None, + }; + Ok(start) +} + fn list_votes( deps: &Extern, proposal_id: u64, @@ -392,11 +402,12 @@ fn list_votes( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = calc_range_start_human(deps.api, start_after)?; - let api = &deps.api; + let start = make_exclusive_bound(deps.api, start_after)?; - let votes: StdResult> = ballots_read(&deps.storage, proposal_id) - .range(start.as_deref(), None, Order::Ascending) + let api = &deps.api; + let votes: StdResult> = BALLOTS + .prefix(proposal_id.into()) + .range(&deps.storage, start.bound(), Bound::None, Order::Ascending) .take(limit) .map(|item| { let (key, ballot) = item?; @@ -431,11 +442,11 @@ fn list_voters( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = calc_range_start_human(deps.api, start_after)?; - let api = &deps.api; + let start = make_exclusive_bound(deps.api, start_after)?; - let voters: StdResult> = voters_read(&deps.storage) - .range(start.as_deref(), None, Order::Ascending) + let api = &deps.api; + let voters: StdResult> = VOTERS + .range(&deps.storage, start.bound(), Bound::None, Order::Ascending) .take(limit) .map(|item| { let (key, weight) = item?; From 3b05b320fe600074857d5cf4f84710f742905dca Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 20:14:58 +0200 Subject: [PATCH 10/13] Fix clippy --- contracts/cw3-fixed-multisig/src/contract.rs | 1 + packages/storage-plus/src/keys.rs | 4 +--- packages/storage-plus/src/prefix.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/contracts/cw3-fixed-multisig/src/contract.rs b/contracts/cw3-fixed-multisig/src/contract.rs index 23e9b4770..931e90963 100644 --- a/contracts/cw3-fixed-multisig/src/contract.rs +++ b/contracts/cw3-fixed-multisig/src/contract.rs @@ -386,6 +386,7 @@ fn query_vote( Ok(VoteResponse { vote }) } +// TODO: pull this out into a helper - one for Canonical as well fn make_exclusive_bound(api: A, limit: Option) -> StdResult { let canon: Option = limit.map(|x| api.canonical_address(&x)).transpose()?; let start = match canon { diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index 9a3871bfd..f406f07b8 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -66,9 +66,7 @@ pub trait EmptyPrefix { } impl EmptyPrefix for () { - fn new() -> Self { - () - } + fn new() {} } // Add support for an dynamic keys - constructor functions below diff --git a/packages/storage-plus/src/prefix.rs b/packages/storage-plus/src/prefix.rs index 52508cbd3..a5385a5a6 100644 --- a/packages/storage-plus/src/prefix.rs +++ b/packages/storage-plus/src/prefix.rs @@ -30,7 +30,7 @@ pub enum OwnedBound { } impl OwnedBound { - pub fn bound<'a>(&'a self) -> Bound<'a> { + pub fn bound(&self) -> Bound<'_> { match self { OwnedBound::Inclusive(limit) => Bound::Inclusive(&limit), OwnedBound::Exclusive(limit) => Bound::Exclusive(&limit), From f97a7de44b6a930fd1d5179ee6f95b786a532f32 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 20:29:16 +0200 Subject: [PATCH 11/13] Simplify pagination with HumanAddr --- contracts/cw3-fixed-multisig/src/contract.rs | 22 ++--- packages/cw0/src/lib.rs | 4 +- packages/cw0/src/pagination.rs | 10 ++- packages/storage-plus/src/prefix.rs | 94 +++++++++++--------- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/contracts/cw3-fixed-multisig/src/contract.rs b/contracts/cw3-fixed-multisig/src/contract.rs index 931e90963..4486e2d46 100644 --- a/contracts/cw3-fixed-multisig/src/contract.rs +++ b/contracts/cw3-fixed-multisig/src/contract.rs @@ -5,7 +5,7 @@ use cosmwasm_std::{ HandleResponse, HumanAddr, InitResponse, MessageInfo, Order, Querier, StdResult, Storage, }; -use cw0::Expiration; +use cw0::{maybe_canonical, Expiration}; use cw2::set_contract_version; use cw3::{ ProposalListResponse, ProposalResponse, Status, ThresholdResponse, Vote, VoteInfo, @@ -332,7 +332,7 @@ fn list_proposals( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = OwnedBound::exclusive_or_none(start_after); + let start = OwnedBound::exclusive_int(start_after); let props: StdResult> = PROPOSALS .range(&deps.storage, start.bound(), Bound::None, Order::Ascending) .take(limit) @@ -349,7 +349,7 @@ fn reverse_proposals( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let end = OwnedBound::exclusive_or_none(start_before); + let end = OwnedBound::exclusive_int(start_before); let props: StdResult> = PROPOSALS .range(&deps.storage, Bound::None, end.bound(), Order::Descending) .take(limit) @@ -386,16 +386,6 @@ fn query_vote( Ok(VoteResponse { vote }) } -// TODO: pull this out into a helper - one for Canonical as well -fn make_exclusive_bound(api: A, limit: Option) -> StdResult { - let canon: Option = limit.map(|x| api.canonical_address(&x)).transpose()?; - let start = match canon { - Some(x) => OwnedBound::Exclusive(x.into()), - None => OwnedBound::None, - }; - Ok(start) -} - fn list_votes( deps: &Extern, proposal_id: u64, @@ -403,7 +393,8 @@ fn list_votes( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = make_exclusive_bound(deps.api, start_after)?; + let canon = maybe_canonical(deps.api, start_after)?; + let start = OwnedBound::exclusive(canon); let api = &deps.api; let votes: StdResult> = BALLOTS @@ -443,7 +434,8 @@ fn list_voters( limit: Option, ) -> StdResult { let limit = limit.unwrap_or(DEFAULT_LIMIT).min(MAX_LIMIT) as usize; - let start = make_exclusive_bound(deps.api, start_after)?; + let canon = maybe_canonical(deps.api, start_after)?; + let start = OwnedBound::exclusive(canon); let api = &deps.api; let voters: StdResult> = VOTERS diff --git a/packages/cw0/src/lib.rs b/packages/cw0/src/lib.rs index 8c21089fb..586ecea7e 100644 --- a/packages/cw0/src/lib.rs +++ b/packages/cw0/src/lib.rs @@ -4,4 +4,6 @@ mod pagination; pub use crate::balance::NativeBalance; pub use crate::expiration::{Duration, Expiration, DAY, HOUR, WEEK}; -pub use pagination::{calc_range_end_human, calc_range_start_human, calc_range_start_string}; +pub use pagination::{ + calc_range_end_human, calc_range_start_human, calc_range_start_string, maybe_canonical, +}; diff --git a/packages/cw0/src/pagination.rs b/packages/cw0/src/pagination.rs index 21381d60d..54d340e9f 100644 --- a/packages/cw0/src/pagination.rs +++ b/packages/cw0/src/pagination.rs @@ -1,4 +1,12 @@ -use cosmwasm_std::{Api, HumanAddr, StdResult}; +use cosmwasm_std::{Api, CanonicalAddr, HumanAddr, StdResult}; + +// this is used for pagination. Maybe we move it into the std lib one day? +pub fn maybe_canonical( + api: A, + human: Option, +) -> StdResult> { + human.map(|x| api.canonical_address(&x)).transpose() +} // this will set the first key after the provided key, by appending a 0 byte pub fn calc_range_start_human( diff --git a/packages/storage-plus/src/prefix.rs b/packages/storage-plus/src/prefix.rs index a5385a5a6..42521dede 100644 --- a/packages/storage-plus/src/prefix.rs +++ b/packages/storage-plus/src/prefix.rs @@ -20,47 +20,6 @@ pub enum Bound<'a> { None, } -/// OwnedBound is like bound, but owns the data (as a Vec) inside. -/// It is much easier to use if you dynamically construct the content, and can be passed into range as well. -#[derive(Clone, Debug)] -pub enum OwnedBound { - Inclusive(Vec), - Exclusive(Vec), - None, -} - -impl OwnedBound { - pub fn bound(&self) -> Bound<'_> { - match self { - OwnedBound::Inclusive(limit) => Bound::Inclusive(&limit), - OwnedBound::Exclusive(limit) => Bound::Exclusive(&limit), - OwnedBound::None => Bound::None, - } - } - - pub fn inclusive(limit: T) -> Self { - OwnedBound::Inclusive(limit.to_be_bytes().into()) - } - - pub fn exclusive(limit: T) -> Self { - OwnedBound::Exclusive(limit.to_be_bytes().into()) - } - - pub fn inclusive_or_none(limit: Option) -> Self { - match limit { - Some(t) => Self::inclusive(t), - None => OwnedBound::None, - } - } - - pub fn exclusive_or_none(limit: Option) -> Self { - match limit { - Some(t) => Self::exclusive(t), - None => OwnedBound::None, - } - } -} - pub struct Prefix where T: Serialize + DeserializeOwned, @@ -100,6 +59,59 @@ where } } +/// OwnedBound is like bound, but owns the data (as a Vec) inside. +/// It is much easier to use if you dynamically construct the content, and can be passed into range as well. +/// We provide lots of helpers to create these bounds from other data-types +#[derive(Clone, Debug)] +pub enum OwnedBound { + Inclusive(Vec), + Exclusive(Vec), + None, +} + +impl OwnedBound { + /// Returns a bound that borrows the owned data, to pass into range() + pub fn bound(&self) -> Bound<'_> { + match self { + OwnedBound::Inclusive(limit) => Bound::Inclusive(&limit), + OwnedBound::Exclusive(limit) => Bound::Exclusive(&limit), + OwnedBound::None => Bound::None, + } + } + + /// Turns optional binary, like Option into an inclusive bound + pub fn inclusive>>(maybe: Option) -> Self { + match maybe { + Some(bytes) => OwnedBound::Inclusive(bytes.into()), + None => OwnedBound::None, + } + } + + /// Turns optional binary, like Option into an exclusive bound + pub fn exclusive>>(maybe: Option) -> Self { + match maybe { + Some(bytes) => OwnedBound::Exclusive(bytes.into()), + None => OwnedBound::None, + } + } + + /// Turns an int, like Option into an inclusive bound + pub fn inclusive_int(limit: Option) -> Self { + match limit { + Some(t) => Self::Inclusive(t.to_be_bytes().into()), + None => OwnedBound::None, + } + } + + /// Turns an int, like Option into an exclusive bound + pub fn exclusive_int(limit: Option) -> Self { + match limit { + Some(t) => Self::Exclusive(t.to_be_bytes().into()), + None => OwnedBound::None, + } + } +} + pub(crate) fn range_with_prefix<'a, S: Storage>( storage: &'a S, namespace: &[u8], From 36e135a2105f0e43b94aa631439b3603f69922de Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 20:51:13 +0200 Subject: [PATCH 12/13] Cleanup README from PR #109 comments --- packages/storage-plus/README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/storage-plus/README.md b/packages/storage-plus/README.md index c14ba531f..2fd869c36 100644 --- a/packages/storage-plus/README.md +++ b/packages/storage-plus/README.md @@ -2,7 +2,7 @@ The ideas in here are based on the `cosmwasm-storage` crate. However, after much usage, we decided a complete rewrite could allow us to add -more powerful and easy touse interfaces. Here are those interfaces. +more powerful and easy to use interfaces. Here are those interfaces. **Status: experimental** @@ -11,7 +11,8 @@ repo. This is a first draft of many types. We will update the status after they have been used more heavily and the interfaces stabilized. The ideas/desired functionality in here should be more or final, -just the form to express them which will keep changing. +just the form to express them that is not final. As we add new functionality, +we will continue to refine the foundations, but maintain semver. ## Usage Overview @@ -19,7 +20,7 @@ We introduce two main classes to provide a productive abstraction on top of `cosmwasm_std::Storage`. They are `Item`, which is a typed wrapper around one database key, providing some helper functions for interacting with it without dealing with raw bytes. And `Map`, -which allows you to store multiple typed objects under a prefix, +which allows you to store multiple unique typed objects under a prefix, indexed by a simple (`&[u8]`) or compound (eg. `(&[u8], &[u8])`) primary key. These correspond to the concepts represented in `cosmwasm_storage` by @@ -110,7 +111,7 @@ eg. `(owner, spender)` to look up the balance. Beyond direct lookups, we have a super power not found in Ethereum - iteration. That's right, you can list all items in a `Map`, or only part of them. We can efficiently allow pagination over these items as -well, starting after the last query ended at low, low gas costs. +well, starting at the point the last query ended, with low gas costs. This requires the `iterator` feature to be enabled in `cw-storage-plus` (which automatically enables it in `cosmwasm-std` as well). @@ -191,7 +192,7 @@ fn demo() -> StdResult<()> { There are times when we want to use multiple items as a key, for example, when storing allowances based on account owner and spender. We could try to manually -concatenate them before calling, but that can lead ot overlap, and is a bit +concatenate them before calling, but that can lead to overlap, and is a bit low-level for us. Also, by explicitly separating the keys, we can easily provide helpers to do range queries over a prefix, such as "show me all allowances for one owner" (first part of the composite key). Just like you'd expect from your From 3d77baa51c5dfc1cc2be7a2dde0a81c822f403d4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Mon, 12 Oct 2020 22:46:28 +0200 Subject: [PATCH 13/13] Remove (a, b, c) key as generics covers it --- packages/storage-plus/src/keys.rs | 50 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/storage-plus/src/keys.rs b/packages/storage-plus/src/keys.rs index f406f07b8..d0bfe5b4a 100644 --- a/packages/storage-plus/src/keys.rs +++ b/packages/storage-plus/src/keys.rs @@ -28,14 +28,8 @@ impl<'a, T: PrimaryKey<'a> + Prefixer<'a>, U: PrimaryKey<'a>> PrimaryKey<'a> for } } -// TODO: make this fully generic as above - this involved more work on prefix traits as well -impl<'a> PrimaryKey<'a> for (&'a [u8], &'a [u8], &'a [u8]) { - type Prefix = (&'a [u8], &'a [u8]); - - fn key<'b>(&'b self) -> Vec<&'b [u8]> { - vec![self.0, self.1, self.2] - } -} +// Future work: add more types - 3 or more or slices? +// Right now 3 could be done via ((a, b), c) pub trait Prefixer<'a> { /// returns 0 or more namespaces that should length-prefixed and concatenated for range searches @@ -72,9 +66,6 @@ impl EmptyPrefix for () { // Add support for an dynamic keys - constructor functions below pub struct PkOwned(pub Vec); -// FIXME: simplify all this - I think we can just implement generic for AsRef<&[u8]> and most of the rest falls awayy? -// But then I hit lifetime issues.... - impl<'a> PrimaryKey<'a> for PkOwned { type Prefix = (); @@ -83,6 +74,12 @@ impl<'a> PrimaryKey<'a> for PkOwned { } } +impl<'a> Prefixer<'a> for PkOwned { + fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { + vec![&self.0] + } +} + // this auto-implements PrimaryKey for all the IntKey types (and more!) impl<'a, T: AsRef> PrimaryKey<'a> for T { type Prefix = (); @@ -92,12 +89,6 @@ impl<'a, T: AsRef> PrimaryKey<'a> for T { } } -impl<'a> Prefixer<'a> for PkOwned { - fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { - vec![&self.0] - } -} - // this auto-implements Prefixer for all the IntKey types (and more!) impl<'a, T: AsRef> Prefixer<'a> for T { fn prefix<'b>(&'b self) -> Vec<&'b [u8]> { @@ -110,7 +101,12 @@ pub type U32Key = IntKey; pub type U64Key = IntKey; pub type U128Key = IntKey; -// this reuses Pk1Owned logic with a particular type +/// It will cast one-particular int type into a Key via PkOwned, ensuring you don't mix up u32 and u64 +/// You can use new or the from/into pair to build a key from an int: +/// +/// let k = U64Key::new(12345); +/// let k = U32Key::from(12345); +/// let k: U16Key = 12345.into(); pub struct IntKey { pub wrapped: PkOwned, pub data: PhantomData, @@ -167,6 +163,8 @@ mod test { #[test] fn composite_int_key() { + // Note we don't spec the int types (u32, u64) on the right, + // just the keys they convert into let k: (U32Key, U64Key) = (123.into(), 87654.into()); let path = k.key(); assert_eq!(2, path.len()); @@ -175,4 +173,20 @@ mod test { assert_eq!(path[0].to_vec(), 123u32.to_be_bytes().to_vec()); assert_eq!(path[1].to_vec(), 87654u64.to_be_bytes().to_vec()); } + + #[test] + fn nested_composite_keys() { + // use this to ensure proper type-casts below + let foo: &[u8] = b"foo"; + // this function tests how well the generics extend to "edge cases" + let k: ((&[u8], &[u8]), &[u8]) = ((foo, b"bar"), b"zoom"); + let path = k.key(); + assert_eq!(3, path.len()); + assert_eq!(path, vec![foo, b"bar", b"zoom"]); + + // ensure prefix also works + let dir = k.0.prefix(); + assert_eq!(2, dir.len()); + assert_eq!(dir, vec![foo, b"bar"]); + } }