diff --git a/Cargo.lock b/Cargo.lock index da761b8b1..ffda6c3cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11582,6 +11582,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log", "orml-tokens", "orml-traits", "parity-scale-codec", @@ -11724,7 +11725,7 @@ dependencies = [ [[package]] name = "substrate-stellar-sdk" version = "0.3.0" -source = "git+https://github.com/pendulum-chain/substrate-stellar-sdk?branch=polkadot-v1.1.0#22bd78d713d4992ca7f52975995fe12bae9e220f" +source = "git+https://github.com/pendulum-chain/substrate-stellar-sdk?branch=polkadot-v1.1.0#9b8e2b77b6c6a63e8e837d1e8f2b42b09d49a943" dependencies = [ "base64 0.13.1", "hex", diff --git a/clients/stellar-relay-lib/src/connection/connector/connector.rs b/clients/stellar-relay-lib/src/connection/connector/connector.rs index b9392fc54..80cbeaade 100644 --- a/clients/stellar-relay-lib/src/connection/connector/connector.rs +++ b/clients/stellar-relay-lib/src/connection/connector/connector.rs @@ -234,9 +234,7 @@ impl Connector { local_overlay_version: u32, remote_overlay_version: u32, ) -> StellarMessage { - return self - .flow_controller - .start(local_overlay_version, remote_overlay_version); + return self.flow_controller.start(local_overlay_version, remote_overlay_version); } pub fn handshake_completed(&mut self) { diff --git a/clients/stellar-relay-lib/src/connection/connector/message_handler.rs b/clients/stellar-relay-lib/src/connection/connector/message_handler.rs index 1722bf9e1..dab70ba58 100644 --- a/clients/stellar-relay-lib/src/connection/connector/message_handler.rs +++ b/clients/stellar-relay-lib/src/connection/connector/message_handler.rs @@ -41,7 +41,7 @@ impl Connector { return Err(Error::from(e)); }, other => error!( - "process_raw_message(): Received ErrorMsg during authentication: {:?}", + "process_raw_message(): Received a different message during authentication: {:?}", other ), }, diff --git a/clients/vault/src/oracle/testing_utils.rs b/clients/vault/src/oracle/testing_utils.rs index c78e2f042..6c75e0a7d 100644 --- a/clients/vault/src/oracle/testing_utils.rs +++ b/clients/vault/src/oracle/testing_utils.rs @@ -25,7 +25,7 @@ pub fn specific_stellar_relay_config( fn stellar_relay_config_choices(is_mainnet: bool) -> (Vec<&'static str>, &'static str) { let node_points = if is_mainnet { - vec!["frankfurt", "iowa", "singapore"] + vec!["iowa", "singapore", "frankfurt"] } else { vec!["sdftest1", "sdftest2", "sdftest3"] }; diff --git a/clients/vault/tests/helper/mod.rs b/clients/vault/tests/helper/mod.rs index 115c574eb..efbd1c645 100644 --- a/clients/vault/tests/helper/mod.rs +++ b/clients/vault/tests/helper/mod.rs @@ -149,5 +149,8 @@ where .expect("failed to start agent"); let oracle_agent = Arc::new(oracle_agent); + // continue ONLY if the oracle agent has received the first slot + while oracle_agent.last_slot_index().await == 0 {} + execute(client, vault_wallet, user_wallet, oracle_agent, vault_id, vault_provider).await } diff --git a/pallets/stellar-relay/Cargo.toml b/pallets/stellar-relay/Cargo.toml index 4fbf09eb6..1704f71d8 100644 --- a/pallets/stellar-relay/Cargo.toml +++ b/pallets/stellar-relay/Cargo.toml @@ -17,6 +17,7 @@ base64 = { version = '0.13.0', default-features = false, features = ['alloc'] } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = [ "derive", ] } +log = { version = "0.4.14", default-features = false } serde = {version = "1.0.130", default-features = false, features = ["derive"]} scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } frame-support = { default-features = false, git = "https://github.com/paritytech/polkadot-sdk.git", branch = "release-polkadot-v1.1.0" } diff --git a/pallets/stellar-relay/src/lib.rs b/pallets/stellar-relay/src/lib.rs index 4abae0083..ef8f88105 100644 --- a/pallets/stellar-relay/src/lib.rs +++ b/pallets/stellar-relay/src/lib.rs @@ -31,7 +31,7 @@ use primitives::{derive_shortened_request_id, get_text_memo_from_tx_env, TextMem #[frame_support::pallet] pub mod pallet { use codec::FullCodec; - use frame_support::{pallet_prelude::*, sp_runtime, transactional}; + use frame_support::{pallet_prelude::*, transactional}; use frame_system::pallet_prelude::*; use primitives::stellar::{ compound_types::UnlimitedVarArray, @@ -44,13 +44,14 @@ pub mod pallet { use sp_std::{collections::btree_map::BTreeMap, fmt::Debug, vec, vec::Vec}; use default_weights::WeightInfo; + use primitives::stellar::types::ScpStatementPledges; use crate::{ traits::FieldLength, types::{OrganizationOf, ValidatorOf}, validation::{ - check_for_valid_quorum_set, find_externalized_envelope, get_externalized_info, - validate_envelopes, validators_and_orgs, + check_for_valid_quorum_set, get_externalized_info, validate_envelopes, + validators_and_orgs, }, }; @@ -108,7 +109,6 @@ pub mod pallet { DuplicateOrganizationId, DuplicateValidatorPublicKey, EmptyEnvelopeSet, - EnvelopeSignedByUnknownValidator, EnvelopeSlotIndexMismatch, ExternalizedNHMismatch, ExternalizedValueMismatch, @@ -560,7 +560,12 @@ pub mod pallet { let (validators, organizations) = validators_and_orgs()?; - let externalized_envelope = find_externalized_envelope(envelopes)?; + let externalized_envelope = envelopes + .get_element(|envelope| match envelope.statement.pledges { + ScpStatementPledges::ScpStExternalize(_) => true, + _ => false, + }) + .ok_or(Error::MissingExternalizedMessage)?; // We store the externalized value in a variable so that we can check if it's the same // for all envelopes. We don't distinguish between externalized and confirmed values as @@ -574,7 +579,8 @@ pub mod pallet { .get_tx_set_hash() .map_err(|_| Error::::FailedToComputeNonGenericTxSetContentHash)?; - validate_envelopes( + // returns ONLY the valid envelopes + let validated_envelopes = validate_envelopes( envelopes, &validators, &network, @@ -586,7 +592,7 @@ pub mod pallet { )?; // ---- Check that externalized messages build valid quorum set ---- - check_for_valid_quorum_set(envelopes, validators, organizations.len()) + check_for_valid_quorum_set(validated_envelopes, validators, organizations.len()) } pub(crate) fn get_tx_set_hash(scp_value: &Value) -> Result> { diff --git a/pallets/stellar-relay/src/tests.rs b/pallets/stellar-relay/src/tests.rs index 99b7c74db..efc228f38 100644 --- a/pallets/stellar-relay/src/tests.rs +++ b/pallets/stellar-relay/src/tests.rs @@ -195,7 +195,7 @@ fn validate_stellar_transaction_fails_for_wrong_signature() { } #[test] -fn validate_stellar_transaction_fails_for_unknown_validator() { +fn validate_stellar_transaction_ignores_unknown_validator() { run_test(|organizations, mut validators, mut validator_secret_keys| { let public_network = true; @@ -212,9 +212,8 @@ fn validate_stellar_transaction_fails_for_unknown_validator() { false, ); - let result = - SpacewalkRelay::validate_stellar_transaction(&tx_envelope, &scp_envelopes, &tx_set); - assert!(matches!(result, Err(Error::::EnvelopeSignedByUnknownValidator))); + assert!(SpacewalkRelay::validate_stellar_transaction(&tx_envelope, &scp_envelopes, &tx_set) + .is_ok()) }); } diff --git a/pallets/stellar-relay/src/validation.rs b/pallets/stellar-relay/src/validation.rs index 347f32d43..acbb668ca 100644 --- a/pallets/stellar-relay/src/validation.rs +++ b/pallets/stellar-relay/src/validation.rs @@ -35,16 +35,18 @@ fn validator_count_per_org( /// Builds a map used to identify the targeted organizations fn targeted_organization_map( - envelopes: &UnlimitedVarArray, + envelopes: UnlimitedVarArray<&ScpEnvelope>, validators: &ValidatorsList, ) -> BTreeMap { // Find the validators that are targeted by the SCP messages let targeted_validators = validators .iter() .filter(|validator| { - envelopes.get_vec().iter().any(|envelope| { - envelope.statement.node_id.to_encoding() == validator.public_key.to_vec() - }) + envelopes + .get_element(|envelope| { + envelope.statement.node_id.to_encoding() == validator.public_key.to_vec() + }) + .is_some() }) .collect::>>(); @@ -75,22 +77,29 @@ pub fn get_externalized_info(envelope: &ScpEnvelope) -> Result<(&Valu } /// Returns the node id of the envelope if it is part of the set of validators -fn get_node_id( +fn is_node_id_exist( envelope: &ScpEnvelope, validators: &BoundedVec, T::ValidatorLimit>, -) -> Result> { +) -> Option { let node_id = envelope.statement.node_id.clone(); let node_id_found = validators .iter() .any(|validator| validator.public_key.to_vec() == node_id.to_encoding()); - ensure!(node_id_found, Error::::EnvelopeSignedByUnknownValidator); + if !node_id_found { + log::warn!( + "Envelope with slot index {}: Node id {:?} is not part of validators list", + envelope.statement.slot_index, + envelope.statement.node_id + ); + return None + } - Ok(node_id) + Some(node_id) } pub fn check_for_valid_quorum_set( - envelopes: &UnlimitedVarArray, + envelopes: UnlimitedVarArray<&ScpEnvelope>, validators: BoundedVec, T::ValidatorLimit>, orgs_length: usize, ) -> Result<(), Error> { @@ -136,7 +145,8 @@ pub fn check_for_valid_quorum_set( Ok(()) } -/// Checks that all envelopes have the same values +/// Checks that all envelopes have the same values. IGNORES envelopes with DIFFERENT node id. +/// Returns ONLY the valid envelopes. /// /// # Arguments /// @@ -147,18 +157,25 @@ pub fn check_for_valid_quorum_set( /// * `externalized_n_h` - A value that must be equal amongst all envelopes /// * `expected_tx_set_hash` - A value that must be equal amongst all envelopes /// * `slot_index` - used to check if all envelopes are using the same slot -pub fn validate_envelopes( - envelopes: &UnlimitedVarArray, +pub fn validate_envelopes<'a, T: Config>( + envelopes: &'a UnlimitedVarArray, validators: &BoundedVec, T::ValidatorLimit>, network: &Network, externalized_value: &Value, externalized_n_h: u32, expected_tx_set_hash: Hash, slot_index: u64, -) -> Result<(), Error> { +) -> Result, Error> { + // let's create a new placeholder for valid envelopes + let mut validated_envelopes = sp_std::vec![]; let mut externalized_n_h = externalized_n_h; - for envelope in envelopes.get_vec() { - let node_id = get_node_id(envelope, validators)?; + + let envelopes = envelopes.get_vec(); + for envelope in envelopes { + let Some(node_id) = is_node_id_exist::(envelope, validators) else { + // ignore this envelope; continue to the next ones + continue + }; // Check if all envelopes are using the same slot index ensure!(slot_index == envelope.statement.slot_index, Error::::EnvelopeSlotIndexMismatch); @@ -185,22 +202,12 @@ pub fn validate_envelopes( else if n_h < u32::MAX { ensure!(externalized_n_h == n_h, Error::::ExternalizedNHMismatch); } - } - Ok(()) -} + // if all checks passed, insert. + validated_envelopes.push(envelope); + } -pub fn find_externalized_envelope( - envelopes: &UnlimitedVarArray, -) -> Result<&ScpEnvelope, Error> { - envelopes - .get_vec() - .iter() - .find(|envelope| match envelope.statement.pledges { - ScpStatementPledges::ScpStExternalize(_) => true, - _ => false, - }) - .ok_or(Error::::MissingExternalizedMessage) + Ok(UnlimitedVarArray::new(validated_envelopes).unwrap_or(UnlimitedVarArray::new_empty())) } pub fn validators_and_orgs(