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.

Sign up for GitHub

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

add pallet::call to hotfix sufficients for non-zero nonce accounts #619

Merged
merged 12 commits into from
Jun 2, 2022
Merged

add pallet::call to hotfix sufficients for non-zero nonce accounts #619

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fc65741
add pallet::call to hotfix sufficients for non-zero nonce accounts
tgmichel Mar 31, 2022
57cdd4b
make hotfix to fix sufficients payable
nbaztec May 4, 2022
ab3b902
Merge branch 'master' into nish-upstream-hotfix
nbaztec May 4, 2022
5bf2723
cargo fmt
nbaztec May 4, 2022
8a2b6d1
Merge branch 'master' into nish-upstream-hotfix
nbaztec May 10, 2022
c52d1ac
use distinct pallet for hotfix
nbaztec May 31, 2022
8a7177c
cleanup traces from pallet evm
nbaztec May 31, 2022
2bdee74
add pallet-hotfix-sufficients
nbaztec May 31, 2022
fb507fa
fix description
nbaztec Jun 1, 2022
411ca3d
fix benchmarking
nbaztec Jun 1, 2022
b121bdf
update test condition, add docs
nbaztec Jun 1, 2022
748ef65
use safe add
nbaztec Jun 1, 2022
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
Prev Previous commit
Next Next commit
use distinct pallet for hotfix
  • Loading branch information
@nbaztec
nbaztec committed May 31, 2022
commit c52d1ac60cb4599b39738e5998a9c3682651ece3
16 changes: 16 additions & 0 deletions Cargo.lock
View file
Open in desktop

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

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ members = [
"frame/dynamic-fee",
"frame/ethereum",
"frame/evm",
"frame/hotfix-sufficients",
"frame/evm/precompile/sha3fips",
"frame/evm/precompile/simple",
"frame/evm/precompile/modexp",
Expand Down
38 changes: 0 additions & 38 deletions frame/evm/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,44 +124,6 @@ benchmarks! {
);
assert_eq!(call_runner_results.is_ok(), true, "call() failed");
}


hotfix_inc_account_sufficients {
// This benchmark tests the resource utilization by hotfixing N number of accounts
// by incrementing their `sufficients` if `nonce` is > 0.

let n in 0 .. 1000;

use frame_benchmarking::{vec, whitelisted_caller};
use sp_core::H160;
use frame_system::RawOrigin;

// The caller account is whitelisted for DB reads/write by the benchmarking macro.
let caller: T::AccountId = whitelisted_caller();
let addresses = (0..n as u64)
.map(H160::from_low_u64_le)
.collect::<Vec<H160>>();
let accounts = addresses
.iter()
.cloned()
.map(|addr| {
<crate::AccountCodes<T>>::insert(addr, &vec![0]);
let account_id = T::AddressMapping::into_account_id(addr);
frame_system::Pallet::<T>::inc_account_nonce(&account_id);
assert_eq!(frame_system::Pallet::<T>::sufficients(&account_id), 0);

account_id
})
.collect::<Vec<_>>();

}: _(RawOrigin::Signed(caller), addresses)
verify {
accounts
.iter()
.for_each(|id| {
assert_eq!(frame_system::Pallet::<T>::sufficients(&id), 1);
});
}
}

impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::tests::Test);
35 changes: 0 additions & 35 deletions frame/evm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@

#[cfg(any(test, feature = "runtime-benchmarks"))]
pub mod benchmarking;
pub mod weights;
pub use weights::WeightInfo;

#[cfg(test)]
mod mock;
Expand Down Expand Up @@ -144,9 +142,6 @@ pub mod pallet {
/// Find author for the current block.
type FindAuthor: FindAuthor<H160>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// EVM config used in the module.
fn config() -> &'static EvmConfig {
&LONDON_CONFIG
Expand Down Expand Up @@ -333,36 +328,6 @@ pub mod pallet {
pays_fee: Pays::No,
})
}

/// Increment `sufficients` for existing accounts having a nonzero `nonce` but zero `sufficients` value.
#[pallet::weight(
<T as pallet::Config>::WeightInfo::hotfix_inc_account_sufficients(addresses.len().try_into().unwrap_or(u32::MAX))
)]
pub fn hotfix_inc_account_sufficients(
origin: OriginFor<T>,
addresses: Vec<H160>,
) -> DispatchResultWithPostInfo {
const MAX_ADDRESS_COUNT: usize = 1000;

frame_system::ensure_signed(origin)?;
ensure!(
addresses.len() <= MAX_ADDRESS_COUNT,
Error::<T>::MaxAddressCountExceeded
);

for address in addresses {
let account_id = T::AddressMapping::into_account_id(address);
let nonce = frame_system::Pallet::<T>::account_nonce(&account_id);
if !nonce.is_zero() {
frame_system::Pallet::<T>::inc_sufficients(&account_id);
}
}

Ok(PostDispatchInfo {
actual_weight: None,
pays_fee: Pays::Yes,
})
}
}

#[pallet::event]
Expand Down
85 changes: 0 additions & 85 deletions frame/evm/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,91 +404,6 @@ fn handle_sufficient_reference() {
});
}

#[test]
fn test_hotfix_inc_account_sufficients_returns_error_if_max_addresses_exceeded() {
new_test_ext().execute_with(|| {
let max_address_count = 1000;
let addresses = (0..max_address_count + 1 as u64)
.map(H160::from_low_u64_le)
.collect::<Vec<H160>>();

let result =
EVM::hotfix_inc_account_sufficients(Origin::signed(H160::default()), addresses);

assert!(result.is_err(), "expected error");
});
}

#[test]
fn test_hotfix_inc_account_sufficients_requires_signed_origin() {
new_test_ext().execute_with(|| {
let addr = H160::from_str("1230000000000000000000000000000000000001").unwrap();
<crate::AccountCodes<Test>>::insert(addr, &vec![0]);

let unsigned_origin = Origin::root();
let result = EVM::hotfix_inc_account_sufficients(unsigned_origin, vec![addr]);

assert!(result.is_err(), "expected error");
});
}

#[test]
fn test_hotfix_inc_account_sufficients_increments_if_nonce_nonzero() {
new_test_ext().execute_with(|| {
let addr_1 = H160::from_str("1230000000000000000000000000000000000001").unwrap();
let addr_2 = H160::from_str("1234000000000000000000000000000000000001").unwrap();
let substrate_addr_1 = <Test as Config>::AddressMapping::into_account_id(addr_1);
let substrate_addr_2 = <Test as Config>::AddressMapping::into_account_id(addr_2);

<crate::AccountCodes<Test>>::insert(addr_1, &vec![0]);
<crate::AccountCodes<Test>>::insert(addr_2, &vec![0]);

frame_system::Pallet::<Test>::inc_account_nonce(&substrate_addr_1);

let account_1 = frame_system::Account::<Test>::get(substrate_addr_1);
let account_2 = frame_system::Account::<Test>::get(substrate_addr_2);
assert_eq!(account_1.nonce, 1);
assert_eq!(account_1.sufficients, 0);
assert_eq!(account_2.nonce, 0);
assert_eq!(account_2.sufficients, 0);

EVM::hotfix_inc_account_sufficients(Origin::signed(H160::default()), vec![addr_1, addr_2])
.unwrap();

let account_1 = frame_system::Account::<Test>::get(substrate_addr_1);
let account_2 = frame_system::Account::<Test>::get(substrate_addr_2);
assert_eq!(account_1.nonce, 1);
assert_eq!(account_1.sufficients, 1);
assert_eq!(account_2.nonce, 0);
assert_eq!(account_2.sufficients, 0);
});
}

#[test]
fn test_hotfix_inc_account_sufficients_increments_with_saturation_if_nonce_nonzero() {
new_test_ext().execute_with(|| {
let addr = H160::from_str("1230000000000000000000000000000000000001").unwrap();
let substrate_addr = <Test as Config>::AddressMapping::into_account_id(addr);

<crate::AccountCodes<Test>>::insert(addr, &vec![0]);
frame_system::Account::<Test>::mutate(substrate_addr, |x| {
x.nonce = 1;
x.sufficients = u32::MAX;
});

let account = frame_system::Account::<Test>::get(substrate_addr);

assert_eq!(account.sufficients, u32::MAX);
assert_eq!(account.nonce, 1);

EVM::hotfix_inc_account_sufficients(Origin::signed(H160::default()), vec![addr]).unwrap();

let account = frame_system::Account::<Test>::get(substrate_addr);
assert_eq!(account.sufficients, u32::MAX);
assert_eq!(account.nonce, 1);
});
}

#[test]
fn runner_non_transactional_calls_with_non_balance_accounts_is_ok_without_gas_price() {
// Expect to skip checks for gas price and account balance when both:
Expand Down
95 changes: 0 additions & 95 deletions frame/evm/src/weights.rs
View file
Open in desktop

This file was deleted.