Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in fraud proof transaction fee value, and combine storage #3320

Merged
merged 2 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(|_| ())
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
.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
2 changes: 1 addition & 1 deletion crates/sp-domains-fraud-proof/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NumberFor<Block>, 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<NumberFor<Block>>) -> Vec<u8>;
}
}
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 @@ -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,
};
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
Loading