From 24177fac262fd403cc198f107ac21ffb67b5d85e Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Mon, 4 Nov 2024 18:12:14 +0700 Subject: [PATCH 01/15] feat: hardcoded identity transfers in strategy tests --- .../tests/strategy_tests/main.rs | 7 +- .../tests/strategy_tests/strategy.rs | 14 ++- .../tests/strategy_tests/voting_tests.rs | 90 +++++++++++-------- packages/strategy-tests/src/lib.rs | 71 ++++++++++----- packages/strategy-tests/src/operations.rs | 19 +++- packages/strategy-tests/src/transitions.rs | 2 +- 6 files changed, 137 insertions(+), 66 deletions(-) diff --git a/packages/rs-drive-abci/tests/strategy_tests/main.rs b/packages/rs-drive-abci/tests/strategy_tests/main.rs index 2312241cc64..f2122d627ef 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/main.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/main.rs @@ -2602,7 +2602,10 @@ mod tests { &simple_signer, &mut rng, platform_version, - ); + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let strategy = NetworkStrategy { strategy: Strategy { @@ -3910,7 +3913,7 @@ mod tests { strategy: Strategy { start_contracts: vec![], operations: vec![Operation { - op_type: OperationType::IdentityTransfer, + op_type: OperationType::IdentityTransfer(None), frequency: Frequency { times_per_block_range: 1..3, chance_per_block: None, diff --git a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs index 667b8468688..4d1a7ccb624 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs @@ -41,7 +41,7 @@ use drive_abci::rpc::core::MockCoreRPCLike; use rand::prelude::{IteratorRandom, SliceRandom, StdRng}; use rand::Rng; use strategy_tests::Strategy; -use strategy_tests::transitions::{create_state_transitions_for_identities, create_state_transitions_for_identities_and_proofs, instant_asset_lock_proof_fixture, instant_asset_lock_proof_fixture_with_dynamic_range}; +use strategy_tests::transitions::{create_state_transitions_for_identities, create_state_transitions_for_identities_and_proofs, instant_asset_lock_proof_fixture_with_dynamic_range}; use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; use std::ops::RangeInclusive; @@ -405,7 +405,15 @@ impl NetworkStrategy { state_transitions.append(&mut new_transitions); } if !self.strategy.start_identities.hard_coded.is_empty() { - state_transitions.extend(self.strategy.start_identities.hard_coded.clone()); + state_transitions.extend( + self.strategy.start_identities.hard_coded.iter().filter_map( + |(identity, transition)| { + transition.as_ref().map(|create_transition| { + (identity.clone(), create_transition.clone()) + }) + }, + ), + ); } } let frequency = &self.strategy.identity_inserts.frequency; @@ -1196,7 +1204,7 @@ impl NetworkStrategy { operations.push(state_transition); } } - OperationType::IdentityTransfer if current_identities.len() > 1 => { + OperationType::IdentityTransfer(_) if current_identities.len() > 1 => { let identities_clone = current_identities.clone(); // Sender is the first in the list, which should be loaded_identity diff --git a/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs index e14f8d7b1b0..2264a4cd5fb 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs @@ -79,13 +79,17 @@ mod tests { simple_signer.add_keys(keys1); - let start_identities = create_state_transitions_for_identities( - vec![identity1], - &(dash_to_duffs!(1)..=dash_to_duffs!(1)), - &simple_signer, - &mut rng, - platform_version, - ); + let start_identities: Vec<(Identity, Option)> = + create_state_transitions_for_identities( + vec![identity1], + &(dash_to_duffs!(1)..=dash_to_duffs!(1)), + &simple_signer, + &mut rng, + platform_version, + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let dpns_contract = platform .drive @@ -363,13 +367,17 @@ mod tests { simple_signer.add_keys(keys2); - let start_identities = create_state_transitions_for_identities( - vec![identity1, identity2], - &(dash_to_duffs!(1)..=dash_to_duffs!(1)), - &simple_signer, - &mut rng, - platform_version, - ); + let start_identities: Vec<(Identity, Option)> = + create_state_transitions_for_identities( + vec![identity1, identity2], + &(dash_to_duffs!(1)..=dash_to_duffs!(1)), + &simple_signer, + &mut rng, + platform_version, + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let dpns_contract = platform .drive @@ -635,13 +643,17 @@ mod tests { simple_signer.add_keys(keys2); - let start_identities = create_state_transitions_for_identities( - vec![identity1, identity2], - &(dash_to_duffs!(1)..=dash_to_duffs!(1)), - &simple_signer, - &mut rng, - platform_version, - ); + let start_identities: Vec<(Identity, Option)> = + create_state_transitions_for_identities( + vec![identity1, identity2], + &(dash_to_duffs!(1)..=dash_to_duffs!(1)), + &simple_signer, + &mut rng, + platform_version, + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let dpns_contract = platform .drive @@ -988,13 +1000,17 @@ mod tests { simple_signer.add_keys(keys2); - let start_identities = create_state_transitions_for_identities( - vec![identity1, identity2], - &(dash_to_duffs!(1)..=dash_to_duffs!(1)), - &simple_signer, - &mut rng, - platform_version, - ); + let start_identities: Vec<(Identity, Option)> = + create_state_transitions_for_identities( + vec![identity1, identity2], + &(dash_to_duffs!(1)..=dash_to_duffs!(1)), + &simple_signer, + &mut rng, + platform_version, + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let dpns_contract = platform .drive @@ -1353,13 +1369,17 @@ mod tests { simple_signer.add_keys(keys2); - let start_identities = create_state_transitions_for_identities( - vec![identity1, identity2], - &(dash_to_duffs!(1)..=dash_to_duffs!(1)), - &simple_signer, - &mut rng, - platform_version, - ); + let start_identities: Vec<(Identity, Option)> = + create_state_transitions_for_identities( + vec![identity1, identity2], + &(dash_to_duffs!(1)..=dash_to_duffs!(1)), + &simple_signer, + &mut rng, + platform_version, + ) + .iter() + .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .collect(); let dpns_contract = platform .drive diff --git a/packages/strategy-tests/src/lib.rs b/packages/strategy-tests/src/lib.rs index 61395d99f2a..65e8a51f851 100644 --- a/packages/strategy-tests/src/lib.rs +++ b/packages/strategy-tests/src/lib.rs @@ -44,6 +44,7 @@ use platform_version::TryFromPlatformVersioned; use rand::prelude::StdRng; use rand::seq::{IteratorRandom, SliceRandom}; use rand::Rng; +use transitions::create_identity_credit_transfer_transition; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::ops::RangeInclusive; use bincode::{Decode, Encode}; @@ -146,7 +147,7 @@ pub struct StartIdentities { pub keys_per_identity: u8, pub starting_balances: u64, // starting balance in duffs pub extra_keys: KeyMaps, - pub hard_coded: Vec<(Identity, StateTransition)>, + pub hard_coded: Vec<(Identity, Option)>, } /// Identities to register on the first block of the strategy @@ -1287,38 +1288,66 @@ impl Strategy { } // Generate state transition for identity transfer operation - OperationType::IdentityTransfer if current_identities.len() > 1 => { + OperationType::IdentityTransfer(identity_transfer_info) => { for _ in 0..count { - let identities_count = current_identities.len(); - if identities_count == 0 { - break; - } + if let Some(transfer_info) = identity_transfer_info { + let sender = self + .start_identities + .hard_coded + .iter() + .find(|(identity, _)| identity.id() == transfer_info.from) + .expect( + "Expected to find sender identity in hardcoded start identities", + ); + let recipient = self + .start_identities + .hard_coded + .iter() + .find(|(identity, _)| identity.id() == transfer_info.to) + .expect( + "Expected to find recipient identity in hardcoded start identities", + ); + + let state_transition = create_identity_credit_transfer_transition( + &sender.0, + &recipient.0, + identity_nonce_counter, + signer, // Does this mean the loaded identity must be the sender since we're signing with it? + transfer_info.amount, + ); + operations.push(state_transition); + } else if current_identities.len() > 1 { + let identities_count = current_identities.len(); + if identities_count == 0 { + break; + } - // Select a random identity from the current_identities for the sender - let random_index_sender = rng.gen_range(0..identities_count); + // Select a random identity from the current_identities for the sender + let random_index_sender = rng.gen_range(0..identities_count); - // Clone current_identities to a Vec for manipulation - let mut unused_identities: Vec<_> = - current_identities.iter().cloned().collect(); - unused_identities.remove(random_index_sender); // Remove the sender - let unused_identities_count = unused_identities.len(); + // Clone current_identities to a Vec for manipulation + let mut unused_identities: Vec<_> = + current_identities.iter().cloned().collect(); + unused_identities.remove(random_index_sender); // Remove the sender + let unused_identities_count = unused_identities.len(); - // Select a random identity from the remaining ones for the recipient - let random_index_recipient = rng.gen_range(0..unused_identities_count); - let recipient = &unused_identities[random_index_recipient]; + // Select a random identity from the remaining ones for the recipient + let random_index_recipient = + rng.gen_range(0..unused_identities_count); + let recipient = &unused_identities[random_index_recipient]; - // Use the sender index on the original slice - let sender = &mut current_identities[random_index_sender]; + // Use the sender index on the original slice + let sender = &mut current_identities[random_index_sender]; - let state_transition = - crate::transitions::create_identity_credit_transfer_transition( + let state_transition = create_identity_credit_transfer_transition( sender, recipient, identity_nonce_counter, signer, 300000, ); - operations.push(state_transition); + operations.push(state_transition); + } } } diff --git a/packages/strategy-tests/src/operations.rs b/packages/strategy-tests/src/operations.rs index 675e9968433..d35fc9f5036 100644 --- a/packages/strategy-tests/src/operations.rs +++ b/packages/strategy-tests/src/operations.rs @@ -497,6 +497,13 @@ impl VoteAction { pub type AmountRange = RangeInclusive; +#[derive(Clone, Debug, PartialEq, Encode, Decode)] +pub struct IdentityTransferInfo { + pub from: Identifier, + pub to: Identifier, + pub amount: Credits, +} + #[derive(Clone, Debug, PartialEq)] pub enum OperationType { Document(DocumentOp), @@ -505,7 +512,7 @@ pub enum OperationType { IdentityWithdrawal(AmountRange), ContractCreate(RandomDocumentTypeParameters, DocumentTypeCount), ContractUpdate(DataContractUpdateOp), - IdentityTransfer, + IdentityTransfer(Option), ResourceVote(ResourceVoteOp), } @@ -517,7 +524,7 @@ enum OperationTypeInSerializationFormat { IdentityWithdrawal(AmountRange), ContractCreate(RandomDocumentTypeParameters, DocumentTypeCount), ContractUpdate(Vec), - IdentityTransfer, + IdentityTransfer(Option), ResourceVote(ResourceVoteOpSerializable), } @@ -563,7 +570,9 @@ impl PlatformSerializableWithPlatformVersion for OperationType { contract_op_in_serialization_format, ) } - OperationType::IdentityTransfer => OperationTypeInSerializationFormat::IdentityTransfer, + OperationType::IdentityTransfer(identity_transfer_info) => { + OperationTypeInSerializationFormat::IdentityTransfer(identity_transfer_info) + } OperationType::ResourceVote(resource_vote_op) => { let vote_op_in_serialization_format = resource_vote_op.try_into_platform_versioned(platform_version)?; @@ -626,7 +635,9 @@ impl PlatformDeserializableWithPotentialValidationFromVersionedStructure for Ope )?; OperationType::ContractUpdate(update_op) } - OperationTypeInSerializationFormat::IdentityTransfer => OperationType::IdentityTransfer, + OperationTypeInSerializationFormat::IdentityTransfer(identity_transfer_info) => { + OperationType::IdentityTransfer(identity_transfer_info) + } OperationTypeInSerializationFormat::ResourceVote(resource_vote_op) => { let vote_op = resource_vote_op.try_into_platform_versioned(platform_version)?; OperationType::ResourceVote(vote_op) diff --git a/packages/strategy-tests/src/transitions.rs b/packages/strategy-tests/src/transitions.rs index 85d03eb333d..c77b51e2903 100644 --- a/packages/strategy-tests/src/transitions.rs +++ b/packages/strategy-tests/src/transitions.rs @@ -802,7 +802,7 @@ pub fn create_identity_withdrawal_transition_with_output_address( /// - If the sender's identity does not have a suitable authentication key available for signing. /// - If there's an error during the signing process. pub fn create_identity_credit_transfer_transition( - identity: &mut Identity, + identity: &Identity, recipient: &Identity, identity_nonce_counter: &mut BTreeMap, signer: &mut SimpleSigner, From 99fe5fa402ddcae329795bb29eaf73fd5044b4f2 Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Mon, 4 Nov 2024 23:16:36 +0700 Subject: [PATCH 02/15] add comment --- packages/strategy-tests/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/strategy-tests/src/lib.rs b/packages/strategy-tests/src/lib.rs index 65e8a51f851..c2d81ea9905 100644 --- a/packages/strategy-tests/src/lib.rs +++ b/packages/strategy-tests/src/lib.rs @@ -1312,7 +1312,7 @@ impl Strategy { &sender.0, &recipient.0, identity_nonce_counter, - signer, // Does this mean the loaded identity must be the sender since we're signing with it? + signer, // This means the TUI loaded identity must always be the sender since we're always signing with it for now transfer_info.amount, ); operations.push(state_transition); From 0d3e091a5591fde03dbfd8e501864b5ffa2e3602 Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Tue, 5 Nov 2024 13:07:49 +0700 Subject: [PATCH 03/15] comment --- packages/rs-drive-abci/tests/strategy_tests/strategy.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs index 4d1a7ccb624..bf3235ea788 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/strategy.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/strategy.rs @@ -404,6 +404,8 @@ impl NetworkStrategy { ); state_transitions.append(&mut new_transitions); } + // Extend the state transitions with the strategy's hard coded start identities + // Filtering out the ones that have no create transition if !self.strategy.start_identities.hard_coded.is_empty() { state_transitions.extend( self.strategy.start_identities.hard_coded.iter().filter_map( From e4215145ad7889300810472c5a91f15ae987c1eb Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Tue, 5 Nov 2024 13:16:38 +0700 Subject: [PATCH 04/15] use into_iter instead of iter --- .../tests/strategy_tests/main.rs | 4 ++-- .../tests/strategy_tests/voting_tests.rs | 20 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/rs-drive-abci/tests/strategy_tests/main.rs b/packages/rs-drive-abci/tests/strategy_tests/main.rs index f2122d627ef..03bb92bc1ad 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/main.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/main.rs @@ -2603,8 +2603,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let strategy = NetworkStrategy { diff --git a/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs b/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs index 2264a4cd5fb..83834520c0c 100644 --- a/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs +++ b/packages/rs-drive-abci/tests/strategy_tests/voting_tests.rs @@ -87,8 +87,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let dpns_contract = platform @@ -375,8 +375,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let dpns_contract = platform @@ -651,8 +651,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let dpns_contract = platform @@ -1008,8 +1008,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let dpns_contract = platform @@ -1377,8 +1377,8 @@ mod tests { &mut rng, platform_version, ) - .iter() - .map(|(identity, transition)| (identity.clone(), Some(transition.clone()))) + .into_iter() + .map(|(identity, transition)| (identity, Some(transition))) .collect(); let dpns_contract = platform From 3d941ec5ab7f947e53d41c68eb5758ff2e0bd074 Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Tue, 5 Nov 2024 13:31:07 +0700 Subject: [PATCH 05/15] use current identities instead of hardcoded start identities --- packages/strategy-tests/src/lib.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/strategy-tests/src/lib.rs b/packages/strategy-tests/src/lib.rs index c2d81ea9905..efdb702a482 100644 --- a/packages/strategy-tests/src/lib.rs +++ b/packages/strategy-tests/src/lib.rs @@ -1290,33 +1290,32 @@ impl Strategy { // Generate state transition for identity transfer operation OperationType::IdentityTransfer(identity_transfer_info) => { for _ in 0..count { + // Handle the case where specific sender, recipient, and amount are provided if let Some(transfer_info) = identity_transfer_info { - let sender = self - .start_identities - .hard_coded + let sender = current_identities .iter() - .find(|(identity, _)| identity.id() == transfer_info.from) + .find(|identity| identity.id() == transfer_info.from) .expect( "Expected to find sender identity in hardcoded start identities", ); - let recipient = self - .start_identities - .hard_coded + let recipient = current_identities .iter() - .find(|(identity, _)| identity.id() == transfer_info.to) + .find(|identity| identity.id() == transfer_info.to) .expect( "Expected to find recipient identity in hardcoded start identities", ); let state_transition = create_identity_credit_transfer_transition( - &sender.0, - &recipient.0, + &sender, + &recipient, identity_nonce_counter, - signer, // This means the TUI loaded identity must always be the sender since we're always signing with it for now + signer, // This means in the TUI, the loaded identity must always be the sender since we're always signing with it for now transfer_info.amount, ); operations.push(state_transition); } else if current_identities.len() > 1 { + // Handle the case where no sender, recipient, and amount are provided + let identities_count = current_identities.len(); if identities_count == 0 { break; From 4bc0a653de328adacae4e8e85ab6971c42984786 Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Tue, 5 Nov 2024 16:19:48 +0700 Subject: [PATCH 06/15] let transfer keys be any security level or key type --- packages/strategy-tests/src/transitions.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/strategy-tests/src/transitions.rs b/packages/strategy-tests/src/transitions.rs index c77b51e2903..a9cb113d83c 100644 --- a/packages/strategy-tests/src/transitions.rs +++ b/packages/strategy-tests/src/transitions.rs @@ -824,8 +824,8 @@ pub fn create_identity_credit_transfer_transition( let identity_public_key = identity .get_first_public_key_matching( Purpose::TRANSFER, - HashSet::from([SecurityLevel::CRITICAL]), - HashSet::from([KeyType::ECDSA_SECP256K1, KeyType::BLS12_381]), + SecurityLevel::full_range().into(), + KeyType::all_key_types().into(), false, ) .expect("expected to get a signing key"); From dc4882725f6ed6f7a6e4bc8835d84d95bd4ec41f Mon Sep 17 00:00:00 2001 From: pauldelucia Date: Tue, 5 Nov 2024 17:44:37 +0700 Subject: [PATCH 07/15] fix --- packages/strategy-tests/src/transitions.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/strategy-tests/src/transitions.rs b/packages/strategy-tests/src/transitions.rs index a9cb113d83c..c77b51e2903 100644 --- a/packages/strategy-tests/src/transitions.rs +++ b/packages/strategy-tests/src/transitions.rs @@ -824,8 +824,8 @@ pub fn create_identity_credit_transfer_transition( let identity_public_key = identity .get_first_public_key_matching( Purpose::TRANSFER, - SecurityLevel::full_range().into(), - KeyType::all_key_types().into(), + HashSet::from([SecurityLevel::CRITICAL]), + HashSet::from([KeyType::ECDSA_SECP256K1, KeyType::BLS12_381]), false, ) .expect("expected to get a signing key"); From ae97f47b72a9e5a1c914469107865cb971929878 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 6 Nov 2024 21:56:50 +0700 Subject: [PATCH 08/15] ci: run devcontainers workflow only on push to master (#2295) --- .github/workflows/prebuild-devcontainers.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/prebuild-devcontainers.yml b/.github/workflows/prebuild-devcontainers.yml index 794fa3d4a56..1825985c82a 100644 --- a/.github/workflows/prebuild-devcontainers.yml +++ b/.github/workflows/prebuild-devcontainers.yml @@ -7,6 +7,8 @@ on: - '.github/workflows/prebuild-devcontainers.yml' - rust-toolchain.toml - Dockerfile + branches: + - master workflow_dispatch: concurrency: From 48cca1a319505ff7abda72c9a3bb87883d6e8a07 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Wed, 6 Nov 2024 21:57:07 +0700 Subject: [PATCH 09/15] ci: do not run test on push (#2308) --- .github/workflows/tests.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c823d0cd061..cca5f1c471d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,12 +7,6 @@ on: branches: - master - 'v[0-9]+\.[0-9]+-dev' - push: - branches: - - master - - 'v[0-9]+\.[0-9]+-dev' - schedule: - - cron: "30 4 * * *" concurrency: group: ${{ github.workflow }}-${{ github.ref }} From b8a887e6b252f22e5e75bbec1e0495d554aa4a38 Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Fri, 8 Nov 2024 12:44:15 +0700 Subject: [PATCH 10/15] fix(sdk): create channel error due to empty address --- packages/rs-dapi-client/src/address_list.rs | 55 +++++++++---------- packages/rs-dapi-client/src/transport/grpc.rs | 4 +- packages/rs-sdk/examples/read_contract.rs | 8 +-- packages/rs-sdk/src/platform/types/evonode.rs | 4 +- packages/rs-sdk/src/sync.rs | 7 +-- packages/rs-sdk/tests/fetch/config.rs | 6 +- packages/rs-sdk/tests/fetch/evonode.rs | 7 ++- 7 files changed, 44 insertions(+), 47 deletions(-) diff --git a/packages/rs-dapi-client/src/address_list.rs b/packages/rs-dapi-client/src/address_list.rs index 0c21ecc0b1a..081f5199004 100644 --- a/packages/rs-dapi-client/src/address_list.rs +++ b/packages/rs-dapi-client/src/address_list.rs @@ -1,7 +1,6 @@ //! Subsystem to manage DAPI nodes. use chrono::Utc; -use dapi_grpc::tonic::codegen::http; use dapi_grpc::tonic::transport::Uri; use rand::{rngs::SmallRng, seq::IteratorRandom, SeedableRng}; use std::collections::HashSet; @@ -26,8 +25,8 @@ impl FromStr for Address { fn from_str(s: &str) -> Result { Uri::from_str(s) - .map(Address::from) - .map_err(AddressListError::from) + .map_err(|e| AddressListError::InvalidAddressUri(e.to_string())) + .map(Address::try_from)? } } @@ -49,13 +48,19 @@ impl Hash for Address { } } -impl From for Address { - fn from(uri: Uri) -> Self { - Address { +impl TryFrom for Address { + type Error = AddressListError; + + fn try_from(value: Uri) -> Result { + if value.host().is_none() { + return Err(AddressListError::InvalidAddressUri("uri must contain host".to_string())); + } + + Ok(Address { ban_count: 0, banned_until: None, - uri, - } + uri: value, + }) } } @@ -96,7 +101,7 @@ pub enum AddressListError { /// A valid uri is required to create an Address #[error("unable parse address: {0}")] #[cfg_attr(feature = "mocks", serde(skip))] - InvalidAddressUri(#[from] http::uri::InvalidUri), + InvalidAddressUri(String), } /// A structure to manage DAPI addresses to select from @@ -167,15 +172,6 @@ impl AddressList { self.addresses.insert(address) } - // TODO: this is the most simple way to add an address - // however we need to support bulk loading (e.g. providing a network name) - // and also fetch updated from SML. - /// Add a node [Address] to [AddressList] by [Uri]. - /// Returns false if the address is already in the list. - pub fn add_uri(&mut self, uri: Uri) -> bool { - self.addresses.insert(uri.into()) - } - /// Randomly select a not banned address. pub fn get_live_address(&self) -> Option<&Address> { let mut rng = SmallRng::from_entropy(); @@ -185,7 +181,7 @@ impl AddressList { /// Get all addresses that are not banned. fn unbanned(&self) -> Vec<&Address> { - let now = chrono::Utc::now(); + let now = Utc::now(); self.addresses .iter() @@ -216,23 +212,24 @@ impl AddressList { } } -// TODO: Must be changed to FromStr -impl From<&str> for AddressList { - fn from(value: &str) -> Self { - let uri_list: Vec = value +impl FromStr for AddressList { + type Err = AddressListError; + + fn from_str(s: &str) -> Result { + let uri_list: Vec
= s .split(',') - .map(|uri| Uri::from_str(uri).expect("invalid uri")) - .collect(); + .map(Address::from_str) + .collect::>()?; - Self::from_iter(uri_list) + Ok(Self::from_iter(uri_list)) } } -impl FromIterator for AddressList { - fn from_iter>(iter: T) -> Self { +impl FromIterator
for AddressList { + fn from_iter>(iter: T) -> Self { let mut address_list = Self::new(); for uri in iter { - address_list.add_uri(uri); + address_list.add(uri); } address_list diff --git a/packages/rs-dapi-client/src/transport/grpc.rs b/packages/rs-dapi-client/src/transport/grpc.rs index fb1f08c842b..05a3210db78 100644 --- a/packages/rs-dapi-client/src/transport/grpc.rs +++ b/packages/rs-dapi-client/src/transport/grpc.rs @@ -44,8 +44,8 @@ impl TransportClient for PlatformGrpcClient { .get_or_create(PoolPrefix::Platform, &uri, None, || { match create_channel(uri.clone(), None) { Ok(channel) => Ok(Self::new(channel).into()), - Err(e) => Err(dapi_grpc::tonic::Status::failed_precondition(format!( - "Channel creation failed: {}", + Err(e) => Err(dapi_grpc::tonic::Status::invalid_argument(format!( + "channel creation failed: {}", e ))), } diff --git a/packages/rs-sdk/examples/read_contract.rs b/packages/rs-sdk/examples/read_contract.rs index 7ac2cc333db..75e1e1214d0 100644 --- a/packages/rs-sdk/examples/read_contract.rs +++ b/packages/rs-sdk/examples/read_contract.rs @@ -3,7 +3,7 @@ use std::{num::NonZeroUsize, str::FromStr}; use clap::Parser; use dash_sdk::{mock::provider::GrpcContextProvider, platform::Fetch, Sdk, SdkBuilder}; use dpp::prelude::{DataContract, Identifier}; -use rs_dapi_client::AddressList; +use rs_dapi_client::{Address, AddressList}; use zeroize::Zeroizing; #[derive(clap::Parser, Debug)] @@ -80,14 +80,14 @@ fn setup_sdk(config: &Config) -> Sdk { // Let's build the Sdk. // First, we need an URI of some Dash Platform DAPI host to connect to and use as seed. - let uri = http::Uri::from_str(&format!( - "http://{}:{}", + let address = Address::from_str(&format!( + "https://{}:{}", config.server_address, config.platform_port )) .expect("parse uri"); // Now, we create the Sdk with the wallet and context provider. - let sdk = SdkBuilder::new(AddressList::from_iter([uri])) + let sdk = SdkBuilder::new(AddressList::from_iter([address])) .build() .expect("cannot build sdk"); diff --git a/packages/rs-sdk/src/platform/types/evonode.rs b/packages/rs-sdk/src/platform/types/evonode.rs index 70bbabee616..2f91e171067 100644 --- a/packages/rs-sdk/src/platform/types/evonode.rs +++ b/packages/rs-sdk/src/platform/types/evonode.rs @@ -25,8 +25,8 @@ use std::fmt::Debug; /// use futures::executor::block_on; /// /// let sdk = Sdk::new_mock(); -/// let uri: http::Uri = "http://127.0.0.1:1".parse().unwrap(); -/// let node = EvoNode::new(uri.into()); +/// let address = "http://127.0.0.1:1".parse().expect("valid address"); +/// let node = EvoNode::new(address); /// let status = block_on(EvoNodeStatus::fetch_unproved(&sdk, node)).unwrap(); /// ``` diff --git a/packages/rs-sdk/src/sync.rs b/packages/rs-sdk/src/sync.rs index 38a878e174c..88e9fcd14b5 100644 --- a/packages/rs-sdk/src/sync.rs +++ b/packages/rs-sdk/src/sync.rs @@ -194,10 +194,10 @@ where let result= ::backon::Retryable::retry(closure,backoff_strategy) .when(|e| { if e.can_retry() { - // requests sent for current execution attempt; + // requests sent for current execution attempt; let requests_sent = e.retries + 1; - // requests sent in all preceeding attempts; user expects `settings.retries +1` + // requests sent in all preceeding attempts; user expects `settings.retries +1` retries += requests_sent; let all_requests_sent = retries; @@ -231,7 +231,6 @@ where #[cfg(test)] mod test { use super::*; - use http::Uri; use rs_dapi_client::ExecutionError; use std::{ future::Future, @@ -342,7 +341,7 @@ mod test { Err(ExecutionError { inner: MockError::Generic, retries, - address: Some(Uri::from_static("http://localhost").into()), + address: Some("http://localhost".parse().expect("valid address")), }) } diff --git a/packages/rs-sdk/tests/fetch/config.rs b/packages/rs-sdk/tests/fetch/config.rs index c2f8edbc4ed..27480632075 100644 --- a/packages/rs-sdk/tests/fetch/config.rs +++ b/packages/rs-sdk/tests/fetch/config.rs @@ -8,7 +8,7 @@ use dpp::{ dashcore::{hashes::Hash, ProTxHash}, prelude::Identifier, }; -use rs_dapi_client::AddressList; +use rs_dapi_client::{Address, AddressList}; use serde::Deserialize; use std::{path::PathBuf, str::FromStr}; use zeroize::Zeroizing; @@ -131,9 +131,9 @@ impl Config { false => "http", }; - let address: String = format!("{}://{}:{}", scheme, self.platform_host, self.platform_port); + let address: Address = format!("{}://{}:{}", scheme, self.platform_host, self.platform_port).parse().expect("valid address"); - AddressList::from_iter(vec![http::Uri::from_str(&address).expect("valid uri")]) + AddressList::from_iter([address]) } /// Create new SDK instance diff --git a/packages/rs-sdk/tests/fetch/evonode.rs b/packages/rs-sdk/tests/fetch/evonode.rs index 0d35d5be9f3..3ad36999159 100644 --- a/packages/rs-sdk/tests/fetch/evonode.rs +++ b/packages/rs-sdk/tests/fetch/evonode.rs @@ -8,6 +8,7 @@ use http::Uri; use std::time::Duration; /// Given some existing evonode URIs, WHEN we connect to them, THEN we get status. use tokio::time::timeout; +use rs_dapi_client::Address; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_evonode_status() { @@ -61,11 +62,11 @@ async fn test_evonode_status_refused() { let cfg = Config::new(); let sdk = cfg.setup_api("test_evonode_status_refused").await; - let uri: Uri = "http://127.0.0.1:1".parse().unwrap(); + let address: Address = "http://127.0.0.1:1".parse().expect("valid address"); - let node = EvoNode::new(uri.clone().into()); + let node = EvoNode::new(address.clone()); let result = EvoNodeStatus::fetch_unproved(&sdk, node).await; - tracing::debug!(?result, ?uri, "evonode status"); + tracing::debug!(?result, ?address, "evonode status"); assert!(result.is_err()); } From cea6ca9838d1e24068054544d1617523f3e79b66 Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Fri, 8 Nov 2024 16:30:38 +0700 Subject: [PATCH 11/15] chore: mark add_uri as deprecated --- packages/rs-dapi-client/src/address_list.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/rs-dapi-client/src/address_list.rs b/packages/rs-dapi-client/src/address_list.rs index 081f5199004..8694ff34cc6 100644 --- a/packages/rs-dapi-client/src/address_list.rs +++ b/packages/rs-dapi-client/src/address_list.rs @@ -172,6 +172,14 @@ impl AddressList { self.addresses.insert(address) } + #[deprecated] + // TODO: Remove in favor of add + /// Add a node [Address] to [AddressList] by [Uri]. + /// Returns false if the address is already in the list. + pub fn add_uri(&mut self, uri: Uri) -> bool { + self.addresses.insert(uri.try_into().expect("valid uri")) + } + /// Randomly select a not banned address. pub fn get_live_address(&self) -> Option<&Address> { let mut rng = SmallRng::from_entropy(); From 1307fb484cd0220eb29e302ab5acd77d34aa1504 Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Thu, 28 Nov 2024 20:50:49 +0700 Subject: [PATCH 12/15] style: fix formatting --- packages/rs-dapi-client/src/address_list.rs | 4 +++- packages/rs-sdk/tests/fetch/config.rs | 5 ++++- packages/rs-sdk/tests/fetch/evonode.rs | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/rs-dapi-client/src/address_list.rs b/packages/rs-dapi-client/src/address_list.rs index 8694ff34cc6..6d9067cd32f 100644 --- a/packages/rs-dapi-client/src/address_list.rs +++ b/packages/rs-dapi-client/src/address_list.rs @@ -53,7 +53,9 @@ impl TryFrom for Address { fn try_from(value: Uri) -> Result { if value.host().is_none() { - return Err(AddressListError::InvalidAddressUri("uri must contain host".to_string())); + return Err(AddressListError::InvalidAddressUri( + "uri must contain host".to_string(), + )); } Ok(Address { diff --git a/packages/rs-sdk/tests/fetch/config.rs b/packages/rs-sdk/tests/fetch/config.rs index 27480632075..f55484f5ce3 100644 --- a/packages/rs-sdk/tests/fetch/config.rs +++ b/packages/rs-sdk/tests/fetch/config.rs @@ -131,7 +131,10 @@ impl Config { false => "http", }; - let address: Address = format!("{}://{}:{}", scheme, self.platform_host, self.platform_port).parse().expect("valid address"); + let address: Address = + format!("{}://{}:{}", scheme, self.platform_host, self.platform_port) + .parse() + .expect("valid address"); AddressList::from_iter([address]) } diff --git a/packages/rs-sdk/tests/fetch/evonode.rs b/packages/rs-sdk/tests/fetch/evonode.rs index 3ad36999159..a4c1be2a0a9 100644 --- a/packages/rs-sdk/tests/fetch/evonode.rs +++ b/packages/rs-sdk/tests/fetch/evonode.rs @@ -5,10 +5,10 @@ use dash_sdk::platform::{types::evonode::EvoNode, FetchUnproved}; use dpp::dashcore::{hashes::Hash, ProTxHash}; use drive_proof_verifier::types::EvoNodeStatus; use http::Uri; +use rs_dapi_client::Address; use std::time::Duration; /// Given some existing evonode URIs, WHEN we connect to them, THEN we get status. use tokio::time::timeout; -use rs_dapi_client::Address; #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_evonode_status() { From 02e04745ca0e8bf32499bd6fadc24c0b2452bca5 Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Fri, 29 Nov 2024 13:14:08 +0700 Subject: [PATCH 13/15] refactor: use an invalid argument error --- packages/rs-dapi-client/src/transport/grpc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rs-dapi-client/src/transport/grpc.rs b/packages/rs-dapi-client/src/transport/grpc.rs index 22f9b495c03..62a75904064 100644 --- a/packages/rs-dapi-client/src/transport/grpc.rs +++ b/packages/rs-dapi-client/src/transport/grpc.rs @@ -65,7 +65,7 @@ impl TransportClient for PlatformGrpcClient { Some(settings), || match create_channel(uri.clone(), Some(settings)) { Ok(channel) => Ok(Self::new(channel).into()), - Err(e) => Err(dapi_grpc::tonic::Status::failed_precondition(format!( + Err(e) => Err(dapi_grpc::tonic::Status::invalid_argument(format!( "Channel creation failed: {}", e ))), @@ -81,7 +81,7 @@ impl TransportClient for CoreGrpcClient { .get_or_create(PoolPrefix::Core, &uri, None, || { match create_channel(uri.clone(), None) { Ok(channel) => Ok(Self::new(channel).into()), - Err(e) => Err(dapi_grpc::tonic::Status::failed_precondition(format!( + Err(e) => Err(dapi_grpc::tonic::Status::invalid_argument(format!( "Channel creation failed: {}", e ))), @@ -102,7 +102,7 @@ impl TransportClient for CoreGrpcClient { Some(settings), || match create_channel(uri.clone(), Some(settings)) { Ok(channel) => Ok(Self::new(channel).into()), - Err(e) => Err(dapi_grpc::tonic::Status::failed_precondition(format!( + Err(e) => Err(dapi_grpc::tonic::Status::invalid_argument(format!( "Channel creation failed: {}", e ))), From c46fa26f9e7bdffc185fd966b91badb57c6f3e9b Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Wed, 4 Dec 2024 17:49:12 +0700 Subject: [PATCH 14/15] fix: invalid add_uri --- packages/rs-dapi-client/src/address_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rs-dapi-client/src/address_list.rs b/packages/rs-dapi-client/src/address_list.rs index cddb64f5907..2f59b22c3bc 100644 --- a/packages/rs-dapi-client/src/address_list.rs +++ b/packages/rs-dapi-client/src/address_list.rs @@ -207,7 +207,7 @@ impl AddressList { /// Add a node [Address] to [AddressList] by [Uri]. /// Returns false if the address is already in the list. pub fn add_uri(&mut self, uri: Uri) -> bool { - self.addresses.insert(uri.try_into().expect("valid uri")) + self.add(Address::try_from(uri).expect("valid uri")) } /// Randomly select a not banned address. From 79188ec5759fce4a35b75fb7f942098e1099f435 Mon Sep 17 00:00:00 2001 From: ivanshumkov Date: Wed, 4 Dec 2024 19:51:07 +0700 Subject: [PATCH 15/15] revert: test workflow merge issue --- .github/workflows/tests.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ca0ae99a84f..4cf511cfbb1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -7,6 +7,12 @@ on: branches: - master - 'v[0-9]+\.[0-9]+-dev' + push: + branches: + - master + - 'v[0-9]+\.[0-9]+-dev' + schedule: + - cron: "30 4 * * *" concurrency: group: ${{ github.workflow }}-${{ github.ref }}