Skip to content

Commit

Permalink
Fix bug in transaction fee fraud proof and consolidate storage
Browse files Browse the repository at this point in the history
  • Loading branch information
teor2345 committed Dec 19, 2024
1 parent b3af00c commit 880c03e
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 89 deletions.
13 changes: 11 additions & 2 deletions crates/pallet-domains/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Hash: Into<H256>> {
Expand Down Expand Up @@ -1916,16 +1916,25 @@ mod pallet {
Into::<H256>::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::<T>::set(Some(invalid_inherent_extrinsic_data));
Expand Down
4 changes: 0 additions & 4 deletions crates/pallet-transaction-fees/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,6 @@ where
}
}

pub fn transaction_byte_fee_storage_key() -> Vec<u8> {
TransactionByteFee::<T>::hashed_key().to_vec()
}

pub fn calculate_transaction_byte_fee() -> BalanceOf<T> {
let credit_supply = T::CreditSupply::get();

Expand Down
74 changes: 4 additions & 70 deletions crates/sp-domains-fraud-proof/src/storage_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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),
Expand All @@ -56,11 +54,9 @@ pub enum VerificationError {
pub enum FraudProofStorageKeyRequest<Number> {
InvalidInherentExtrinsicData,
SuccessfulBundles(DomainId),
TransactionByteFee,
DomainAllowlistUpdates(DomainId),
BlockDigest,
RuntimeRegistry(RuntimeId),
DynamicCostOfStorage,
DomainSudoCall(DomainId),
MmrRoot(Number),
}
Expand All @@ -72,13 +68,11 @@ impl<Number> FraudProofStorageKeyRequest<Number> {
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)
}
Expand Down Expand Up @@ -185,28 +179,6 @@ impl<Block: BlockT> BasicStorageProof<Block> for DomainChainsAllowlistUpdateStor
}
}

#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
pub struct DynamicCostOfStorageProof(StorageProof);

impl_storage_proof!(DynamicCostOfStorageProof);
impl<Block: BlockT> BasicStorageProof<Block> for DynamicCostOfStorageProof {
type StorageValue = bool;
fn storage_key_request(_key: Self::Key) -> FraudProofStorageKeyRequest<NumberFor<Block>> {
FraudProofStorageKeyRequest::DynamicCostOfStorage
}
}

#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
pub struct ConsensusTransactionByteFeeProof(StorageProof);

impl_storage_proof!(ConsensusTransactionByteFeeProof);
impl<Block: BlockT> BasicStorageProof<Block> for ConsensusTransactionByteFeeProof {
type StorageValue = BlockTransactionByteFee<Balance>;
fn storage_key_request(_key: Self::Key) -> FraudProofStorageKeyRequest<NumberFor<Block>> {
FraudProofStorageKeyRequest::TransactionByteFee
}
}

#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
pub struct BlockDigestProof(StorageProof);

Expand Down Expand Up @@ -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 {
Expand All @@ -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,
}
Expand All @@ -434,7 +403,6 @@ pub struct InvalidInherentExtrinsicProof {
#[derive(Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo)]
pub struct InvalidInherentExtrinsicVerified {
pub maybe_domain_runtime_upgrade: Option<Vec<u8>>,
pub consensus_transaction_byte_fee: Balance,
pub domain_chain_allowlist: DomainAllowlistUpdates,
}

Expand All @@ -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,
Expand All @@ -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,
})
}
Expand All @@ -495,25 +449,6 @@ impl InvalidInherentExtrinsicProof {
.maybe_domain_runtime_upgrade_proof
.verify::<Block, SKP>(runtime_id, state_root)?;

let dynamic_cost_of_storage =
<DynamicCostOfStorageProof as BasicStorageProof<Block>>::verify::<SKP>(
self.dynamic_cost_of_storage_proof.clone(),
(),
state_root,
)?;
let consensus_transaction_byte_fee = if dynamic_cost_of_storage {
let raw_transaction_byte_fee =
<ConsensusTransactionByteFeeProof as BasicStorageProof<Block>>::verify::<SKP>(
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 =
<DomainChainsAllowlistUpdateStorageProof as BasicStorageProof<Block>>::verify::<SKP>(
self.domain_chain_allowlist_proof.clone(),
Expand All @@ -523,7 +458,6 @@ impl InvalidInherentExtrinsicProof {

Ok(InvalidInherentExtrinsicVerified {
maybe_domain_runtime_upgrade,
consensus_transaction_byte_fee,
domain_chain_allowlist,
})
}
Expand Down
3 changes: 2 additions & 1 deletion crates/sp-domains-fraud-proof/src/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ where
extrinsics_shuffling_seed: invalid_inherent_extrinsic_data.extrinsics_shuffling_seed,
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,
};
Expand Down
6 changes: 0 additions & 6 deletions crates/subspace-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,19 +1045,13 @@ impl FraudProofStorageKeyProvider<NumberFor<Block>> for StorageKeyProvider {
FraudProofStorageKeyRequest::SuccessfulBundles(domain_id) => {
pallet_domains::SuccessfulBundles::<Runtime>::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)
}
FraudProofStorageKeyRequest::BlockDigest => sp_domains::system_digest_final_key(),
FraudProofStorageKeyRequest::RuntimeRegistry(runtime_id) => {
pallet_domains::RuntimeRegistry::<Runtime>::hashed_key_for(runtime_id)
}
FraudProofStorageKeyRequest::DynamicCostOfStorage => {
pallet_runtime_configs::EnableDynamicCostOfStorage::<Runtime>::hashed_key().to_vec()
}
FraudProofStorageKeyRequest::DomainSudoCall(domain_id) => {
pallet_domains::DomainSudoCalls::<Runtime>::hashed_key_for(domain_id)
}
Expand Down
6 changes: 0 additions & 6 deletions test/subspace-test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1122,19 +1122,13 @@ impl FraudProofStorageKeyProvider<NumberFor<Block>> for StorageKeyProvider {
FraudProofStorageKeyRequest::SuccessfulBundles(domain_id) => {
pallet_domains::SuccessfulBundles::<Runtime>::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)
}
FraudProofStorageKeyRequest::BlockDigest => sp_domains::system_digest_final_key(),
FraudProofStorageKeyRequest::RuntimeRegistry(runtime_id) => {
pallet_domains::RuntimeRegistry::<Runtime>::hashed_key_for(runtime_id)
}
FraudProofStorageKeyRequest::DynamicCostOfStorage => {
pallet_runtime_configs::EnableDynamicCostOfStorage::<Runtime>::hashed_key().to_vec()
}
FraudProofStorageKeyRequest::DomainSudoCall(domain_id) => {
pallet_domains::DomainSudoCalls::<Runtime>::hashed_key_for(domain_id)
}
Expand Down

0 comments on commit 880c03e

Please sign in to comment.