From 412d797a3b17787581546b091b004892518f0606 Mon Sep 17 00:00:00 2001 From: Rim Rakhimov Date: Mon, 10 Jan 2022 18:01:46 +0300 Subject: [PATCH] Implements eip-3607 (#593) * implements eip-3607 * Tests fixed * Added tests for eip3607 functionality * rast fmt --- bin/oe/configuration.rs | 3 +- bin/oe/run.rs | 9 ++- crates/concensus/miner/src/pool/client.rs | 2 + .../concensus/miner/src/pool/tests/client.rs | 6 ++ crates/concensus/miner/src/pool/tests/mod.rs | 75 +++++++++++++++++++ crates/concensus/miner/src/pool/verifier.rs | 18 +++++ .../res/chainspec/test/eip3607_test.json | 47 ++++++++++++ crates/ethcore/src/engines/mod.rs | 8 ++ crates/ethcore/src/miner/miner.rs | 44 ++++++++++- crates/ethcore/src/miner/pool_client.rs | 3 +- crates/ethcore/src/spec/spec.rs | 10 +++ crates/ethcore/types/src/transaction/error.rs | 3 + crates/ethjson/src/spec/params.rs | 2 + crates/rpc/src/v1/helpers/errors.rs | 1 + .../rpc/src/v1/tests/helpers/miner_service.rs | 1 + 15 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 crates/ethcore/res/chainspec/test/eip3607_test.json diff --git a/bin/oe/configuration.rs b/bin/oe/configuration.rs index 5d531b501..e9557fe82 100644 --- a/bin/oe/configuration.rs +++ b/bin/oe/configuration.rs @@ -640,7 +640,7 @@ impl Configuration { fn pool_verification_options(&self) -> Result { Ok(pool::verifier::Options { - // NOTE min_gas_price,block_gas_limit and block_base_fee will be overwritten right after start. + // NOTE min_gas_price,block_gas_limit block_base_fee, and allow_non_eoa_sender will be overwritten right after start. minimal_gas_price: U256::from(20_000_000) * 1_000u32, block_gas_limit: U256::max_value(), block_base_fee: None, @@ -649,6 +649,7 @@ impl Configuration { None => U256::max_value(), }, no_early_reject: self.args.flag_tx_queue_no_early_reject, + allow_non_eoa_sender: false, }) } diff --git a/bin/oe/run.rs b/bin/oe/run.rs index 809c3ed4e..eaf582bc6 100644 --- a/bin/oe/run.rs +++ b/bin/oe/run.rs @@ -370,7 +370,14 @@ pub fn execute(cmd: RunCmd, logger: Arc) -> Result, /// Is this account a local account? pub is_local: bool, } diff --git a/crates/concensus/miner/src/pool/tests/client.rs b/crates/concensus/miner/src/pool/tests/client.rs index 05d99f3ed..98c812375 100644 --- a/crates/concensus/miner/src/pool/tests/client.rs +++ b/crates/concensus/miner/src/pool/tests/client.rs @@ -43,6 +43,7 @@ impl Default for TestClient { account_details: AccountDetails { nonce: 123.into(), balance: 63_100.into(), + code_hash: None, is_local: false, }, gas_required: 21_000.into(), @@ -68,6 +69,11 @@ impl TestClient { self } + pub fn with_code_hash>(mut self, code_hash: T) -> Self { + self.account_details.code_hash = Some(code_hash.into()); + self + } + pub fn with_gas_required>(mut self, gas_required: T) -> Self { self.gas_required = gas_required.into(); self diff --git a/crates/concensus/miner/src/pool/tests/mod.rs b/crates/concensus/miner/src/pool/tests/mod.rs index 807c3baee..9057b6970 100644 --- a/crates/concensus/miner/src/pool/tests/mod.rs +++ b/crates/concensus/miner/src/pool/tests/mod.rs @@ -15,6 +15,7 @@ // along with OpenEthereum. If not, see . use ethereum_types::U256; +use hash::KECCAK_EMPTY; use txpool; use types::transaction::{self, PendingTransaction}; @@ -47,6 +48,7 @@ fn new_queue() -> TransactionQueue { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ) @@ -66,6 +68,7 @@ fn should_return_correct_nonces_when_dropped_because_of_limit() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -127,6 +130,7 @@ fn should_never_drop_local_transactions_from_different_senders() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -272,6 +276,71 @@ fn should_import_transaction_below_min_gas_price_threshold_if_local() { assert_eq!(txq.status().status.transaction_count, 1); } +#[test] +fn should_reject_transaction_from_non_eoa_if_non_eoa_sender_is_not_allowed() { + // given + let txq = new_queue(); + let tx = Tx::default(); + let code_hash = [ + 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, + 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, + 0x0f, 0x0e, + ]; + + // when + let res = txq.import( + TestClient::new().with_code_hash(code_hash), + vec![tx.signed().unverified()], + ); + + // then + assert_eq!(res, vec![Err(transaction::Error::SenderIsNotEOA)]); + assert_eq!(txq.status().status.transaction_count, 0); +} + +#[test] +fn should_import_transaction_from_non_eoa_if_non_eoa_sender_is_allowed() { + // given + let txq = new_queue(); + let tx = Tx::default(); + let code_hash = [ + 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, + 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, 0x0f, 0x0e, 0x0c, 0x0a, + 0x0f, 0x0e, + ]; + txq.set_verifier_options(verifier::Options { + allow_non_eoa_sender: true, + ..Default::default() + }); + + // when + let res = txq.import( + TestClient::new().with_code_hash(code_hash), + vec![tx.signed().unverified()], + ); + + // then + assert_eq!(res, vec![Ok(())]); + assert_eq!(txq.status().status.transaction_count, 1); +} + +#[test] +fn should_import_transaction_if_account_code_hash_is_keccak_empty() { + // given + let txq = new_queue(); + let tx = Tx::default(); + + // when + let res = txq.import( + TestClient::new().with_code_hash(KECCAK_EMPTY), + vec![tx.signed().unverified()], + ); + + // then + assert_eq!(res, vec![Ok(())]); + assert_eq!(txq.status().status.transaction_count, 1); +} + #[test] fn should_import_txs_from_same_sender() { // given @@ -545,6 +614,7 @@ fn should_prefer_current_transactions_when_hitting_the_limit() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -1043,6 +1113,7 @@ fn should_include_local_transaction_to_a_full_pool() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -1076,6 +1147,7 @@ fn should_avoid_verifying_transaction_already_in_pool() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -1112,6 +1184,7 @@ fn should_avoid_reverifying_recently_rejected_transactions() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -1161,6 +1234,7 @@ fn should_reject_early_in_case_gas_price_is_less_than_min_effective() { tx_gas_limit: 1_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); @@ -1204,6 +1278,7 @@ fn should_not_reject_early_in_case_gas_price_is_less_than_min_effective() { tx_gas_limit: 1_000_000.into(), no_early_reject: true, block_base_fee: None, + allow_non_eoa_sender: false, }, PrioritizationStrategy::GasPriceOnly, ); diff --git a/crates/concensus/miner/src/pool/verifier.rs b/crates/concensus/miner/src/pool/verifier.rs index f544ed369..c81be774b 100644 --- a/crates/concensus/miner/src/pool/verifier.rs +++ b/crates/concensus/miner/src/pool/verifier.rs @@ -31,6 +31,7 @@ use std::{ }; use ethereum_types::{H256, U256}; +use hash::KECCAK_EMPTY; use txpool; use types::transaction; @@ -52,6 +53,8 @@ pub struct Options { pub tx_gas_limit: U256, /// Skip checks for early rejection, to make sure that local transactions are always imported. pub no_early_reject: bool, + /// Accept transactions from non EOAs (see EIP-3607) + pub allow_non_eoa_sender: bool, } #[cfg(test)] @@ -63,6 +66,7 @@ impl Default for Options { block_base_fee: None, tx_gas_limit: U256::max_value(), no_early_reject: false, + allow_non_eoa_sender: false, } } } @@ -317,6 +321,20 @@ impl txpool::Verifier let sender = transaction.sender(); let account_details = self.client.account_details(&sender); + if !self.options.allow_non_eoa_sender { + if let Some(code_hash) = account_details.code_hash { + if code_hash != KECCAK_EMPTY { + debug!( + target: "txqueue", + "[{:?}] Rejected tx, sender is not an EOA: {}", + hash, + code_hash + ); + bail!(transaction::Error::SenderIsNotEOA); + } + } + } + let effective_priority_fee = transaction.effective_priority_fee(self.options.block_base_fee); diff --git a/crates/ethcore/res/chainspec/test/eip3607_test.json b/crates/ethcore/res/chainspec/test/eip3607_test.json new file mode 100644 index 000000000..12ca32ecd --- /dev/null +++ b/crates/ethcore/res/chainspec/test/eip3607_test.json @@ -0,0 +1,47 @@ +{ + "name": "Morden", + "engine": { + "null": { + "params": {} + } + }, + "params": { + "gasLimitBoundDivisor": "0x0400", + "accountStartNonce": "0x0", + "maximumExtraDataSize": "0x20", + "minGasLimit": "0x1388", + "networkID" : "0x2", + "registrar" : "0x0000000000000000000000000000000000001337", + "eip140Transition": "0x0", + "eip211Transition": "0x0", + "eip214Transition": "0x0", + "eip658Transition": "0x0", + "eip3607Transition": "0x2" + }, + "genesis": { + "seal": { + "ethereum": { + "nonce": "0x00006d6f7264656e", + "mixHash": "0x00000000000000000000000000000000000000647572616c65787365646c6578" + } + }, + "difficulty": "0x20000", + "author": "0x0000000000000000000000000000000000000000", + "timestamp": "0x00", + "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", + "extraData": "0x", + "gasLimit": "0x2fefd8" + }, + "accounts": { + "0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, + "0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, + "0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, + "0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, + "102e61f5d8f9bc71d0ad4a084df4e65e05ce0e1c": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" }, + "0x71562b71999873DB5b286dF957af199Ec94617F7": { + "balance": "1000000000000000000", + "nonce": "0", + "code": "0xB0B0FACE" + } + } +} \ No newline at end of file diff --git a/crates/ethcore/src/engines/mod.rs b/crates/ethcore/src/engines/mod.rs index 1e940f37e..a80bb33d2 100644 --- a/crates/ethcore/src/engines/mod.rs +++ b/crates/ethcore/src/engines/mod.rs @@ -668,6 +668,14 @@ pub trait EthEngine: Engine<::machine::EthereumMachine> { fn min_gas_limit(&self) -> U256 { self.params().min_gas_limit } + + /// Returns whether transactions from non externally owned accounts (EOA) + /// are allowed in the given block number (see EIP-3607). + /// + /// That is only possible if EIP-3607 is still not activated. + fn allow_non_eoa_sender(&self, best_block_number: BlockNumber) -> bool { + self.params().eip3607_transition > best_block_number + } } // convenience wrappers for existing functions. diff --git a/crates/ethcore/src/miner/miner.rs b/crates/ethcore/src/miner/miner.rs index f9515ac03..21cf1d05d 100644 --- a/crates/ethcore/src/miner/miner.rs +++ b/crates/ethcore/src/miner/miner.rs @@ -190,6 +190,7 @@ impl Default for MinerOptions { block_base_fee: None, tx_gas_limit: U256::max_value(), no_early_reject: false, + allow_non_eoa_sender: false, }, } } @@ -341,6 +342,7 @@ impl Miner { block_base_fee: None, tx_gas_limit: U256::max_value(), no_early_reject: false, + allow_non_eoa_sender: false, }, reseal_min_period: Duration::from_secs(0), force_sealing, @@ -382,6 +384,7 @@ impl Miner { &self, block_gas_limit: U256, block_base_fee: Option, + allow_non_eoa_sender: bool, ) { trace!(target: "miner", "minimal_gas_price: recalibrating..."); let txq = self.transaction_queue.clone(); @@ -391,6 +394,7 @@ impl Miner { options.minimal_gas_price = gas_price; options.block_gas_limit = block_gas_limit; options.block_base_fee = block_base_fee; + options.allow_non_eoa_sender = allow_non_eoa_sender; txq.set_verifier_options(options); }); @@ -1466,7 +1470,10 @@ impl miner::MinerService for Miner { } else { 1 }; - self.update_transaction_queue_limits(gas_limit, base_fee); + let allow_non_eoa_sender = self + .engine + .allow_non_eoa_sender(chain.best_block_header().number() + 1); + self.update_transaction_queue_limits(gas_limit, base_fee, allow_non_eoa_sender); // t_nb 10.2 Then import all transactions from retracted blocks (retracted means from side chain). let client = self.pool_client(chain); @@ -1670,6 +1677,7 @@ mod tests { block_base_fee: None, tx_gas_limit: U256::max_value(), no_early_reject: false, + allow_non_eoa_sender: false, }, }, GasPricer::new_fixed(0u64.into()), @@ -1815,6 +1823,40 @@ mod tests { ); } + #[test] + fn should_activate_eip_3607_according_to_spec() { + // given + let spec = Spec::new_test_eip3607(); + let miner = Miner::new_for_tests(&spec, None); + let client = TestBlockChainClient::new_with_spec(spec); + + let imported = [H256::zero()]; + let empty = &[]; + + // the client best block is below EIP-3607 transition number + miner.chain_new_blocks(&client, &imported, empty, &imported, empty, false); + assert!( + miner.queue_status().options.allow_non_eoa_sender, + "The client best block is below EIP-3607 transition number. Non EOA senders should be allowed" + ); + + // the client best block equals EIP-3607 transition number + client.add_block(EachBlockWith::Nothing, |header| header); + miner.chain_new_blocks(&client, &imported, empty, &imported, empty, false); + assert!( + !miner.queue_status().options.allow_non_eoa_sender, + "The client best block equals EIP-3607 transition number. Non EOA senders should not be allowed" + ); + + // the client best block is above EIP-3607 transition number + client.add_block(EachBlockWith::Nothing, |header| header); + miner.chain_new_blocks(&client, &imported, empty, &imported, empty, false); + assert!( + !miner.queue_status().options.allow_non_eoa_sender, + "The client best block is above EIP-3607 transition number. Non EOA senders should not be allowed" + ); + } + #[test] fn should_treat_unfamiliar_locals_selectively() { // given diff --git a/crates/ethcore/src/miner/pool_client.rs b/crates/ethcore/src/miner/pool_client.rs index f9fdbc768..b39dc996b 100644 --- a/crates/ethcore/src/miner/pool_client.rs +++ b/crates/ethcore/src/miner/pool_client.rs @@ -30,7 +30,7 @@ use types::{ }; use call_contract::CallContract; -use client::{BlockInfo, Nonce, TransactionId}; +use client::{BlockId, BlockInfo, Nonce, TransactionId}; use engines::EthEngine; use miner; use transaction_ext::Transaction; @@ -168,6 +168,7 @@ where pool::client::AccountDetails { nonce: self.cached_nonces.account_nonce(address), balance: self.chain.latest_balance(address), + code_hash: self.chain.code_hash(address, BlockId::Latest), is_local: self.accounts.is_local(address), } } diff --git a/crates/ethcore/src/spec/spec.rs b/crates/ethcore/src/spec/spec.rs index b50d859d3..bc903c9f7 100644 --- a/crates/ethcore/src/spec/spec.rs +++ b/crates/ethcore/src/spec/spec.rs @@ -147,6 +147,8 @@ pub struct CommonParams { pub eip3529_transition: BlockNumber, /// Number of first block where EIP-3541 rule begins. pub eip3541_transition: BlockNumber, + /// Number of first block where EIP-3607 rule begins. + pub eip3607_transition: BlockNumber, /// Number of first block where dust cleanup rules (EIP-168 and EIP169) begin. pub dust_protection_transition: BlockNumber, /// Nonce cap increase per block. Nonce cap is only checked if dust protection is enabled. @@ -434,6 +436,7 @@ impl From for CommonParams { dust_protection_transition: p .dust_protection_transition .map_or_else(BlockNumber::max_value, Into::into), + eip3607_transition: p.eip3607_transition.map_or(0, Into::into), nonce_cap_increment: p.nonce_cap_increment.map_or(64, Into::into), remove_dust_contracts: p.remove_dust_contracts.unwrap_or(false), gas_limit_bound_divisor: p.gas_limit_bound_divisor.into(), @@ -1151,6 +1154,13 @@ impl Spec { load_bundled!("test/constructor") } + /// Create a new Spec which is a NullEngine consensus with EIP3607 transition equal to 2, + /// and with a contract at address '0x71562b71999873DB5b286dF957af199Ec94617F7'. + #[cfg(any(test, feature = "test-helpers"))] + pub fn new_test_eip3607() -> Self { + load_bundled!("test/eip3607_test") + } + /// Create a new Spec with Autority Round randomness contract #[cfg(any(test, feature = "test-helpers"))] pub fn new_test_round_randomness_contract() -> Spec { diff --git a/crates/ethcore/types/src/transaction/error.rs b/crates/ethcore/types/src/transaction/error.rs index b8094d650..b4dac5676 100644 --- a/crates/ethcore/types/src/transaction/error.rs +++ b/crates/ethcore/types/src/transaction/error.rs @@ -93,6 +93,8 @@ pub enum Error { InvalidRlp(String), /// Transaciton is still not enabled. TransactionTypeNotEnabled, + /// Transaction sender is not an EOA (see EIP-3607) + SenderIsNotEOA, } impl From for Error { @@ -154,6 +156,7 @@ impl fmt::Display for Error { TransactionTypeNotEnabled => { format!("Transaction type is not enabled for current block") } + SenderIsNotEOA => "Transaction sender is not an EOA (see EIP-3607)".into(), }; f.write_fmt(format_args!("Transaction error ({})", msg)) diff --git a/crates/ethjson/src/spec/params.rs b/crates/ethjson/src/spec/params.rs index 9e157ba1b..4ddfe86b2 100644 --- a/crates/ethjson/src/spec/params.rs +++ b/crates/ethjson/src/spec/params.rs @@ -120,6 +120,8 @@ pub struct Params { /// See `CommonParams` docs. pub eip3541_transition: Option, /// See `CommonParams` docs. + pub eip3607_transition: Option, + /// See `CommonParams` docs. pub dust_protection_transition: Option, /// See `CommonParams` docs. pub nonce_cap_increment: Option, diff --git a/crates/rpc/src/v1/helpers/errors.rs b/crates/rpc/src/v1/helpers/errors.rs index 1883a82f0..94585e03c 100644 --- a/crates/rpc/src/v1/helpers/errors.rs +++ b/crates/rpc/src/v1/helpers/errors.rs @@ -408,6 +408,7 @@ pub fn transaction_message(error: &TransactionError) -> String { TooBig => "Transaction is too big, see chain specification for the limit.".into(), InvalidRlp(ref descr) => format!("Invalid RLP data: {}", descr), TransactionTypeNotEnabled => format!("Transaction type is not enabled for current block"), + SenderIsNotEOA => "Transaction sender is not an EOA (see EIP-3607)".into(), } } diff --git a/crates/rpc/src/v1/tests/helpers/miner_service.rs b/crates/rpc/src/v1/tests/helpers/miner_service.rs index 03fd98741..c81bb0a49 100644 --- a/crates/rpc/src/v1/tests/helpers/miner_service.rs +++ b/crates/rpc/src/v1/tests/helpers/miner_service.rs @@ -335,6 +335,7 @@ impl MinerService for TestMinerService { tx_gas_limit: 5_000_000.into(), no_early_reject: false, block_base_fee: None, + allow_non_eoa_sender: false, }, status: txpool::LightStatus { mem_usage: 1_000,