From 7a628518e5b396c8044ae02c32ec851f3a577e09 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 27 Nov 2024 12:08:21 +1000 Subject: [PATCH 1/3] Remove register_operator(_signing_key_proof_of_ownership) argument --- crates/pallet-domains/src/benchmarking.rs | 22 +----- crates/pallet-domains/src/lib.rs | 4 +- crates/pallet-domains/src/staking.rs | 77 +------------------ crates/pallet-domains/src/staking_epoch.rs | 20 +---- .../src/malicious_bundle_producer.rs | 21 +---- domains/client/domain-operator/src/tests.rs | 8 +- 6 files changed, 8 insertions(+), 144 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 9fc43c099c..8dafc286dc 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -25,7 +25,6 @@ use frame_support::traits::fungible::{Inspect, Mutate}; use frame_support::traits::Hooks; use frame_system::{Pallet as System, RawOrigin}; use sp_core::crypto::{Ss58Codec, UncheckedFrom}; -use sp_core::ByteArray; use sp_domains::{ dummy_opaque_bundle, DomainId, ExecutionReceipt, OperatorAllowList, OperatorId, OperatorPublicKey, OperatorRewardSource, OperatorSignature, PermissionedActionAllowedBy, @@ -580,23 +579,9 @@ mod benchmarks { // TODO: the `(key, signature)` is failed to verify in `cargo test --features runtime-benchmarks` but it // will pass when doing the actual benchmark with `subspace-node benchmark pallet ...`, need more investigations. - let (key, signature) = { - let key = OperatorPublicKey::from_ss58check( - "5Gv1Uopoqo1k7125oDtFSCmxH4DzuCiBU7HBKu2bF1GZFsEb", - ) - .unwrap(); - - // signature data included operator_account since result from `account` with same - // input is always deterministic - let sig = OperatorSignature::from_slice(&[ - 88, 91, 154, 118, 137, 117, 109, 164, 232, 186, 101, 199, 94, 12, 91, 47, 228, 198, - 61, 146, 200, 227, 152, 191, 205, 114, 81, 127, 192, 158, 48, 96, 211, 199, 237, - 121, 170, 38, 118, 109, 3, 44, 198, 54, 155, 133, 240, 77, 200, 117, 107, 34, 248, - 238, 144, 101, 200, 146, 20, 94, 180, 98, 40, 134, - ]) - .unwrap(); - (key, sig) - }; + let key = + OperatorPublicKey::from_ss58check("5Gv1Uopoqo1k7125oDtFSCmxH4DzuCiBU7HBKu2bF1GZFsEb") + .unwrap(); let operator_config = OperatorConfig { signing_key: key, minimum_nominator_stake: T::MinNominatorStake::get(), @@ -609,7 +594,6 @@ mod benchmarks { domain_id, T::MinOperatorStake::get(), operator_config.clone(), - signature, ); assert_eq!(NextOperatorId::::get(), operator_id + 1); diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index 494c3c29d3..e179c1f21f 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -231,8 +231,7 @@ mod pallet { use sp_domains::{ BundleDigest, DomainBundleSubmitted, DomainId, DomainSudoCall, DomainsTransfersTracker, EpochIndex, GenesisDomain, OnChainRewards, OnDomainInstantiated, OperatorAllowList, - OperatorId, OperatorPublicKey, OperatorRewardSource, OperatorSignature, RuntimeId, - RuntimeObject, RuntimeType, + OperatorId, OperatorPublicKey, OperatorRewardSource, RuntimeId, RuntimeObject, RuntimeType, }; use sp_domains_fraud_proof::fraud_proof_runtime_interface::domain_runtime_call; use sp_domains_fraud_proof::storage_proof::{self, FraudProofStorageKeyProvider}; @@ -1357,7 +1356,6 @@ mod pallet { domain_id: DomainId, amount: BalanceOf, config: OperatorConfig>, - _signing_key_proof_of_ownership: OperatorSignature, ) -> DispatchResult { let owner = ensure_signed(origin)?; diff --git a/crates/pallet-domains/src/staking.rs b/crates/pallet-domains/src/staking.rs index 7a672ab01f..62f1b52b04 100644 --- a/crates/pallet-domains/src/staking.rs +++ b/crates/pallet-domains/src/staking.rs @@ -1438,16 +1438,14 @@ pub(crate) mod tests { use crate::{ bundle_storage_fund, BalanceOf, Error, NominatorId, SlashedReason, MAX_NOMINATORS_TO_SLASH, }; - use codec::Encode; use frame_support::traits::fungible::Mutate; use frame_support::traits::Currency; use frame_support::weights::Weight; use frame_support::{assert_err, assert_ok}; - use sp_core::crypto::UncheckedFrom; use sp_core::{sr25519, Pair, U256}; use sp_domains::{ DomainId, OperatorAllowList, OperatorId, OperatorPair, OperatorPublicKey, - OperatorRewardSource, OperatorSignature, OperatorSigningKeyProofOfOwnershipData, + OperatorRewardSource, }; use sp_runtime::traits::Zero; use sp_runtime::{PerThing, Perbill}; @@ -1468,7 +1466,6 @@ pub(crate) mod tests { operator_stake: BalanceOf, minimum_nominator_stake: BalanceOf, signing_key: OperatorPublicKey, - signature: OperatorSignature, mut nominators: BTreeMap, (BalanceOf, BalanceOf)>, ) -> (OperatorId, OperatorConfig>) { nominators.insert(operator_account, (operator_free_balance, operator_stake)); @@ -1525,7 +1522,6 @@ pub(crate) mod tests { domain_id, operator_stake, operator_config.clone(), - signature, ); assert_ok!(res); @@ -1569,7 +1565,6 @@ pub(crate) mod tests { domain_id, Default::default(), operator_config, - OperatorSignature::unchecked_from([1u8; 64]), ); assert_err!( res, @@ -1592,17 +1587,11 @@ pub(crate) mod tests { nomination_tax: Default::default(), }; - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); - let res = Domains::register_operator( RuntimeOrigin::signed(operator_account), domain_id, Default::default(), operator_config, - signature, ); assert_err!( res, @@ -1623,10 +1612,6 @@ pub(crate) mod tests { let mut ext = new_test_ext(); ext.execute_with(|| { - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let (operator_id, mut operator_config) = register_operator( domain_id, operator_account, @@ -1634,7 +1619,6 @@ pub(crate) mod tests { operator_total_stake, SSC, pair.public(), - signature.clone(), BTreeMap::new(), ); @@ -1677,7 +1661,6 @@ pub(crate) mod tests { domain_id, operator_stake, operator_config.clone(), - signature.clone(), ); assert_err!( res, @@ -1687,16 +1670,11 @@ pub(crate) mod tests { // cannot use the locked funds to register a new operator let new_pair = OperatorPair::from_seed(&U256::from(1u32).into()); operator_config.signing_key = new_pair.public(); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = new_pair.sign(&data.encode()); let res = Domains::register_operator( RuntimeOrigin::signed(operator_account), domain_id, operator_stake, operator_config, - signature, ); assert_err!( res, @@ -1717,10 +1695,6 @@ pub(crate) mod tests { let operator_stake = 800 * SSC; let operator_storage_fee_deposit = 200 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let nominator_account = 2; let nominator_free_balance = 150 * SSC; @@ -1737,7 +1711,6 @@ pub(crate) mod tests { operator_total_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(vec![( nominator_account, (nominator_free_balance, nominator_total_stake), @@ -1835,10 +1808,6 @@ pub(crate) mod tests { let operator_stake = 200 * SSC; let operator_free_balance = 250 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let mut ext = new_test_ext(); ext.execute_with(|| { let (operator_id, _) = register_operator( @@ -1848,7 +1817,6 @@ pub(crate) mod tests { operator_stake, SSC, pair.public(), - signature, BTreeMap::new(), ); @@ -1960,10 +1928,6 @@ pub(crate) mod tests { let domain_id = DomainId::new(0); let operator_account = 0; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let mut total_balance = nominators.iter().map(|n| n.1).sum::>() + operator_reward + maybe_deposit.unwrap_or(0); @@ -1986,7 +1950,6 @@ pub(crate) mod tests { operator_stake, minimum_nominator_stake, pair.public(), - signature, nominators, ); @@ -2601,10 +2564,6 @@ pub(crate) mod tests { let operator_free_balance = 250 * SSC; let operator_stake = 200 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let nominator_account = 2; let nominator_free_balance = 150 * SSC; let nominator_stake = 100 * SSC; @@ -2627,7 +2586,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators), ); @@ -2732,10 +2690,6 @@ pub(crate) mod tests { let operator_stake = 200 * SSC; let operator_extra_deposit = 40 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let nominator_account = 2; let nominator_free_balance = 150 * SSC; let nominator_stake = 100 * SSC; @@ -2765,7 +2719,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators), ); @@ -2888,10 +2841,6 @@ pub(crate) mod tests { let operator_stake = 200 * SSC; let operator_extra_deposit = 40 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let nominator_accounts: Vec = (2..22).collect(); let nominator_free_balance = 150 * SSC; @@ -2929,7 +2878,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators), ); @@ -3066,21 +3014,6 @@ pub(crate) mod tests { let pair_2 = OperatorPair::from_seed(&U256::from(1u32).into()); let pair_3 = OperatorPair::from_seed(&U256::from(2u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account_1, - }; - let signature_1 = pair_1.sign(&data.encode()); - - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account_2, - }; - let signature_2 = pair_2.sign(&data.encode()); - - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account_3, - }; - let signature_3 = pair_3.sign(&data.encode()); - let mut ext = new_test_ext(); ext.execute_with(|| { let (operator_id_1, _) = register_operator( @@ -3090,7 +3023,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair_1.public(), - signature_1, Default::default(), ); @@ -3101,7 +3033,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair_2.public(), - signature_2, Default::default(), ); @@ -3112,7 +3043,6 @@ pub(crate) mod tests { operator_stake, 10 * SSC, pair_3.public(), - signature_3, Default::default(), ); @@ -3213,10 +3143,6 @@ pub(crate) mod tests { let operator_stake = 80 * SSC; let operator_storage_fee_deposit = 20 * SSC; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let nominator_account = 2; let mut ext = new_test_ext(); @@ -3228,7 +3154,6 @@ pub(crate) mod tests { operator_total_stake, SSC, pair.public(), - signature, BTreeMap::default(), ); diff --git a/crates/pallet-domains/src/staking_epoch.rs b/crates/pallet-domains/src/staking_epoch.rs index 969158b562..a5628de9d4 100644 --- a/crates/pallet-domains/src/staking_epoch.rs +++ b/crates/pallet-domains/src/staking_epoch.rs @@ -530,13 +530,10 @@ mod tests { use crate::{BalanceOf, Config, HoldIdentifier, NominatorId}; #[cfg(not(feature = "std"))] use alloc::vec; - use codec::Encode; use frame_support::assert_ok; use frame_support::traits::fungible::InspectHold; use sp_core::{Pair, U256}; - use sp_domains::{ - DomainId, OperatorPair, OperatorRewardSource, OperatorSigningKeyProofOfOwnershipData, - }; + use sp_domains::{DomainId, OperatorPair, OperatorRewardSource}; use sp_runtime::traits::Zero; use sp_runtime::{PerThing, Percent}; use std::collections::BTreeMap; @@ -554,10 +551,6 @@ mod tests { let domain_id = DomainId::new(0); let operator_account = 1; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let minimum_free_balance = 10 * SSC; let mut nominators = BTreeMap::from_iter( nominators @@ -586,7 +579,6 @@ mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators.clone()), ); @@ -692,10 +684,6 @@ mod tests { let domain_id = DomainId::new(0); let operator_account = 0; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let FinalizeDomainParams { total_deposit, rewards, @@ -729,7 +717,6 @@ mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators), ); @@ -808,10 +795,6 @@ mod tests { let domain_id = DomainId::new(0); let operator_account = 1; let pair = OperatorPair::from_seed(&U256::from(0u32).into()); - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: operator_account, - }; - let signature = pair.sign(&data.encode()); let operator_rewards = 10 * SSC; let mut nominators = BTreeMap::from_iter(vec![(1, (110 * SSC, 100 * SSC)), (2, (60 * SSC, 50 * SSC))]); @@ -827,7 +810,6 @@ mod tests { operator_stake, 10 * SSC, pair.public(), - signature, BTreeMap::from_iter(nominators), ); diff --git a/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs b/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs index 9a66c0a694..d898938e91 100644 --- a/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs +++ b/crates/subspace-malicious-operator/src/malicious_bundle_producer.rs @@ -18,10 +18,7 @@ use sp_blockchain::Info; use sp_consensus_slots::Slot; use sp_core::crypto::UncheckedFrom; use sp_domains::core_api::DomainCoreApi; -use sp_domains::{ - BundleProducerElectionApi, DomainId, DomainsApi, OperatorId, OperatorPublicKey, - OperatorSignature, OperatorSigningKeyProofOfOwnershipData, -}; +use sp_domains::{BundleProducerElectionApi, DomainId, DomainsApi, OperatorId, OperatorPublicKey}; use sp_keyring::Sr25519Keyring; use sp_keystore::{Keystore, KeystorePtr}; use sp_messenger::MessengerApi; @@ -272,19 +269,6 @@ where } }; - let data = OperatorSigningKeyProofOfOwnershipData { - operator_owner: self.sudo_account.clone(), - }; - let signature = OperatorSignature::from( - self.operator_keystore - .sr25519_sign( - OperatorPublicKey::ID, - signing_key.clone().as_ref(), - &data.encode(), - )? - .expect("key pair must be avaible on keystore for signing"), - ); - let maybe_operator_id = self .consensus_client .runtime_api() @@ -299,7 +283,6 @@ where self.submit_register_operator( nonce, signing_key, - signature, // Ideally we should use the `next_total_stake` but it is tricky to get MALICIOUS_OPR_STAKE_MULTIPLIER * current_total_stake, )?; @@ -347,7 +330,6 @@ where &self, nonce: Nonce, signing_key: OperatorPublicKey, - signature: OperatorSignature, staking_amount: Balance, ) -> Result<(), Box> { let call = pallet_domains::Call::register_operator { @@ -358,7 +340,6 @@ where minimum_nominator_stake: Balance::MAX, nomination_tax: Default::default(), }, - signing_key_proof_of_ownership: signature, }; self.submit_consensus_extrinsic(Some(nonce), call.into()) } diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index 28547b48a3..bc5c89f2ee 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -32,7 +32,7 @@ use sp_domains::core_api::DomainCoreApi; use sp_domains::merkle_tree::MerkleTree; use sp_domains::{ Bundle, BundleValidity, ChainId, ChannelId, DomainsApi, HeaderHashingFor, InboxedBundle, - InvalidBundleType, OperatorSignature, OperatorSigningKeyProofOfOwnershipData, Transfers, + InvalidBundleType, Transfers, }; use sp_domains_fraud_proof::fraud_proof::{ ApplyExtrinsicMismatch, ExecutionPhase, FinalizeBlockMismatch, FraudProofVariant, @@ -4535,12 +4535,6 @@ async fn test_bad_receipt_chain() { minimum_nominator_stake: Balance::MAX, nomination_tax: Default::default(), }, - signing_key_proof_of_ownership: OperatorSignature::from( - OperatorSigningKeyProofOfOwnershipData { - operator_owner: Sr25519Alice.to_account_id(), - } - .using_encoded(|e| Sr25519Keyring::Charlie.sign(e)), - ), }) .await .unwrap(); From 2654d063fe661aac369a1304da4f0519e3537b3e Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 27 Nov 2024 12:21:25 +1000 Subject: [PATCH 2/3] Remove OperatorSigningKeyProofOfOwnershipData --- crates/sp-domains/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/sp-domains/src/lib.rs b/crates/sp-domains/src/lib.rs index e0d84ff5bc..80cca0deda 100644 --- a/crates/sp-domains/src/lib.rs +++ b/crates/sp-domains/src/lib.rs @@ -1456,14 +1456,6 @@ pub fn operator_block_fees_final_key() -> Vec { .to_vec() } -/// Preimage to verify the proof of ownership of Operator Signing key. -/// Operator owner is used to ensure the signature is used by anyone except -/// the owner of the Signing key pair. -#[derive(Debug, Encode)] -pub struct OperatorSigningKeyProofOfOwnershipData { - pub operator_owner: AccountId, -} - /// Hook to handle chain rewards. pub trait OnChainRewards { fn on_chain_rewards(chain_id: ChainId, reward: Balance); From 749a31f77c7e212a1a1ea7f949ed2a1f750512e0 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 27 Nov 2024 14:17:32 +1000 Subject: [PATCH 3/3] Remove outdated comment --- crates/pallet-domains/src/benchmarking.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/pallet-domains/src/benchmarking.rs b/crates/pallet-domains/src/benchmarking.rs index 8dafc286dc..cee5e4cc4f 100644 --- a/crates/pallet-domains/src/benchmarking.rs +++ b/crates/pallet-domains/src/benchmarking.rs @@ -576,9 +576,6 @@ mod benchmarks { let domain_id = register_domain::(); let operator_id = NextOperatorId::::get(); - - // TODO: the `(key, signature)` is failed to verify in `cargo test --features runtime-benchmarks` but it - // will pass when doing the actual benchmark with `subspace-node benchmark pallet ...`, need more investigations. let key = OperatorPublicKey::from_ss58check("5Gv1Uopoqo1k7125oDtFSCmxH4DzuCiBU7HBKu2bF1GZFsEb") .unwrap();