Skip to content

Commit

Permalink
Merge pull request #2723 from subspace/evm_fixes
Browse files Browse the repository at this point in the history
Domains: update evm runtimes to account for nonce non updates during the `pre_dispatch`
  • Loading branch information
vedhavyas authored Apr 30, 2024
2 parents eb09630 + 65d6a64 commit 8a1dadf
Show file tree
Hide file tree
Showing 13 changed files with 517 additions and 35 deletions.
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion crates/pallet-domains/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use sp_domains_fraud_proof::{
use sp_runtime::traits::{
AccountIdConversion, BlakeTwo256, BlockNumberProvider, Hash as HashT, IdentityLookup, One,
};
use sp_runtime::transaction_validity::TransactionValidityError;
use sp_runtime::{BuildStorage, Digest, OpaqueExtrinsic, Saturating};
use sp_state_machine::backend::AsTrieBackend;
use sp_state_machine::{prove_read, Backend, TrieBackendBuilder};
Expand Down Expand Up @@ -299,7 +300,12 @@ impl domain_pallet_executive::ExtrinsicStorageFees<Test> for ExtrinsicStorageFee
(None, DispatchInfo::default())
}

fn on_storage_fees_charged(_charged_fees: Balance, _tx_size: u32) {}
fn on_storage_fees_charged(
_charged_fees: Balance,
_tx_size: u32,
) -> Result<(), TransactionValidityError> {
Ok(())
}
}

impl domain_pallet_executive::Config for Test {
Expand Down
32 changes: 32 additions & 0 deletions domains/pallets/evm_nonce_tracker/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[package]
name = "pallet-evm-nonce-tracker"
version = "0.1.0"
authors = ["Subspace Labs <https://subspace.network>"]
edition = "2021"
license = "Apache-2.0"
homepage = "https://subspace.network"
repository = "https://github.com/subspace/subspace"
description = "Subspace node pallet for EVM account nonce tracker."
include = [
"/src",
"/Cargo.toml",
]

[dependencies]
codec = { package = "parity-scale-codec", version = "3.6.5", default-features = false, features = ["derive"] }
frame-support = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" }
frame-system = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" }
scale-info = { version = "2.11.1", default-features = false, features = ["derive"] }
sp-core = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" }
sp-runtime = { default-features = false, git = "https://github.com/subspace/polkadot-sdk", rev = "44d742b90e7852aed1f08ab5299d5d88cfa1c6ed" }

[features]
default = ["std"]
std = [
"codec/std",
"frame-support/std",
"frame-system/std",
"scale-info/std",
"sp-core/std",
"sp-runtime/std",
]
125 changes: 125 additions & 0 deletions domains/pallets/evm_nonce_tracker/src/check_nonce.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use crate::Config;
#[cfg(not(feature = "std"))]
use alloc::fmt;
#[cfg(not(feature = "std"))]
use alloc::vec;
use codec::{Decode, Encode};
use core::cmp::max;
use core::result::Result;
use frame_support::dispatch::DispatchInfo;
use frame_support::pallet_prelude::{
InvalidTransaction, TransactionLongevity, TransactionValidity, TransactionValidityError,
TypeInfo, ValidTransaction,
};
use frame_support::sp_runtime::traits::{DispatchInfoOf, One, SignedExtension};
use sp_runtime::traits::{Dispatchable, Zero};
#[cfg(feature = "std")]
use std::fmt;
#[cfg(feature = "std")]
use std::vec;

#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckNonce<T: Config>(#[codec(compact)] pub T::Nonce);

impl<T: Config> CheckNonce<T> {
/// utility constructor. Used only in client/factory code.
pub fn from(nonce: T::Nonce) -> Self {
Self(nonce)
}
}

impl<T: Config> fmt::Debug for CheckNonce<T> {
#[cfg(feature = "std")]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "CheckNonce({})", self.0)
}

#[cfg(not(feature = "std"))]
fn fmt(&self, _: &mut fmt::Formatter) -> fmt::Result {
Ok(())
}
}

impl<T: Config> SignedExtension for CheckNonce<T>
where
T::RuntimeCall: Dispatchable<Info = DispatchInfo>,
{
const IDENTIFIER: &'static str = "CheckNonce";
type AccountId = T::AccountId;
type Call = T::RuntimeCall;
type AdditionalSigned = ();
type Pre = ();

fn additional_signed(&self) -> Result<(), TransactionValidityError> {
Ok(())
}

fn validate(
&self,
who: &Self::AccountId,
_call: &Self::Call,
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> TransactionValidity {
let account = frame_system::Account::<T>::get(who);
if account.providers.is_zero() && account.sufficients.is_zero() {
// Nonce storage not paid for
return InvalidTransaction::Payment.into();
}
if self.0 < account.nonce {
return InvalidTransaction::Stale.into();
}

let provides = vec![Encode::encode(&(who, self.0))];
let requires = if account.nonce < self.0 {
vec![Encode::encode(&(who, self.0 - One::one()))]
} else {
vec![]
};

Ok(ValidTransaction {
priority: 0,
requires,
provides,
longevity: TransactionLongevity::MAX,
propagate: true,
})
}

fn pre_dispatch(
self,
who: &Self::AccountId,
_call: &Self::Call,
_info: &DispatchInfoOf<Self::Call>,
_len: usize,
) -> Result<(), TransactionValidityError> {
let mut account = frame_system::Account::<T>::get(who);
if account.providers.is_zero() && account.sufficients.is_zero() {
// Nonce storage not paid for
return Err(InvalidTransaction::Payment.into());
}
// if a sender sends an evm transaction first and substrate transaction
// after with same nonce, then reject the second transaction
// if sender reverse the transaction types, substrate first and evm second,
// evm transaction will be rejected, since substrate updates nonce in pre_dispatch.
let account_nonce = if let Some(tracked_nonce) = crate::AccountNonce::<T>::get(who.clone())
{
max(tracked_nonce.as_u32().into(), account.nonce)
} else {
account.nonce
};

if self.0 != account_nonce {
return Err(if self.0 < account.nonce {
InvalidTransaction::Stale
} else {
InvalidTransaction::Future
}
.into());
}
account.nonce += T::Nonce::one();
frame_system::Account::<T>::insert(who, account);
Ok(())
}
}
69 changes: 69 additions & 0 deletions domains/pallets/evm_nonce_tracker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (C) 2023 Subspace Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! Pallet EVM Account nonce tracker

#![cfg_attr(not(feature = "std"), no_std)]

#[cfg(not(feature = "std"))]
extern crate alloc;

mod check_nonce;
pub use check_nonce::CheckNonce;
pub use pallet::*;
use sp_core::U256;

#[frame_support::pallet]
mod pallet {
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::BlockNumberFor;
use sp_core::U256;

#[pallet::config]
pub trait Config: frame_system::Config {}

/// Storage to hold evm account nonces.
/// This is only used for pre_dispatch since EVM pre_dispatch does not
/// increment account nonce.
#[pallet::storage]
pub(super) type AccountNonce<T: Config> =
StorageMap<_, Identity, T::AccountId, U256, OptionQuery>;

/// Pallet EVM account nonce tracker.
#[pallet::pallet]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_finalize(_now: BlockNumberFor<T>) {
// clear the storage since we would start with updated nonce
// during the pre_dispatch in next block
let _ = AccountNonce::<T>::clear(u32::MAX, None);
}
}
}

impl<T: Config> Pallet<T> {
/// Returns current nonce for the given account.
pub fn account_nonce(account: T::AccountId) -> Option<U256> {
AccountNonce::<T>::get(account)
}

/// Set nonce to the account.
pub fn set_account_nonce(account: T::AccountId, nonce: U256) {
AccountNonce::<T>::set(account, Some(nonce))
}
}
7 changes: 5 additions & 2 deletions domains/pallets/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ pub trait ExtrinsicStorageFees<T: Config> {
/// Extracts signer from given extrinsic and its dispatch info.
fn extract_signer(xt: ExtrinsicOf<T>) -> (Option<AccountIdOf<T>>, DispatchInfo);
/// Hook to note operator rewards for charged storage fees.
fn on_storage_fees_charged(charged_fees: BalanceOf<T>, tx_size: u32);
fn on_storage_fees_charged(
charged_fees: BalanceOf<T>,
tx_size: u32,
) -> Result<(), TransactionValidityError>;
}

type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
Expand Down Expand Up @@ -501,7 +504,7 @@ where
ExecutiveConfig::ExtrinsicStorageFees::on_storage_fees_charged(
charged_fees,
encoded.len() as u32,
)
)?;
}
}

Expand Down
8 changes: 7 additions & 1 deletion domains/pallets/executive/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use frame_support::weights::IdentityFee;
use frame_support::{derive_impl, parameter_types};
use frame_system::mocking::MockUncheckedExtrinsic;
use pallet_balances::AccountData;
use sp_runtime::transaction_validity::TransactionValidityError;
use sp_runtime::BuildStorage;

type Block = frame_system::mocking::MockBlock<MockRuntime>;
Expand Down Expand Up @@ -46,7 +47,12 @@ impl crate::ExtrinsicStorageFees<MockRuntime> for ExtrinsicStorageFees {
(None, DispatchInfo::default())
}

fn on_storage_fees_charged(_charged_fees: Balance, _tx_size: u32) {}
fn on_storage_fees_charged(
_charged_fees: Balance,
_tx_size: u32,
) -> Result<(), TransactionValidityError> {
Ok(())
}
}

impl Config for MockRuntime {
Expand Down
5 changes: 5 additions & 0 deletions domains/primitives/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ pub const MAXIMUM_MANDATORY_BLOCK_LENGTH: u32 = 5 * 1024 * 1024;
/// Maximum block length for operational and normal dispatches.
pub const MAXIMUM_OPERATIONAL_AND_NORMAL_BLOCK_LENGTH: u32 = u32::MAX;

/// Custom error when nonce overflow occurs.
pub const ERR_NONCE_OVERFLOW: u8 = 100;
/// Custom error when balance overflow occurs.
pub const ERR_BALANCE_OVERFLOW: u8 = 200;

/// Maximum block length for all dispatches.
pub fn maximum_block_length() -> BlockLength {
BlockLength {
Expand Down
12 changes: 9 additions & 3 deletions domains/runtime/auto-id/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use domain_runtime_primitives::{
};
use domain_runtime_primitives::{
AccountId, Address, CheckExtrinsicsValidityError, DecodeExtrinsicError, Signature,
SLOT_DURATION,
ERR_BALANCE_OVERFLOW, SLOT_DURATION,
};
use frame_support::dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo};
use frame_support::genesis_builder_helper::{build_config, create_default_config};
Expand Down Expand Up @@ -269,8 +269,13 @@ impl domain_pallet_executive::ExtrinsicStorageFees<Runtime> for ExtrinsicStorage
(maybe_signer, dispatch_info)
}

fn on_storage_fees_charged(charged_fees: Balance, tx_size: u32) {
let consensus_storage_fee = BlockFees::consensus_chain_byte_fee() * Balance::from(tx_size);
fn on_storage_fees_charged(
charged_fees: Balance,
tx_size: u32,
) -> Result<(), TransactionValidityError> {
let consensus_storage_fee = BlockFees::consensus_chain_byte_fee()
.checked_mul(Balance::from(tx_size))
.ok_or(InvalidTransaction::Custom(ERR_BALANCE_OVERFLOW))?;

let (paid_consensus_storage_fee, paid_domain_fee) = if charged_fees <= consensus_storage_fee
{
Expand All @@ -281,6 +286,7 @@ impl domain_pallet_executive::ExtrinsicStorageFees<Runtime> for ExtrinsicStorage

BlockFees::note_consensus_storage_fee(paid_consensus_storage_fee);
BlockFees::note_domain_execution_fee(paid_domain_fee);
Ok(())
}
}

Expand Down
2 changes: 2 additions & 0 deletions domains/runtime/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pallet-domain-id = { version = "0.1.0", path = "../../pallets/domain-id", defaul
pallet-ethereum = { default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
pallet-evm = { version = "6.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
pallet-evm-chain-id = { version = "1.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
pallet-evm-nonce-tracker = { version = "0.1.0", path = "../../pallets/evm_nonce_tracker", default-features = false }
pallet-evm-precompile-modexp = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
pallet-evm-precompile-sha3fips = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
pallet-evm-precompile-simple = { version = "2.0.0-dev", default-features = false, git = "https://github.com/subspace/frontier", rev = "4354330f71535a5459b8e3c7e7ed0c0d612b5e0e" }
Expand Down Expand Up @@ -93,6 +94,7 @@ std = [
"pallet-ethereum/std",
"pallet-evm/std",
"pallet-evm-chain-id/std",
"pallet-evm-nonce-tracker/std",
"pallet-evm-precompile-modexp/std",
"pallet-evm-precompile-sha3fips/std",
"pallet-evm-precompile-simple/std",
Expand Down
Loading

0 comments on commit 8a1dadf

Please sign in to comment.