From b566147f2fa5ccedaac8124dbe4679ac7b98696f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Wed, 3 Jan 2024 15:28:29 +0000 Subject: [PATCH] storage and query tests --- .../coconut-dkg/src/types.rs | 11 +- contracts/coconut-dkg/src/dealings/queries.rs | 291 +++++++++++------- contracts/coconut-dkg/src/dealings/storage.rs | 222 +++++++++++-- .../src/epoch_state/transactions.rs | 75 ++--- .../coconut-dkg/src/support/tests/fixtures.rs | 2 +- 5 files changed, 421 insertions(+), 180 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs index 74b6d59c9d..19ead7c077 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs @@ -21,10 +21,19 @@ pub const TOTAL_DEALINGS: usize = 2 + 2 + 1; #[cw_serde] pub struct PartialContractDealing { - pub index: u32, + pub index: DealingIndex, pub data: ContractDealing, } +impl From<(DealingIndex, ContractDealing)> for PartialContractDealing { + fn from(value: (DealingIndex, ContractDealing)) -> Self { + PartialContractDealing { + index: value.0, + data: value.1, + } + } +} + #[cw_serde] pub struct InitialReplacementData { pub initial_dealers: Vec, diff --git a/contracts/coconut-dkg/src/dealings/queries.rs b/contracts/coconut-dkg/src/dealings/queries.rs index 3f2f0470f7..c85c4737ef 100644 --- a/contracts/coconut-dkg/src/dealings/queries.rs +++ b/contracts/coconut-dkg/src/dealings/queries.rs @@ -7,6 +7,7 @@ use cosmwasm_std::{Deps, StdResult}; use cw_storage_plus::Bound; use nym_coconut_dkg_common::dealer::{DealingResponse, PagedDealingsResponse}; use nym_coconut_dkg_common::types::{DealingIndex, EpochId}; + pub fn query_dealing( deps: Deps<'_>, epoch_id: EpochId, @@ -56,148 +57,200 @@ pub fn query_dealings_paged( pub(crate) mod tests { use super::*; use crate::dealings::storage::{DEALINGS_PAGE_DEFAULT_LIMIT, DEALINGS_PAGE_MAX_LIMIT}; - use crate::support::tests::fixtures::dealing_bytes_fixture; + use crate::support::tests::fixtures::{dealing_bytes_fixture, partial_dealing_fixture}; use crate::support::tests::helpers::init_contract; use cosmwasm_std::{Addr, DepsMut}; + use nym_coconut_dkg_common::types::PartialContractDealing; - fn fill_dealings(deps: DepsMut<'_>, size: usize) { - for n in 0..size { - let dealing_share = dealing_bytes_fixture(); - let sender = Addr::unchecked(format!("owner{}", n)); - (0..TOTAL_DEALINGS).for_each(|idx| { - DEALINGS_BYTES[idx] - .save(deps.storage, &sender, &dealing_share) - .unwrap(); - }); + fn fill_dealings(deps: DepsMut<'_>, epoch: EpochId, dealers: usize, key_size: u32) { + for i in 0..dealers { + let dealer = Addr::unchecked(format!("dealer{i}")); + for dealing_index in 0..key_size { + StoredDealing::save( + deps.storage, + epoch, + &dealer, + PartialContractDealing { + index: dealing_index, + data: dealing_bytes_fixture(), + }, + ) + } } } #[test] - fn empty_on_bad_idx() { - let mut deps = init_contract(); - fill_dealings(deps.as_mut(), 1000); - - for idx in TOTAL_DEALINGS as u64..100 * TOTAL_DEALINGS as u64 { - let page1 = query_dealings_paged(deps.as_ref(), idx, None, None).unwrap(); - assert_eq!(0, page1.dealings.len() as u32); - } - } - - #[test] - fn dealings_empty_on_init() { - let deps = init_contract(); - for idx in 0..TOTAL_DEALINGS as u64 { - let response = query_dealings_paged(deps.as_ref(), idx, None, Option::from(2)).unwrap(); - assert_eq!(0, response.dealings.len()); - } - } - - #[test] - fn dealings_paged_retrieval_obeys_limits() { - let mut deps = init_contract(); - let limit = 2; - fill_dealings(deps.as_mut(), 1000); - - for idx in 0..TOTAL_DEALINGS as u64 { - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(limit)).unwrap(); - assert_eq!(limit, page1.dealings.len() as u32); - } - } - - #[test] - fn dealings_paged_retrieval_has_default_limit() { - let mut deps = init_contract(); - fill_dealings(deps.as_mut(), 1000); - - for idx in 0..TOTAL_DEALINGS as u64 { - // query without explicitly setting a limit - let page1 = query_dealings_paged(deps.as_ref(), idx, None, None).unwrap(); - - assert_eq!(DEALINGS_PAGE_DEFAULT_LIMIT, page1.dealings.len() as u32); - } - } - - #[test] - fn dealings_paged_retrieval_has_max_limit() { - let mut deps = init_contract(); - fill_dealings(deps.as_mut(), 1000); - - // query with a crazily high limit in an attempt to use too many resources - let crazy_limit = 1000 * DEALINGS_PAGE_MAX_LIMIT; - for idx in 0..TOTAL_DEALINGS as u64 { - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(crazy_limit)).unwrap(); - - // we default to a decent sized upper bound instead - let expected_limit = DEALINGS_PAGE_MAX_LIMIT; - assert_eq!(expected_limit, page1.dealings.len() as u32); - } - } - - #[test] - fn dealings_pagination_works() { + fn test_query_dealing() { let mut deps = init_contract(); - fill_dealings(deps.as_mut(), 1); + let bad_address = "FOOMP".to_string(); + assert!(query_dealing(deps.as_ref(), 0, bad_address, 0).is_err()); - let per_page = 2; + let empty = query_dealing(deps.as_ref(), 0, "foo".to_string(), 0).unwrap(); + assert_eq!(empty.epoch_id, 0); + assert_eq!(empty.dealing_index, 0); + assert_eq!(empty.dealer, Addr::unchecked("foo")); + assert!(empty.dealing.is_none()); - for idx in 0..TOTAL_DEALINGS as u64 { - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(per_page)).unwrap(); + // insert the dealing + let dealing = partial_dealing_fixture(); + StoredDealing::save( + deps.as_mut().storage, + 0, + &Addr::unchecked("foo"), + dealing.clone(), + ); - // page should have 1 result on it - assert_eq!(1, page1.dealings.len()); + let retrieved = query_dealing(deps.as_ref(), 0, "foo".to_string(), 0).unwrap(); + assert_eq!(retrieved.epoch_id, 0); + assert_eq!(retrieved.dealing_index, dealing.index); + assert_eq!(retrieved.dealer, Addr::unchecked("foo")); + assert_eq!(retrieved.dealing.unwrap(), dealing.data); + } + + #[cfg(test)] + mod query_dealings { + use super::*; + use nym_coconut_dkg_common::types::TOTAL_DEALINGS; + + #[test] + fn dealings_empty_on_init() { + let deps = init_contract(); + let all_dealings = StoredDealing::unchecked_all_entries(&deps.storage); + assert!(all_dealings.is_empty()) } - // save another - fill_dealings(deps.as_mut(), 2); + #[test] + fn dealings_paged_retrieval_obeys_limits() { + let mut deps = init_contract(); + let limit = 2; + fill_dealings(deps.as_mut(), 0, 10, TOTAL_DEALINGS as u32); - for idx in 0..TOTAL_DEALINGS as u64 { - // page1 should have 2 results on it - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(per_page)).unwrap(); - assert_eq!(2, page1.dealings.len()); + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + let page1 = + query_dealings_paged(deps.as_ref(), 0, dealer, None, Option::from(limit)) + .unwrap(); + assert_eq!(limit, page1.dealings.len() as u32); + } } - fill_dealings(deps.as_mut(), 3); + #[test] + fn dealings_paged_retrieval_has_default_limit() { + let mut deps = init_contract(); + fill_dealings(deps.as_mut(), 0, 10, TOTAL_DEALINGS as u32); - for idx in 0..TOTAL_DEALINGS as u64 { - // page1 still has 2 results - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(per_page)).unwrap(); - assert_eq!(2, page1.dealings.len()); + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + // query without explicitly setting a limit + let page1 = query_dealings_paged(deps.as_ref(), 0, dealer, None, None).unwrap(); - // retrieving the next page should start after the last key on this page - let start_after = page1.start_next_after.unwrap(); - let page2 = query_dealings_paged( - deps.as_ref(), - idx, - Option::from(start_after.to_string()), - Option::from(per_page), - ) - .unwrap(); - - assert_eq!(1, page2.dealings.len()); + assert_eq!(DEALINGS_PAGE_DEFAULT_LIMIT, page1.dealings.len() as u32); + } } - fill_dealings(deps.as_mut(), 4); + #[test] + fn dealings_paged_retrieval_has_max_limit() { + let mut deps = init_contract(); + fill_dealings(deps.as_mut(), 0, 10, TOTAL_DEALINGS as u32); - for idx in 0..TOTAL_DEALINGS as u64 { - let page1 = - query_dealings_paged(deps.as_ref(), idx, None, Option::from(per_page)).unwrap(); - let start_after = page1.start_next_after.unwrap(); - let page2 = query_dealings_paged( - deps.as_ref(), - idx, - Option::from(start_after.to_string()), - Option::from(per_page), - ) - .unwrap(); + // query with a crazily high limit in an attempt to use too many resources + let crazy_limit = 1000 * DEALINGS_PAGE_MAX_LIMIT; + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + let page1 = + query_dealings_paged(deps.as_ref(), 0, dealer, None, Option::from(crazy_limit)) + .unwrap(); - // now we have 2 pages, with 2 results on the second page - assert_eq!(2, page2.dealings.len()); + // we default to a decent sized upper bound instead + let expected_limit = DEALINGS_PAGE_MAX_LIMIT; + assert_eq!(expected_limit, page1.dealings.len() as u32); + } + } + + #[test] + fn dealings_pagination_works() { + let mut deps = init_contract(); + + fill_dealings(deps.as_mut(), 0, 10, 1); + let per_page = 2; + + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + let page1 = + query_dealings_paged(deps.as_ref(), 0, dealer, None, Option::from(per_page)) + .unwrap(); + + // page should have 1 result on it + assert_eq!(1, page1.dealings.len()); + } + + // save another + fill_dealings(deps.as_mut(), 1, 10, 2); + + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + // page1 should have 2 results on it + let page1 = + query_dealings_paged(deps.as_ref(), 1, dealer, None, Option::from(per_page)) + .unwrap(); + assert_eq!(2, page1.dealings.len()); + } + + fill_dealings(deps.as_mut(), 3, 10, 3); + + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + // page1 still has 2 results + let page1 = query_dealings_paged( + deps.as_ref(), + 3, + dealer.clone(), + None, + Option::from(per_page), + ) + .unwrap(); + assert_eq!(2, page1.dealings.len()); + + // retrieving the next page should start after the last key on this page + let start_after = page1.start_next_after.unwrap(); + let page2 = query_dealings_paged( + deps.as_ref(), + 3, + dealer, + Option::from(start_after), + Option::from(per_page), + ) + .unwrap(); + + assert_eq!(1, page2.dealings.len()); + } + + fill_dealings(deps.as_mut(), 4, 10, 4); + + for dealer in 0..10 { + let dealer = format!("dealer{dealer}"); + let page1 = query_dealings_paged( + deps.as_ref(), + 4, + dealer.clone(), + None, + Option::from(per_page), + ) + .unwrap(); + let start_after = page1.start_next_after.unwrap(); + let page2 = query_dealings_paged( + deps.as_ref(), + 4, + dealer, + Option::from(start_after), + Option::from(per_page), + ) + .unwrap(); + + // now we have 2 pages, with 2 results on the second page + assert_eq!(2, page2.dealings.len()); + } } } } diff --git a/contracts/coconut-dkg/src/dealings/storage.rs b/contracts/coconut-dkg/src/dealings/storage.rs index 0cc3180679..2c5cef952a 100644 --- a/contracts/coconut-dkg/src/dealings/storage.rs +++ b/contracts/coconut-dkg/src/dealings/storage.rs @@ -2,9 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use cosmwasm_std::{Addr, Order, Record, StdResult, Storage}; -use cw_storage_plus::{ - range_with_prefix, Bound, Key, KeyDeserialize, Path, Prefix, Prefixer, PrimaryKey, -}; +use cw_storage_plus::{Bound, Key, KeyDeserialize, Path, Prefix, Prefixer, PrimaryKey}; use nym_coconut_dkg_common::types::{ ContractDealing, ContractSafeBytes, DealingIndex, EpochId, PartialContractDealing, }; @@ -27,12 +25,12 @@ pub(crate) struct StoredDealing; impl StoredDealing { const NAMESPACE: &'static [u8] = b"dealing"; - fn deserialize_raw_dealing(kv: Record) -> StdResult { + fn deserialize_dealing_record(kv: Record) -> StdResult<(DealingIndex, ContractDealing)> { let (k, v) = kv; let index = ::from_vec(k)?; let data = ContractSafeBytes(v); - Ok(PartialContractDealing { index, data }) + Ok((index, data)) } fn storage_key( @@ -48,13 +46,13 @@ impl StoredDealing { ) } - fn prefix(prefix: (EpochId, Dealer)) -> Prefix { + fn prefix(prefix: (EpochId, Dealer)) -> Prefix { Prefix::with_deserialization_functions( Self::NAMESPACE, &prefix.prefix(), &[], // explicitly panic to make sure we're never attempting to call an unexpected deserializer on our data - |_, _, _| panic!("attempted to call custom de_fn_kv"), + |_, _, kv| Self::deserialize_dealing_record(kv), |_, _, _| panic!("attempted to call custom de_fn_v"), ) } @@ -94,22 +92,202 @@ impl StoredDealing { storage: &'a dyn Storage, prefix: (EpochId, Dealer), start: Option>, - ) -> Box> + 'a> { - // replicate the behaviour of `Prefix::range_raw` but without the fallible deserialization on the data - // (since we're reading from the storage directly) - // and whilst combining the data on the spot - let prefix = Self::prefix(prefix); + ) -> impl Iterator> + 'a { + Self::prefix(prefix) + .range(storage, start, None, Order::Ascending) + .map(|maybe_record| maybe_record.map(Into::into)) + } - let mapped = range_with_prefix( - storage, - // note: Prefix's Deref implementation gives back its storage_prefix - &prefix, - start.map(|b| b.to_raw_bound()), - None, - Order::Ascending, - ) - .map(Self::deserialize_raw_dealing); + // iterate over all values, only to be used in tests due to the amount of data being returned + #[cfg(test)] + pub(crate) fn unchecked_all_entries( + storage: &dyn Storage, + ) -> Vec<((EpochId, Addr, DealingIndex), ContractDealing)> { + type StorageKey<'a> = (EpochId, Dealer<'a>, DealingIndex); - Box::new(mapped) + let empty_prefix: Prefix = + Prefix::with_deserialization_functions( + Self::NAMESPACE, + &[], + &[], + |_, _, kv| StorageKey::from_vec(kv.0).map(|kt| (kt, ContractSafeBytes(kv.1))), + |_, _, _| unimplemented!(), + ); + + empty_prefix + .range(storage, None, None, Order::Ascending) + .collect::>() + .unwrap() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::support::tests::helpers::init_contract; + use std::collections::HashMap; + + fn dealing_data( + epoch_id: EpochId, + dealer: Dealer, + dealing_index: DealingIndex, + ) -> ContractDealing { + ContractSafeBytes( + format!("{epoch_id},{dealer},{dealing_index}") + .as_bytes() + .to_vec(), + ) + } + + #[test] + fn saving_dealing() { + let mut deps = init_contract(); + + // make sure to check all combinations of epoch id, dealer address and dealing index to ensure nothing overlaps + let epochs = [54, 423, 754]; + let dealers = [ + Addr::unchecked("dealer1"), + Addr::unchecked("dealer2"), + Addr::unchecked("dealer3"), + Addr::unchecked("dealer4"), + Addr::unchecked("dealer5"), + ]; + let dealing_indices = [0, 1, 2, 3, 4, 5, 6, 7]; + + for epoch_id in &epochs { + for dealer in &dealers { + for dealing_index in &dealing_indices { + assert!(!StoredDealing::exists( + &deps.storage, + *epoch_id, + dealer, + *dealing_index + )); + + StoredDealing::save( + deps.as_mut().storage, + *epoch_id, + dealer, + PartialContractDealing { + index: *dealing_index, + data: dealing_data(*epoch_id, dealer, *dealing_index), + }, + ) + } + } + } + + let all: HashMap<_, _> = StoredDealing::unchecked_all_entries(&deps.storage) + .into_iter() + .collect(); + assert_eq!( + all.len(), + epochs.len() * dealers.len() * dealing_indices.len() + ); + + for epoch_id in &epochs { + for dealer in &dealers { + for dealing_index in &dealing_indices { + assert!(StoredDealing::exists( + &deps.storage, + *epoch_id, + dealer, + *dealing_index + )); + + let content = + StoredDealing::read(&deps.storage, *epoch_id, dealer, *dealing_index) + .unwrap(); + let expected = dealing_data(*epoch_id, dealer, *dealing_index); + assert_eq!(expected, content); + assert_eq!( + &expected, + all.get(&(*epoch_id, dealer.clone(), *dealing_index)) + .unwrap() + ); + } + } + } + } + + #[test] + fn iterating_over_dealings() { + let mut deps = init_contract(); + + let epochs = [54, 423, 754]; + let dealers = [ + Addr::unchecked("dealer1"), + Addr::unchecked("dealer2"), + Addr::unchecked("dealer3"), + Addr::unchecked("dealer4"), + Addr::unchecked("dealer5"), + ]; + let dealing_indices = [0, 1, 2, 3, 4, 5, 6, 7]; + + for epoch_id in &epochs { + for dealer in &dealers { + for dealing_index in &dealing_indices { + StoredDealing::save( + deps.as_mut().storage, + *epoch_id, + dealer, + PartialContractDealing { + index: *dealing_index, + data: dealing_data(*epoch_id, dealer, *dealing_index), + }, + ) + } + } + } + + // remember, we're not testing the iterator implementation + + // nothing under epoch 0 + let dealings = + StoredDealing::prefix_range(&deps.storage, (0, &dealers[0]), None).collect::>(); + assert!(dealings.is_empty()); + + // nothing for dealer "foo" + let foo = Addr::unchecked("foo"); + let dealings = + StoredDealing::prefix_range(&deps.storage, (epochs[0], &foo), None).collect::>(); + assert!(dealings.is_empty()); + + let all = StoredDealing::prefix_range(&deps.storage, (epochs[0], &dealers[0]), None) + .collect::>(); + assert_eq!(all.len(), dealing_indices.len()); + + for (i, dealing) in all.iter().enumerate() { + let expected = dealing_data(epochs[0], &dealers[0], dealing_indices[i]); + assert_eq!(expected, dealing.as_ref().unwrap().data); + assert_eq!(dealing_indices[i], dealing.as_ref().unwrap().index); + } + + // for sanity sake, check another dealer with different epoch + let all_other = StoredDealing::prefix_range(&deps.storage, (epochs[2], &dealers[3]), None) + .collect::>(); + assert_eq!(all_other.len(), dealing_indices.len()); + + for (i, dealing) in all_other.iter().enumerate() { + let expected = dealing_data(epochs[2], &dealers[3], dealing_indices[i]); + assert_eq!(expected, dealing.as_ref().unwrap().data); + assert_eq!(dealing_indices[i], dealing.as_ref().unwrap().index); + } + + let without_first = StoredDealing::prefix_range( + &deps.storage, + (epochs[0], &dealers[0]), + Some(Bound::exclusive(dealing_indices[0])), + ) + .collect::>(); + assert_eq!(&all[1..], without_first); + + let mid = StoredDealing::prefix_range( + &deps.storage, + (epochs[0], &dealers[0]), + Some(Bound::inclusive(dealing_indices[3])), + ) + .collect::>(); + assert_eq!(&all[3..], mid); } } diff --git a/contracts/coconut-dkg/src/epoch_state/transactions.rs b/contracts/coconut-dkg/src/epoch_state/transactions.rs index 02b7efc2be..e4002879bd 100644 --- a/contracts/coconut-dkg/src/epoch_state/transactions.rs +++ b/contracts/coconut-dkg/src/epoch_state/transactions.rs @@ -783,43 +783,44 @@ pub(crate) mod tests { let mut deps = init_contract(); let all_details: [_; 100] = std::array::from_fn(|i| dealer_details_fixture(i as u64)); - THRESHOLD.save(deps.as_mut().storage, &42).unwrap(); - for details in all_details.iter() { - current_dealers() - .save(deps.as_mut().storage, &details.address, details) - .unwrap(); - for dealings in DEALINGS_BYTES { - dealings - .save( - deps.as_mut().storage, - &details.address, - &ContractSafeBytes(vec![1, 2, 3]), - ) - .unwrap(); - } - } - - reset_epoch_state(deps.as_mut().storage).unwrap(); - - assert!(THRESHOLD.may_load(&deps.storage).unwrap().is_none()); - for details in all_details { - for dealings in DEALINGS_BYTES { - assert!(dealings - .may_load(&deps.storage, &details.address) - .unwrap() - .is_none()); - } - assert!(current_dealers() - .may_load(deps.as_mut().storage, &details.address) - .unwrap() - .is_none()); - assert_eq!( - past_dealers() - .load(&deps.storage, &details.address) - .unwrap(), - details - ); - } + todo!() + // THRESHOLD.save(deps.as_mut().storage, &42).unwrap(); + // for details in all_details.iter() { + // current_dealers() + // .save(deps.as_mut().storage, &details.address, details) + // .unwrap(); + // for dealings in DEALINGS_BYTES { + // dealings + // .save( + // deps.as_mut().storage, + // &details.address, + // &ContractSafeBytes(vec![1, 2, 3]), + // ) + // .unwrap(); + // } + // } + // + // reset_epoch_state(deps.as_mut().storage).unwrap(); + // + // assert!(THRESHOLD.may_load(&deps.storage).unwrap().is_none()); + // for details in all_details { + // for dealings in DEALINGS_BYTES { + // assert!(dealings + // .may_load(&deps.storage, &details.address) + // .unwrap() + // .is_none()); + // } + // assert!(current_dealers() + // .may_load(deps.as_mut().storage, &details.address) + // .unwrap() + // .is_none()); + // assert_eq!( + // past_dealers() + // .load(&deps.storage, &details.address) + // .unwrap(), + // details + // ); + // } } #[test] diff --git a/contracts/coconut-dkg/src/support/tests/fixtures.rs b/contracts/coconut-dkg/src/support/tests/fixtures.rs index afb84b62cb..2b587eddee 100644 --- a/contracts/coconut-dkg/src/support/tests/fixtures.rs +++ b/contracts/coconut-dkg/src/support/tests/fixtures.rs @@ -20,7 +20,7 @@ pub fn vk_share_fixture(owner: &str, index: u64) -> ContractVKShare { } pub fn dealing_bytes_fixture() -> ContractSafeBytes { - ContractSafeBytes(vec![]) + ContractSafeBytes(vec![1, 2, 3]) } pub fn partial_dealing_fixture() -> PartialContractDealing {