From a2c0a897f904eb0b6d5c1454688fc14b8ba11ed3 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 18 Dec 2024 13:10:16 +1000 Subject: [PATCH 1/2] Fix bug in transaction fee fraud proof and consolidate storage --- crates/pallet-domains/src/lib.rs | 13 +++- crates/pallet-transaction-fees/src/lib.rs | 4 - .../src/storage_proof.rs | 74 +------------------ .../src/verification.rs | 3 +- crates/subspace-runtime/src/lib.rs | 6 -- test/subspace-test-runtime/src/lib.rs | 6 -- 6 files changed, 17 insertions(+), 89 deletions(-) diff --git a/crates/pallet-domains/src/lib.rs b/crates/pallet-domains/src/lib.rs index e131758b79..aab92ac09f 100644 --- a/crates/pallet-domains/src/lib.rs +++ b/crates/pallet-domains/src/lib.rs @@ -248,7 +248,7 @@ mod pallet { use sp_std::fmt::Debug; use sp_subspace_mmr::MmrProofVerifier; use subspace_core_primitives::{Randomness, U256}; - use subspace_runtime_primitives::StorageFee; + use subspace_runtime_primitives::{Balance, StorageFee}; #[pallet::config] pub trait Config: frame_system::Config> { @@ -1916,16 +1916,25 @@ mod pallet { Into::::into(Self::extrinsics_shuffling_seed()).to_fixed_bytes(), ); - // There is no actual conversion here, but the trait bounds required to prove that + // There are no actual conversions here, but the trait bounds required to prove that // (and debug-print the error in expect()) are very verbose. let timestamp = T::BlockTimestamp::now() .try_into() .map_err(|_| ()) .expect("Moment is the same type in both pallets; qed"); + let transaction_byte_fee: Balance = T::StorageFee::transaction_byte_fee() + .try_into() + .map_err(|_| ()) + .expect("Balance is the same type in both pallets; qed"); + + // The value returned by the consensus_chain_byte_fee() runtime API + let consensus_transaction_byte_fee = + sp_domains::DOMAIN_STORAGE_FEE_MULTIPLIER * transaction_byte_fee; let invalid_inherent_extrinsic_data = InvalidInherentExtrinsicData { extrinsics_shuffling_seed, timestamp, + consensus_transaction_byte_fee, }; BlockInvalidInherentExtrinsicData::::set(Some(invalid_inherent_extrinsic_data)); diff --git a/crates/pallet-transaction-fees/src/lib.rs b/crates/pallet-transaction-fees/src/lib.rs index 2558526451..ac31cab436 100644 --- a/crates/pallet-transaction-fees/src/lib.rs +++ b/crates/pallet-transaction-fees/src/lib.rs @@ -242,10 +242,6 @@ where } } - pub fn transaction_byte_fee_storage_key() -> Vec { - TransactionByteFee::::hashed_key().to_vec() - } - pub fn calculate_transaction_byte_fee() -> BalanceOf { let credit_supply = T::CreditSupply::get(); diff --git a/crates/sp-domains-fraud-proof/src/storage_proof.rs b/crates/sp-domains-fraud-proof/src/storage_proof.rs index 3c4f8afaf3..899ea77b76 100644 --- a/crates/sp-domains-fraud-proof/src/storage_proof.rs +++ b/crates/sp-domains-fraud-proof/src/storage_proof.rs @@ -18,7 +18,7 @@ use sp_std::marker::PhantomData; use sp_std::vec::Vec; use sp_trie::StorageProof; use subspace_core_primitives::Randomness; -use subspace_runtime_primitives::{Balance, BlockTransactionByteFee, Moment}; +use subspace_runtime_primitives::{Balance, Moment}; #[cfg(feature = "std")] use sc_client_api::ProofProvider; @@ -39,11 +39,9 @@ pub enum VerificationError { UnexpectedDomainRuntimeUpgrade, InvalidInherentExtrinsicStorageProof(StorageProofVerificationError), SuccessfulBundlesStorageProof(StorageProofVerificationError), - TransactionByteFeeStorageProof(StorageProofVerificationError), DomainAllowlistUpdatesStorageProof(StorageProofVerificationError), BlockDigestStorageProof(StorageProofVerificationError), RuntimeRegistryStorageProof(StorageProofVerificationError), - DynamicCostOfStorageStorageProof(StorageProofVerificationError), DigestStorageProof(StorageProofVerificationError), BlockFeesStorageProof(StorageProofVerificationError), TransfersStorageProof(StorageProofVerificationError), @@ -56,11 +54,9 @@ pub enum VerificationError { pub enum FraudProofStorageKeyRequest { InvalidInherentExtrinsicData, SuccessfulBundles(DomainId), - TransactionByteFee, DomainAllowlistUpdates(DomainId), BlockDigest, RuntimeRegistry(RuntimeId), - DynamicCostOfStorage, DomainSudoCall(DomainId), MmrRoot(Number), } @@ -72,13 +68,11 @@ impl FraudProofStorageKeyRequest { VerificationError::InvalidInherentExtrinsicStorageProof(err) } Self::SuccessfulBundles(_) => VerificationError::SuccessfulBundlesStorageProof(err), - Self::TransactionByteFee => VerificationError::TransactionByteFeeStorageProof(err), Self::DomainAllowlistUpdates(_) => { VerificationError::DomainAllowlistUpdatesStorageProof(err) } Self::BlockDigest => VerificationError::BlockDigestStorageProof(err), Self::RuntimeRegistry(_) => VerificationError::RuntimeRegistryStorageProof(err), - Self::DynamicCostOfStorage => VerificationError::DynamicCostOfStorageStorageProof(err), FraudProofStorageKeyRequest::DomainSudoCall(_) => { VerificationError::DomainSudoCallStorageProof(err) } @@ -185,28 +179,6 @@ impl BasicStorageProof for DomainChainsAllowlistUpdateStor } } -#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)] -pub struct DynamicCostOfStorageProof(StorageProof); - -impl_storage_proof!(DynamicCostOfStorageProof); -impl BasicStorageProof for DynamicCostOfStorageProof { - type StorageValue = bool; - fn storage_key_request(_key: Self::Key) -> FraudProofStorageKeyRequest> { - FraudProofStorageKeyRequest::DynamicCostOfStorage - } -} - -#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)] -pub struct ConsensusTransactionByteFeeProof(StorageProof); - -impl_storage_proof!(ConsensusTransactionByteFeeProof); -impl BasicStorageProof for ConsensusTransactionByteFeeProof { - type StorageValue = BlockTransactionByteFee; - fn storage_key_request(_key: Self::Key) -> FraudProofStorageKeyRequest> { - FraudProofStorageKeyRequest::TransactionByteFee - } -} - #[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)] pub struct BlockDigestProof(StorageProof); @@ -398,6 +370,9 @@ pub struct InvalidInherentExtrinsicData { /// Block timestamp pub timestamp: Moment, + + /// Transaction byte fee, derived from dynamic cost of storage and the consensus chain byte fee + pub consensus_transaction_byte_fee: Balance, } impl PassBy for InvalidInherentExtrinsicData { @@ -420,12 +395,6 @@ pub struct InvalidInherentExtrinsicProof { /// Optional domain runtime code upgrade storage proof pub maybe_domain_runtime_upgrade_proof: MaybeDomainRuntimeUpgradedProof, - /// Boolean indicating if dynamic cost of storage was used (but as a storage proof) - pub dynamic_cost_of_storage_proof: DynamicCostOfStorageProof, - - /// Transaction fee storage proof - pub consensus_chain_byte_fee_proof: ConsensusTransactionByteFeeProof, - /// Change in the allowed chains storage proof pub domain_chain_allowlist_proof: DomainChainsAllowlistUpdateStorageProof, } @@ -434,7 +403,6 @@ pub struct InvalidInherentExtrinsicProof { #[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)] pub struct InvalidInherentExtrinsicVerified { pub maybe_domain_runtime_upgrade: Option>, - pub consensus_transaction_byte_fee: Balance, pub domain_chain_allowlist: DomainAllowlistUpdates, } @@ -458,18 +426,6 @@ impl InvalidInherentExtrinsicProof { block_hash, maybe_runtime_id, )?; - let dynamic_cost_of_storage_proof = DynamicCostOfStorageProof::generate( - proof_provider, - block_hash, - (), - storage_key_provider, - )?; - let consensus_chain_byte_fee_proof = ConsensusTransactionByteFeeProof::generate( - proof_provider, - block_hash, - (), - storage_key_provider, - )?; let domain_chain_allowlist_proof = DomainChainsAllowlistUpdateStorageProof::generate( proof_provider, block_hash, @@ -479,8 +435,6 @@ impl InvalidInherentExtrinsicProof { Ok(Self { maybe_domain_runtime_upgrade_proof, - dynamic_cost_of_storage_proof, - consensus_chain_byte_fee_proof, domain_chain_allowlist_proof, }) } @@ -495,25 +449,6 @@ impl InvalidInherentExtrinsicProof { .maybe_domain_runtime_upgrade_proof .verify::(runtime_id, state_root)?; - let dynamic_cost_of_storage = - >::verify::( - self.dynamic_cost_of_storage_proof.clone(), - (), - state_root, - )?; - let consensus_transaction_byte_fee = if dynamic_cost_of_storage { - let raw_transaction_byte_fee = - >::verify::( - self.consensus_chain_byte_fee_proof.clone(), - (), - state_root, - )?; - - sp_domains::DOMAIN_STORAGE_FEE_MULTIPLIER * raw_transaction_byte_fee.next - } else { - Balance::from(1u32) - }; - let domain_chain_allowlist = >::verify::( self.domain_chain_allowlist_proof.clone(), @@ -523,7 +458,6 @@ impl InvalidInherentExtrinsicProof { Ok(InvalidInherentExtrinsicVerified { maybe_domain_runtime_upgrade, - consensus_transaction_byte_fee, domain_chain_allowlist, }) } diff --git a/crates/sp-domains-fraud-proof/src/verification.rs b/crates/sp-domains-fraud-proof/src/verification.rs index 5be342a688..6c4a624258 100644 --- a/crates/sp-domains-fraud-proof/src/verification.rs +++ b/crates/sp-domains-fraud-proof/src/verification.rs @@ -94,7 +94,8 @@ where let domain_inherent_extrinsic_data = DomainInherentExtrinsicData { timestamp: invalid_inherent_extrinsic_data.timestamp, maybe_domain_runtime_upgrade: inherent_extrinsic_verified.maybe_domain_runtime_upgrade, - consensus_transaction_byte_fee: inherent_extrinsic_verified.consensus_transaction_byte_fee, + consensus_transaction_byte_fee: invalid_inherent_extrinsic_data + .consensus_transaction_byte_fee, domain_chain_allowlist: inherent_extrinsic_verified.domain_chain_allowlist, maybe_sudo_runtime_call: domain_sudo_call.maybe_call, }; diff --git a/crates/subspace-runtime/src/lib.rs b/crates/subspace-runtime/src/lib.rs index c29579b8ca..ce1bf5389d 100644 --- a/crates/subspace-runtime/src/lib.rs +++ b/crates/subspace-runtime/src/lib.rs @@ -1045,9 +1045,6 @@ impl FraudProofStorageKeyProvider> for StorageKeyProvider { FraudProofStorageKeyRequest::SuccessfulBundles(domain_id) => { pallet_domains::SuccessfulBundles::::hashed_key_for(domain_id) } - FraudProofStorageKeyRequest::TransactionByteFee => { - TransactionFees::transaction_byte_fee_storage_key() - } FraudProofStorageKeyRequest::DomainAllowlistUpdates(domain_id) => { Messenger::domain_allow_list_update_storage_key(domain_id) } @@ -1055,9 +1052,6 @@ impl FraudProofStorageKeyProvider> for StorageKeyProvider { FraudProofStorageKeyRequest::RuntimeRegistry(runtime_id) => { pallet_domains::RuntimeRegistry::::hashed_key_for(runtime_id) } - FraudProofStorageKeyRequest::DynamicCostOfStorage => { - pallet_runtime_configs::EnableDynamicCostOfStorage::::hashed_key().to_vec() - } FraudProofStorageKeyRequest::DomainSudoCall(domain_id) => { pallet_domains::DomainSudoCalls::::hashed_key_for(domain_id) } diff --git a/test/subspace-test-runtime/src/lib.rs b/test/subspace-test-runtime/src/lib.rs index 8e966f99d0..5da265a213 100644 --- a/test/subspace-test-runtime/src/lib.rs +++ b/test/subspace-test-runtime/src/lib.rs @@ -1122,9 +1122,6 @@ impl FraudProofStorageKeyProvider> for StorageKeyProvider { FraudProofStorageKeyRequest::SuccessfulBundles(domain_id) => { pallet_domains::SuccessfulBundles::::hashed_key_for(domain_id) } - FraudProofStorageKeyRequest::TransactionByteFee => { - TransactionFees::transaction_byte_fee_storage_key() - } FraudProofStorageKeyRequest::DomainAllowlistUpdates(domain_id) => { Messenger::domain_allow_list_update_storage_key(domain_id) } @@ -1132,9 +1129,6 @@ impl FraudProofStorageKeyProvider> for StorageKeyProvider { FraudProofStorageKeyRequest::RuntimeRegistry(runtime_id) => { pallet_domains::RuntimeRegistry::::hashed_key_for(runtime_id) } - FraudProofStorageKeyRequest::DynamicCostOfStorage => { - pallet_runtime_configs::EnableDynamicCostOfStorage::::hashed_key().to_vec() - } FraudProofStorageKeyRequest::DomainSudoCall(domain_id) => { pallet_domains::DomainSudoCalls::::hashed_key_for(domain_id) } From 2e2af45ac86f802ee64d391fece9c0cb3bb30e87 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 20 Dec 2024 07:41:54 +1000 Subject: [PATCH 2/2] Fix typos --- crates/sp-domains-fraud-proof/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sp-domains-fraud-proof/src/lib.rs b/crates/sp-domains-fraud-proof/src/lib.rs index f061c28796..7139e1252f 100644 --- a/crates/sp-domains-fraud-proof/src/lib.rs +++ b/crates/sp-domains-fraud-proof/src/lib.rs @@ -160,7 +160,7 @@ sp_api::decl_runtime_apis! { /// Submit the fraud proof via an unsigned extrinsic. fn submit_fraud_proof_unsigned(fraud_proof: FraudProof, Block::Hash, DomainHeader, H256>); - /// Reture the storage key used in fraud proof + /// Return the storage key used in fraud proof fn fraud_proof_storage_key(req: FraudProofStorageKeyRequest>) -> Vec; } }