From db36f72200186aa0dbfecf816435322e73a67ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Wed, 3 Jan 2024 10:30:12 +0000 Subject: [PATCH] storing dealings in new map --- .../coconut-dkg/src/dealer.rs | 1 + .../coconut-dkg/src/msg.rs | 7 +- .../coconut-dkg/src/types.rs | 7 + contracts/coconut-dkg/src/contract.rs | 14 +- contracts/coconut-dkg/src/dealings/storage.rs | 39 +++++- .../coconut-dkg/src/dealings/transactions.rs | 123 +++++++++++++----- contracts/coconut-dkg/src/error.rs | 22 +++- contracts/coconut-dkg/src/state/mod.rs | 3 + .../coconut-dkg/src/support/tests/fixtures.rs | 9 +- .../coconut-dkg/src/support/tests/helpers.rs | 3 +- 10 files changed, 183 insertions(+), 45 deletions(-) diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/dealer.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/dealer.rs index 3547345fa1..629e6cbf12 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/dealer.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/dealer.rs @@ -65,6 +65,7 @@ impl PagedDealerResponse { } } +#[deprecated] #[cw_serde] pub struct ContractDealing { pub dealing: ContractSafeBytes, diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs index b986b77f41..3b760d69a4 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/msg.rs @@ -1,7 +1,7 @@ // Copyright 2021 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use crate::types::{ContractSafeBytes, EncodedBTEPublicKeyWithProof, EpochId, TimeConfiguration}; +use crate::types::{PartialContractDealing, EncodedBTEPublicKeyWithProof, EpochId, TimeConfiguration}; use crate::verification_key::VerificationKeyShare; use cosmwasm_schema::cw_serde; use cosmwasm_std::Addr; @@ -21,6 +21,9 @@ pub struct InstantiateMsg { pub multisig_addr: String, pub time_configuration: Option, pub mix_denom: String, + + /// Specifies the number of elements in the derived keys + pub key_size: u32, } #[cw_serde] @@ -32,7 +35,7 @@ pub enum ExecuteMsg { }, CommitDealing { - dealing_bytes: ContractSafeBytes, + dealing: PartialContractDealing, resharing: bool, }, diff --git a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs index 9a68702167..17d610fd40 100644 --- a/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs +++ b/common/cosmwasm-smart-contracts/coconut-dkg/src/types.rs @@ -13,10 +13,17 @@ pub type EncodedBTEPublicKeyWithProof = String; pub type EncodedBTEPublicKeyWithProofRef<'a> = &'a str; pub type NodeIndex = u64; pub type EpochId = u64; +pub type DealingIndex = u32; // 2 public attributes, 2 private attributes, 1 fixed for coconut credential pub const TOTAL_DEALINGS: usize = 2 + 2 + 1; +#[cw_serde] +pub struct PartialContractDealing { + pub index: u32, + pub data: ContractSafeBytes, +} + #[cw_serde] pub struct InitialReplacementData { pub initial_dealers: Vec, diff --git a/contracts/coconut-dkg/src/contract.rs b/contracts/coconut-dkg/src/contract.rs index be9cfc05e2..caf643ab87 100644 --- a/contracts/coconut-dkg/src/contract.rs +++ b/contracts/coconut-dkg/src/contract.rs @@ -39,7 +39,7 @@ pub fn instantiate( let multisig_addr = deps.api.addr_validate(&msg.multisig_addr)?; MULTISIG.set(deps.branch(), Some(multisig_addr.clone()))?; - let group_addr = Cw4Contract(deps.api.addr_validate(&msg.group_addr).map_err(|_| { + let group_addr = Cw4Contract::new(deps.api.addr_validate(&msg.group_addr).map_err(|_| { ContractError::InvalidGroup { addr: msg.group_addr.clone(), } @@ -49,6 +49,7 @@ pub fn instantiate( group_addr, multisig_addr, mix_denom: msg.mix_denom, + key_size: msg.key_size, }; STATE.save(deps.storage, &state)?; @@ -79,10 +80,9 @@ pub fn execute( announce_address, resharing, } => try_add_dealer(deps, info, bte_key_with_proof, announce_address, resharing), - ExecuteMsg::CommitDealing { - dealing_bytes, - resharing, - } => try_commit_dealings(deps, info, dealing_bytes, resharing), + ExecuteMsg::CommitDealing { dealing, resharing } => { + try_commit_dealings(deps, info, dealing, resharing) + } ExecuteMsg::CommitVerificationKeyShare { share, resharing } => { try_commit_verification_key_share(deps, env, info, share, resharing) } @@ -141,7 +141,7 @@ mod tests { use cw4::Member; use cw_multi_test::{App, AppBuilder, AppResponse, ContractWrapper, Executor}; use nym_coconut_dkg_common::msg::ExecuteMsg::RegisterDealer; - use nym_coconut_dkg_common::types::NodeIndex; + use nym_coconut_dkg_common::types::{NodeIndex, TOTAL_DEALINGS}; use nym_group_contract_common::msg::InstantiateMsg as GroupInstantiateMsg; fn instantiate_with_group(app: &mut App, members: &[Addr]) -> Addr { @@ -178,6 +178,7 @@ mod tests { multisig_addr: MULTISIG_CONTRACT.to_string(), time_configuration: None, mix_denom: TEST_MIX_DENOM.to_string(), + key_size: TOTAL_DEALINGS as u32, }; app.instantiate_contract( coconut_dkg_code_id, @@ -213,6 +214,7 @@ mod tests { multisig_addr: "multisig_addr".to_string(), time_configuration: None, mix_denom: "nym".to_string(), + key_size: 5, }; let info = mock_info("creator", &[]); diff --git a/contracts/coconut-dkg/src/dealings/storage.rs b/contracts/coconut-dkg/src/dealings/storage.rs index 0395fa8c74..521ccd89be 100644 --- a/contracts/coconut-dkg/src/dealings/storage.rs +++ b/contracts/coconut-dkg/src/dealings/storage.rs @@ -1,15 +1,50 @@ // Copyright 2022 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use cosmwasm_std::Addr; +use crate::error::ContractError; +use cosmwasm_std::{Addr, Storage}; use cw_storage_plus::Map; -use nym_coconut_dkg_common::types::{ContractSafeBytes, TOTAL_DEALINGS}; +use nym_coconut_dkg_common::types::{ + ContractSafeBytes, DealingIndex, EpochId, PartialContractDealing, TOTAL_DEALINGS, +}; pub(crate) const DEALINGS_PAGE_MAX_LIMIT: u32 = 2; pub(crate) const DEALINGS_PAGE_DEFAULT_LIMIT: u32 = 1; +type Dealer<'a> = &'a Addr; + +#[deprecated] type DealingKey<'a> = &'a Addr; +// dealings are stored in a multilevel map with the following hierarchy: +// - epoch-id: +// - issuer-address: +// - dealing id: +// - dealing content +// NOTE: we're storing raw bytes bypassing serialization, so make sure you always use the below methods for using the storage! +pub(crate) const DEALINGS: Map<(EpochId, Dealer, DealingIndex), ContractSafeBytes> = + Map::new("dealing"); + +pub fn has_committed_dealing( + storage: &dyn Storage, + epoch_id: EpochId, + dealer: &Addr, + dealing_index: DealingIndex, +) -> bool { + DEALINGS.has(storage, (epoch_id, dealer, dealing_index)) +} + +pub fn save_dealing( + storage: &mut dyn Storage, + epoch_id: EpochId, + dealer: &Addr, + dealing: PartialContractDealing, +) { + // NOTE: we're storing bytes directly here! + let storage_key = DEALINGS.key((epoch_id, dealer, dealing.index)); + storage.set(&storage_key, dealing.data.as_slice()); +} + // Note to whoever is looking at this implementation and is thinking of using something similar // for storing small commitments/hashes of data on chain: // If there's a lot of entries you want to store thinking, "oh, this digest is only 32 bytes, it's not that much", diff --git a/contracts/coconut-dkg/src/dealings/transactions.rs b/contracts/coconut-dkg/src/dealings/transactions.rs index 8a592073fd..a68eb2a0b2 100644 --- a/contracts/coconut-dkg/src/dealings/transactions.rs +++ b/contracts/coconut-dkg/src/dealings/transactions.rs @@ -2,17 +2,18 @@ // SPDX-License-Identifier: Apache-2.0 use crate::dealers::storage as dealers_storage; -use crate::dealings::storage::DEALINGS_BYTES; -use crate::epoch_state::storage::INITIAL_REPLACEMENT_DATA; +use crate::dealings::storage::{has_committed_dealing, save_dealing}; +use crate::epoch_state::storage::{CURRENT_EPOCH, INITIAL_REPLACEMENT_DATA}; use crate::epoch_state::utils::check_epoch_state; use crate::error::ContractError; +use crate::state::STATE; use cosmwasm_std::{DepsMut, MessageInfo, Response}; -use nym_coconut_dkg_common::types::{ContractSafeBytes, EpochState}; +use nym_coconut_dkg_common::types::{EpochState, PartialContractDealing}; pub fn try_commit_dealings( deps: DepsMut<'_>, info: MessageInfo, - dealing_bytes: ContractSafeBytes, + dealing: PartialContractDealing, resharing: bool, ) -> Result { check_epoch_state(deps.storage, EpochState::DealingExchange { resharing })?; @@ -32,18 +33,32 @@ pub fn try_commit_dealings( return Err(ContractError::NotAnInitialDealer); } - // check if this dealer has already committed to all dealings - // (we don't want to allow overwriting anything) - for dealings in DEALINGS_BYTES { - if !dealings.has(deps.storage, &info.sender) { - dealings.save(deps.storage, &info.sender, &dealing_bytes)?; - return Ok(Response::default()); - } + let state = STATE.load(deps.storage)?; + let epoch = CURRENT_EPOCH.load(deps.storage)?; + + // check if the index is in range without doing expensive storage reads + // note: dealing indexing starts from 0 + if dealing.index >= state.key_size { + return Err(ContractError::DealingOutOfRange { + epoch_id: epoch.epoch_id, + dealer: info.sender, + index: dealing.index, + key_size: state.key_size, + }); } - Err(ContractError::AlreadyCommitted { - commitment: String::from("dealing"), - }) + // check if this dealer has already committed this particular dealing + if has_committed_dealing(deps.storage, epoch.epoch_id, &info.sender, dealing.index) { + return Err(ContractError::DealingAlreadyCommitted { + epoch_id: epoch.epoch_id, + dealer: info.sender, + index: dealing.index, + }); + } + + save_dealing(deps.storage, epoch.epoch_id, &info.sender, dealing); + + Ok(Response::new()) } #[cfg(test)] @@ -51,13 +66,15 @@ pub(crate) mod tests { use super::*; use crate::epoch_state::storage::CURRENT_EPOCH; use crate::epoch_state::transactions::advance_epoch_state; - use crate::support::tests::fixtures::{dealer_details_fixture, dealing_bytes_fixture}; + use crate::support::tests::fixtures::{dealer_details_fixture, partial_dealing_fixture}; use crate::support::tests::helpers; use crate::support::tests::helpers::add_fixture_dealer; use cosmwasm_std::testing::{mock_env, mock_info}; use cosmwasm_std::Addr; use nym_coconut_dkg_common::dealer::DealerDetails; - use nym_coconut_dkg_common::types::{InitialReplacementData, TimeConfiguration}; + use nym_coconut_dkg_common::types::{ + ContractSafeBytes, InitialReplacementData, TimeConfiguration, TOTAL_DEALINGS, + }; #[test] fn invalid_commit_dealing() { @@ -65,10 +82,10 @@ pub(crate) mod tests { let owner = Addr::unchecked("owner1"); let mut env = mock_env(); let info = mock_info(owner.as_str(), &[]); - let dealing_bytes = dealing_bytes_fixture(); + let dealing = partial_dealing_fixture(); - let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing_bytes.clone(), false) - .unwrap_err(); + let ret = + try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), false).unwrap_err(); assert_eq!( ret, ContractError::IncorrectEpochState { @@ -84,8 +101,8 @@ pub(crate) mod tests { add_fixture_dealer(deps.as_mut()); advance_epoch_state(deps.as_mut(), env).unwrap(); - let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing_bytes.clone(), false) - .unwrap_err(); + let ret = + try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), false).unwrap_err(); assert_eq!(ret, ContractError::NotADealer); let dealer_details = DealerDetails { @@ -114,8 +131,8 @@ pub(crate) mod tests { }, ) .unwrap(); - let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing_bytes.clone(), true) - .unwrap_err(); + let ret = + try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), true).unwrap_err(); assert_eq!(ret, ContractError::NotAnInitialDealer); INITIAL_REPLACEMENT_DATA @@ -125,18 +142,60 @@ pub(crate) mod tests { }) .unwrap(); - for dealings in DEALINGS_BYTES { - assert!(!dealings.has(deps.as_mut().storage, &owner)); - let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing_bytes.clone(), true); - assert!(ret.is_ok()); - assert!(dealings.has(deps.as_mut().storage, &owner)); - } - let ret = try_commit_dealings(deps.as_mut(), info, dealing_bytes, true).unwrap_err(); + // back to 'normal' mode + CURRENT_EPOCH + .update::<_, ContractError>(deps.as_mut().storage, |mut epoch| { + epoch.state = EpochState::DealingExchange { resharing: false }; + Ok(epoch) + }) + .unwrap(); + + // dealing out of range + let ret = try_commit_dealings( + deps.as_mut(), + info.clone(), + PartialContractDealing { + index: 42, + data: ContractSafeBytes(vec![1, 2, 3]), + }, + false, + ) + .unwrap_err(); assert_eq!( ret, - ContractError::AlreadyCommitted { - commitment: String::from("dealing"), + ContractError::DealingOutOfRange { + epoch_id: 0, + dealer: info.sender.clone(), + index: 42, + key_size: TOTAL_DEALINGS as u32, } ); + + // 'good' dealing + let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), false); + assert!(ret.is_ok()); + + // duplicate dealing + let ret = + try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), false).unwrap_err(); + assert_eq!( + ret, + ContractError::DealingAlreadyCommitted { + epoch_id: 0, + dealer: info.sender.clone(), + index: 0, + } + ); + + // same index, but next epoch + CURRENT_EPOCH + .update::<_, ContractError>(deps.as_mut().storage, |mut epoch| { + epoch.epoch_id += 1; + Ok(epoch) + }) + .unwrap(); + + let ret = try_commit_dealings(deps.as_mut(), info.clone(), dealing.clone(), false); + assert!(ret.is_ok()); } } diff --git a/contracts/coconut-dkg/src/error.rs b/contracts/coconut-dkg/src/error.rs index f8458d0c47..3a00ba4b62 100644 --- a/contracts/coconut-dkg/src/error.rs +++ b/contracts/coconut-dkg/src/error.rs @@ -1,8 +1,9 @@ // Copyright 2022 - Nym Technologies SA // SPDX-License-Identifier: Apache-2.0 -use cosmwasm_std::StdError; +use cosmwasm_std::{Addr, StdError}; use cw_controllers::AdminError; +use nym_coconut_dkg_common::types::{DealingIndex, EpochId}; use thiserror::Error; /// Custom errors for contract failure conditions. @@ -43,6 +44,25 @@ pub enum ContractError { #[error("This sender is not a dealer for the current resharing epoch")] NotAnInitialDealer, + #[error( + "Dealer {dealer} has already committed dealing for epoch {epoch_id} with index {index}" + )] + DealingAlreadyCommitted { + epoch_id: EpochId, + dealer: Addr, + index: DealingIndex, + }, + + #[error( + "Dealer {dealer} has attempted to commit dealing for epoch {epoch_id} with index {index} while the key size is set to {key_size}" + )] + DealingOutOfRange { + epoch_id: EpochId, + dealer: Addr, + index: DealingIndex, + key_size: u32, + }, + #[error("This dealer has already committed {commitment}")] AlreadyCommitted { commitment: String }, diff --git a/contracts/coconut-dkg/src/state/mod.rs b/contracts/coconut-dkg/src/state/mod.rs index 90abdc3711..bb829e0eb0 100644 --- a/contracts/coconut-dkg/src/state/mod.rs +++ b/contracts/coconut-dkg/src/state/mod.rs @@ -16,4 +16,7 @@ pub struct State { pub mix_denom: String, pub multisig_addr: Addr, pub group_addr: Cw4Contract, + + /// Specifies the number of elements in the derived keys + pub key_size: u32, } diff --git a/contracts/coconut-dkg/src/support/tests/fixtures.rs b/contracts/coconut-dkg/src/support/tests/fixtures.rs index 1771ee1310..afb84b62cb 100644 --- a/contracts/coconut-dkg/src/support/tests/fixtures.rs +++ b/contracts/coconut-dkg/src/support/tests/fixtures.rs @@ -3,7 +3,7 @@ use cosmwasm_std::Addr; use nym_coconut_dkg_common::dealer::DealerDetails; -use nym_coconut_dkg_common::types::ContractSafeBytes; +use nym_coconut_dkg_common::types::{ContractSafeBytes, PartialContractDealing}; use nym_coconut_dkg_common::verification_key::ContractVKShare; pub const TEST_MIX_DENOM: &str = "unym"; @@ -23,6 +23,13 @@ pub fn dealing_bytes_fixture() -> ContractSafeBytes { ContractSafeBytes(vec![]) } +pub fn partial_dealing_fixture() -> PartialContractDealing { + PartialContractDealing { + index: 0, + data: ContractSafeBytes(vec![1, 2, 3]), + } +} + pub fn dealer_details_fixture(assigned_index: u64) -> DealerDetails { DealerDetails { address: Addr::unchecked(format!("owner{}", assigned_index)), diff --git a/contracts/coconut-dkg/src/support/tests/helpers.rs b/contracts/coconut-dkg/src/support/tests/helpers.rs index 8bb21d5c40..698b2f5691 100644 --- a/contracts/coconut-dkg/src/support/tests/helpers.rs +++ b/contracts/coconut-dkg/src/support/tests/helpers.rs @@ -11,7 +11,7 @@ use cosmwasm_std::{ use cw4::{Cw4QueryMsg, Member, MemberListResponse, MemberResponse}; use lazy_static::lazy_static; use nym_coconut_dkg_common::msg::InstantiateMsg; -use nym_coconut_dkg_common::types::DealerDetails; +use nym_coconut_dkg_common::types::{DealerDetails, TOTAL_DEALINGS}; use std::sync::Mutex; use super::fixtures::TEST_MIX_DENOM; @@ -87,6 +87,7 @@ pub fn init_contract() -> OwnedDeps> multisig_addr: String::from(MULTISIG_CONTRACT), time_configuration: None, mix_denom: TEST_MIX_DENOM.to_string(), + key_size: TOTAL_DEALINGS as u32, }; let env = mock_env(); let info = mock_info(ADMIN_ADDRESS, &[]);