diff --git a/.github/workflows/ci-contracts.yml b/.github/workflows/ci-contracts.yml index 3448eb0834..4a958ecf73 100644 --- a/.github/workflows/ci-contracts.yml +++ b/.github/workflows/ci-contracts.yml @@ -54,7 +54,7 @@ jobs: uses: actions-rs/cargo@v1 with: command: test - args: --lib --manifest-path contracts/Cargo.toml + args: --lib --manifest-path contracts/Cargo.toml --all-features - name: Check formatting uses: actions-rs/cargo@v1 diff --git a/common/client-core/src/init/helpers.rs b/common/client-core/src/init/helpers.rs index e574466701..76dd94deb7 100644 --- a/common/client-core/src/init/helpers.rs +++ b/common/client-core/src/init/helpers.rs @@ -151,7 +151,7 @@ pub async fn gateways_for_init( } let retry_count = retry_count.unwrap_or(DEFAULT_NYM_API_RETRIES); - let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone()) + let mut builder = nym_http_api_client::ClientBuilder::new_with_urls(nym_api_urls.clone())? .with_retries(retry_count) .with_bincode(); diff --git a/common/http-api-client/src/lib.rs b/common/http-api-client/src/lib.rs index ec1f524b50..3baa390c53 100644 --- a/common/http-api-client/src/lib.rs +++ b/common/http-api-client/src/lib.rs @@ -296,6 +296,9 @@ impl std::error::Error for ReqwestErrorWrapper {} #[derive(Debug, Error)] #[allow(missing_docs)] pub enum HttpClientError { + #[error("did not provide any valid client URLs")] + NoUrlsProvided, + #[error("failed to construct inner reqwest client: {source}")] ReqwestBuildError { #[source] @@ -582,25 +585,29 @@ impl ClientBuilder { Self::new(alt) } else { let url = url.to_url()?; - Ok(Self::new_with_urls(vec![url])) + Self::new_with_urls(vec![url]) } } /// Create a client builder from network details with sensible defaults #[cfg(feature = "network-defaults")] + // deprecating function since it's not clear from its signature whether the client + // would be constructed using `nym_api_urls` or `nym_vpn_api_urls` + #[deprecated(note = "use explicit Self::new_with_fronted_urls instead")] pub fn from_network( network: &nym_network_defaults::NymNetworkDetails, ) -> Result { - let api_urls = - network - .nym_api_urls - .as_ref() - .ok_or_else(|| HttpClientError::UrlParseFailure { - source: url::ParseError::EmptyHost, - })?; + let urls = network.nym_api_urls.as_ref().cloned().unwrap_or_default(); + Self::new_with_fronted_urls(urls.clone()) + } - let urls = api_urls - .iter() + /// Create a client builder using the provided set of domain-fronted URLs + #[cfg(feature = "network-defaults")] + pub fn new_with_fronted_urls( + urls: Vec, + ) -> Result { + let urls = urls + .into_iter() .map(|api_url| { // Convert ApiUrl to our Url type with fronting support let mut url = Url::parse(&api_url.url)?; @@ -624,7 +631,7 @@ impl ClientBuilder { }) .collect::, HttpClientError>>()?; - let mut builder = Self::new_with_urls(urls); + let mut builder = Self::new_with_urls(urls)?; // Enable domain fronting by default (on retry) #[cfg(feature = "tunneling")] @@ -636,7 +643,11 @@ impl ClientBuilder { } /// Constructs a new http `ClientBuilder` from a valid url. - pub fn new_with_urls(urls: Vec) -> Self { + pub fn new_with_urls(urls: Vec) -> Result { + if urls.is_empty() { + return Err(HttpClientError::NoUrlsProvided); + } + let urls = Self::check_urls(urls); #[cfg(target_arch = "wasm32")] @@ -645,7 +656,7 @@ impl ClientBuilder { #[cfg(not(target_arch = "wasm32"))] let reqwest_client_builder = default_builder(); - ClientBuilder { + Ok(ClientBuilder { urls, timeout: None, custom_user_agent: false, @@ -656,7 +667,7 @@ impl ClientBuilder { retry_limit: 0, serialization: SerializationFormat::Json, - } + }) } /// Add an additional URL to the set usable by this constructed `Client` diff --git a/common/http-api-client/src/registry.rs b/common/http-api-client/src/registry.rs index 05ad9a5daa..4e09570dd9 100644 --- a/common/http-api-client/src/registry.rs +++ b/common/http-api-client/src/registry.rs @@ -21,6 +21,10 @@ inventory::collect!(ConfigRecord); /// Returns the default builder with all registered configurations applied. pub fn default_builder() -> ReqwestClientBuilder { let mut b = ReqwestClientBuilder::new(); + + #[cfg(feature = "debug-inventory")] + let mut test_client = ReqwestClientBuilder::new(); + let mut records: Vec<&'static ConfigRecord> = inventory::iter::.into_iter().collect(); records.sort_by_key(|r| r.priority); // lower runs first @@ -35,6 +39,10 @@ pub fn default_builder() -> ReqwestClientBuilder { for r in records { b = (r.apply)(b); + #[cfg(feature = "debug-inventory")] + { + test_client = (r.apply)(test_client); + } } #[cfg(feature = "debug-inventory")] @@ -47,7 +55,7 @@ pub fn default_builder() -> ReqwestClientBuilder { eprintln!("[HTTP-INVENTORY] Building test client to verify configuration..."); // Try to build a client to see if it works - match b.try_clone().unwrap().build() { + match test_client.build() { Ok(client) => { eprintln!("[HTTP-INVENTORY] ✓ Client built successfully"); eprintln!("[HTTP-INVENTORY] Client debug info: {:#?}", client); diff --git a/common/http-api-client/src/tests.rs b/common/http-api-client/src/tests.rs index 2e38f6caf6..fffb2eb991 100644 --- a/common/http-api-client/src/tests.rs +++ b/common/http-api-client/src/tests.rs @@ -93,7 +93,7 @@ async fn api_client_retry() -> Result<(), Box> { let client = ClientBuilder::new_with_urls(vec![ "http://broken.nym.test".parse()?, // This will fail "http://httpbin.org/".parse()?, // This will succeed - ]) + ])? .with_retries(3) .build()?; diff --git a/contracts/coconut-dkg/src/dealers/transactions.rs b/contracts/coconut-dkg/src/dealers/transactions.rs index 5bd957dcdc..5d9f8838e4 100644 --- a/contracts/coconut-dkg/src/dealers/transactions.rs +++ b/contracts/coconut-dkg/src/dealers/transactions.rs @@ -9,6 +9,7 @@ use crate::epoch_state::storage::{load_current_epoch, save_epoch}; use crate::epoch_state::utils::check_epoch_state; use crate::error::ContractError; use crate::state::storage::STATE; +use crate::verification_key_shares::storage::vk_shares; use crate::Dealer; use cosmwasm_std::{Deps, DepsMut, Env, Event, MessageInfo, Response}; use nym_coconut_dkg_common::dealer::{DealerRegistrationDetails, OwnershipTransfer}; @@ -109,7 +110,7 @@ pub fn try_transfer_ownership( DEALERS_INDICES.save(deps.storage, &transfer_to, ¤t_index)?; DEALERS_INDICES.remove(deps.storage, &info.sender); - // update registration detail for every epoch the current dealer has participated in the protocol + // update registration detail and share information for every epoch the current dealer has participated in the protocol // ideally, we'd have only updated the current epoch, but the way the contract is constructed // forbids that otherwise we'd have introduced inconsistency for epoch_id in 0..=epoch.epoch_id { @@ -117,6 +118,10 @@ pub fn try_transfer_ownership( EPOCH_DEALERS_MAP.remove(deps.storage, (epoch_id, &info.sender)); EPOCH_DEALERS_MAP.save(deps.storage, (epoch_id, &transfer_to), &details)?; } + if let Some(vk_share) = vk_shares().may_load(deps.storage, (&info.sender, epoch_id))? { + vk_shares().remove(deps.storage, (&info.sender, epoch_id))?; + vk_shares().save(deps.storage, (&transfer_to, epoch_id), &vk_share)?; + } } let Some(transaction_info) = env.transaction else { @@ -161,6 +166,14 @@ pub fn try_update_announce_address( details.announce_address = new_address.clone(); EPOCH_DEALERS_MAP.save(deps.storage, (epoch.epoch_id, &info.sender), &details)?; + let mut contract_share = vk_shares().load(deps.storage, (&info.sender, epoch.epoch_id))?; + contract_share.announce_address = new_address.clone(); + vk_shares().save( + deps.storage, + (&info.sender, epoch.epoch_id), + &contract_share, + )?; + Ok(Response::new().add_event( Event::new("dkg-announce-address-update") .add_attribute("dealer", info.sender) @@ -228,9 +241,14 @@ pub(crate) mod tests { #[cfg(feature = "testable-dkg-contract")] mod tests_with_mock { use super::*; - use crate::testable_dkg_contract::{init_contract_tester, DkgContractTesterExt}; + use crate::testable_dkg_contract::{ + init_contract_tester, init_contract_tester_with_group_members, DkgContractTesterExt, + }; + use anyhow::Context; use cosmwasm_std::testing::message_info; - use nym_contracts_common_testing::ContractOpts; + use nym_coconut_dkg_common::msg::QueryMsg; + use nym_coconut_dkg_common::verification_key::PagedVKSharesResponse; + use nym_contracts_common_testing::{ChainOpts, ContractOpts}; #[test] fn transferring_ownership() -> anyhow::Result<()> { @@ -248,6 +266,7 @@ mod tests_with_mock { contract.run_initial_dummy_dkg(); let old_index = DEALERS_INDICES.load(&contract, &group_member)?; let old_details = EPOCH_DEALERS_MAP.load(&contract, (0, &group_member))?; + let old_share = vk_shares().load(&contract, (&group_member, 0))?; let not_group_member = contract.addr_make("not_group_member"); let (deps, env) = contract.deps_mut_env(); @@ -277,13 +296,18 @@ mod tests_with_mock { assert!(EPOCH_DEALERS_MAP .may_load(&contract, (0, &group_member))? .is_none()); + assert!(vk_shares() + .may_load(&contract, (&group_member, 0))? + .is_none()); let new_index = DEALERS_INDICES.load(&contract, &new_group_member)?; let new_details = EPOCH_DEALERS_MAP.load(&contract, (0, &new_group_member))?; + let new_share = vk_shares().load(&contract, (&new_group_member, 0))?; // the underlying info hasn't changed assert_eq!(old_index, new_index); assert_eq!(old_details, new_details); + assert_eq!(old_share, new_share); assert_eq!( OWNERSHIP_TRANSFER_LOG.load( @@ -436,9 +460,91 @@ mod tests_with_mock { assert_eq!(old_details1, new_details1); assert_eq!(old_details2, new_details2); - // most recent entry is updated + // most recent entry is updated assert_eq!(new_details3.announce_address, new_address); Ok(()) } + + #[test] + fn updating_announce_address_updates_vk_shares() -> anyhow::Result<()> { + let mut contract = init_contract_tester_with_group_members(3); + let group_member = contract.random_group_member(); + + contract.run_initial_dummy_dkg(); // => epoch 0 + contract.run_reset_dkg(); // => epoch 1 + + // LEAVE DKG MEMBERSHIP + contract.remove_group_member(group_member.clone()); + contract.run_reset_dkg(); // => epoch 2 + + // COME BACK + contract.add_group_member(group_member.clone()); + contract.run_reset_dkg(); // => epoch 3 + + let old_address = EPOCH_DEALERS_MAP + .load(&contract, (3, &group_member))? + .announce_address; + + let old_share0 = vk_shares().load(&contract, (&group_member, 0))?; + let old_share1 = vk_shares().load(&contract, (&group_member, 1))?; + let old_share2 = vk_shares().may_load(&contract, (&group_member, 2))?; + assert!(old_share2.is_none()); + let old_share3 = vk_shares().may_load(&contract, (&group_member, 3))?; + assert!(old_share3.is_some()); + + let new_address = "https://new-address.com".to_string(); + try_update_announce_address( + contract.deps_mut(), + message_info(&group_member, &[]), + new_address.clone(), + )?; + + let new_share0 = vk_shares().load(&contract, (&group_member, 0))?; + let new_share1 = vk_shares().load(&contract, (&group_member, 1))?; + let new_share2 = vk_shares().may_load(&contract, (&group_member, 2))?; + assert!(new_share2.is_none()); + let new_share3 = vk_shares().load(&contract, (&group_member, 3))?; + + // old epoch data is unchanged + assert_eq!(old_share0, new_share0); + assert_eq!(old_share1, new_share1); + assert_eq!(old_share2, new_share2); + + // most recent entry is updated + assert_eq!(new_share3.announce_address, new_address); + + // finally an integration check against query endpoint + let epoch0_shares: PagedVKSharesResponse = + contract.query(&QueryMsg::GetVerificationKeys { + epoch_id: 0, + limit: None, + start_after: None, + })?; + assert_eq!(epoch0_shares.shares.len(), 3); + + let member_share = epoch0_shares + .shares + .iter() + .find(|s| s.owner == group_member) + .context("failed to find member's share")?; + assert_eq!(member_share.announce_address, old_address); + + let epoch0_shares: PagedVKSharesResponse = + contract.query(&QueryMsg::GetVerificationKeys { + epoch_id: 3, + limit: None, + start_after: None, + })?; + assert_eq!(epoch0_shares.shares.len(), 3); + + let member_share = epoch0_shares + .shares + .iter() + .find(|s| s.owner == group_member) + .context("failed to find member's share")?; + assert_eq!(member_share.announce_address, new_address); + + Ok(()) + } } diff --git a/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs b/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs index 528498f600..2ceaee2965 100644 --- a/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs +++ b/contracts/coconut-dkg/src/testable_dkg_contract/mod.rs @@ -62,12 +62,18 @@ impl TestableNymContract for DkgContract { where Self: Sized, { - init_contract_tester_with_group_members(DEFAULT_GROUP_MEMBERS) + init_contract_tester() } } pub fn init_contract_tester() -> ContractTester { - DkgContract::init().with_common_storage_key(CommonStorageKeys::Admin, "dkg-admin") + init_contract_tester_with_group_members(DEFAULT_GROUP_MEMBERS) +} + +pub fn init_contract_tester_with_group_members(members: usize) -> ContractTester { + prepare_contract_tester_builder_with_group_members(members) + .build() + .with_common_storage_key(CommonStorageKeys::Admin, "dkg-admin") } pub fn prepare_contract_tester_builder_with_group_members( @@ -137,12 +143,6 @@ where builder } -pub fn init_contract_tester_with_group_members(members: usize) -> ContractTester { - prepare_contract_tester_builder_with_group_members(members) - .build() - .with_common_storage_key(CommonStorageKeys::Admin, "dkg-admin") -} - pub trait DkgContractTesterExt: ContractOpts + ChainOpts diff --git a/contracts/mixnet/src/testing/legacy.rs b/contracts/mixnet/src/testing/legacy.rs deleted file mode 100644 index 38a919fd33..0000000000 --- a/contracts/mixnet/src/testing/legacy.rs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2024 - Nym Technologies SA -// SPDX-License-Identifier: Apache-2.0 - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn legacy_mixnode_bonding() { - todo!() - } -} diff --git a/contracts/mixnet/src/testing/mod.rs b/contracts/mixnet/src/testing/mod.rs index ac99770b50..30de4b3136 100644 --- a/contracts/mixnet/src/testing/mod.rs +++ b/contracts/mixnet/src/testing/mod.rs @@ -2,7 +2,3 @@ // SPDX-License-Identifier: Apache-2.0 pub(crate) mod transactions; - -// the purpose of that module is to keep track of tests of legacy features that will eventually be phased out -// such as standalone mixnode/gateway bonding -pub(crate) mod legacy; diff --git a/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs b/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs index 55bf382308..c89fcc40c3 100644 --- a/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/metrics_scraper/mod.rs @@ -57,7 +57,7 @@ async fn run( .clone() .expect("rust sdk mainnet default missing api_url"); - let nym_api = nym_http_api_client::ClientBuilder::new_with_urls(vec![default_api_url.into()]) + let nym_api = nym_http_api_client::ClientBuilder::new_with_urls(vec![default_api_url.into()])? .no_hickory_dns() .with_timeout(nym_api_client_timeout) .build()?; diff --git a/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs b/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs index 688e644af5..76846d1fb2 100644 --- a/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs +++ b/nym-node-status-api/nym-node-status-api/src/monitor/mod.rs @@ -98,7 +98,7 @@ impl Monitor { .expect("rust sdk mainnet default missing api_url"); let nym_api = - nym_http_api_client::ClientBuilder::new_with_urls(vec![default_api_url.into()]) + nym_http_api_client::ClientBuilder::new_with_urls(vec![default_api_url.into()])? .no_hickory_dns() .with_timeout(self.nym_api_client_timeout) .build()?; diff --git a/nym-wallet/Cargo.lock b/nym-wallet/Cargo.lock index 4603c90e9b..546ee6409d 100644 --- a/nym-wallet/Cargo.lock +++ b/nym-wallet/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "NymWallet" @@ -773,7 +773,7 @@ dependencies = [ [[package]] name = "bls12_381" version = "0.8.0" -source = "git+https://github.com/jstuczyn/bls12_381?branch=temp/experimental-serdect-updated#9bf520059cb28323fc51469cae86868ef4fa6fbd" +source = "git+https://github.com/jstuczyn/bls12_381?branch=temp%2Fexperimental-serdect-updated#9bf520059cb28323fc51469cae86868ef4fa6fbd" dependencies = [ "digest 0.10.7", "ff", @@ -1723,15 +1723,6 @@ dependencies = [ "dirs-sys 0.3.7", ] -[[package]] -name = "dirs" -version = "5.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" -dependencies = [ - "dirs-sys 0.4.1", -] - [[package]] name = "dirs" version = "6.0.0" @@ -1752,18 +1743,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "dirs-sys" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" -dependencies = [ - "libc", - "option-ext", - "redox_users 0.4.6", - "windows-sys 0.48.0", -] - [[package]] name = "dirs-sys" version = "0.5.0" @@ -4100,7 +4079,7 @@ dependencies = [ name = "nym-config" version = "0.1.0" dependencies = [ - "dirs 5.0.1", + "dirs 6.0.0", "handlebars", "log", "nym-network-defaults",