From c1e4b5af5ec1180f39274c39a1f071f61d9f5469 Mon Sep 17 00:00:00 2001 From: Jouzo <15011228+Jouzo@users.noreply.github.com> Date: Fri, 9 Jun 2023 11:27:27 +0200 Subject: [PATCH] Return nonce informations to miner (#2025) * Return nonce on evm_prevalidate_raw_tx * Catch thrown error from evm_prevalidate_raw_tx * Returns sender address in evm_prevalidate_raw_tx * Check if nonce is the expected value * Get evm nonce and balance at latest block height * With RustRes instead of throw * Use RustRes in evm_try_queue_tx * Parse metadata into txMessage * Add evm_get_nonce_in_context FFI method * Document TransactionQueue nonce handling * Return InvalidNonce if queued nonce is not in increasing order * Add unit test for nonce order * Use FFI evm_get_nonce_in_context * Get the next valid nonce * Track failed nonces and try them once other TXs are added * Prevalidate all TXs with nonce > account nonce * Test transactions added in order * Time order mempool TXs * Remove commented out gas check in validate raw tx * Add more EVM TXs for test --------- Co-authored-by: Peter Bushnell --- lib/ain-evm/src/evm.rs | 48 ++++++++-- lib/ain-evm/src/tx_queue.rs | 151 ++++++++++++++++++++++++++++++- lib/ain-grpc/src/rpc/eth.rs | 2 +- lib/ain-rs-exports/src/lib.rs | 122 +++++++++++++++++++++---- src/masternodes/mn_checks.cpp | 17 +++- src/masternodes/rpc_accounts.cpp | 5 +- src/miner.cpp | 40 +++++++- src/validation.cpp | 58 ++++++------ test/functional/feature_evm.py | 62 ++++++++----- 9 files changed, 418 insertions(+), 87 deletions(-) diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index 45a17032446..24690868c42 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -166,7 +166,7 @@ impl EVMHandler { signed_tx.nonce() ); debug!("[validate_raw_tx] nonce : {:#?}", nonce); - if nonce != signed_tx.nonce() { + if nonce > signed_tx.nonce() { return Err(anyhow!( "Invalid nonce. Account nonce {}, signed_tx nonce {}", nonce, @@ -175,11 +175,6 @@ impl EVMHandler { .into()); } - // TODO validate balance to pay gas - // if account.balance < MIN_GAS { - // return Err(anyhow!("Insufficiant balance to pay fees").into()); - // } - Ok(signed_tx) } @@ -316,6 +311,47 @@ impl EVMHandler { debug!("Account {:x?} nonce {:x?}", address, nonce); Ok(nonce) } + + /// Retrieves the next valid nonce for the specified account within a particular context. + /// + /// The method first attempts to retrieve the next valid nonce from the transaction queue associated with the + /// provided context. If no nonce is found in the transaction queue, that means that no transactions have been + /// queued for this account in this context. It falls back to retrieving the nonce from the storage at the latest + /// block. If no nonce is found in the storage (i.e., no transactions for this account have been committed yet), + /// the nonce is defaulted to zero. + /// + /// This method provides a unified view of the nonce for an account, taking into account both transactions that are + /// waiting to be processed in the queue and transactions that have already been processed and committed to the storage. + /// + /// # Arguments + /// + /// * `context` - The context queue number. + /// * `address` - The EVM address of the account whose nonce we want to retrieve. + /// + /// # Returns + /// + /// Returns the next valid nonce as a `U256`. Defaults to U256::zero() + pub fn get_next_valid_nonce_in_context(&self, context: u64, address: H160) -> U256 { + let nonce = self + .tx_queues + .get_next_valid_nonce(context, address) + .unwrap_or_else(|| { + let latest_block = self + .storage + .get_latest_block() + .map(|b| b.header.number) + .unwrap_or_else(|| U256::zero()); + + self.get_nonce(address, latest_block) + .unwrap_or_else(|_| U256::zero()) + }); + + debug!( + "Account {:x?} nonce {:x?} in context {context}", + address, nonce + ); + nonce + } } use std::fmt; diff --git a/lib/ain-evm/src/tx_queue.rs b/lib/ain-evm/src/tx_queue.rs index 9997661faf0..a037cb53ab4 100644 --- a/lib/ain-evm/src/tx_queue.rs +++ b/lib/ain-evm/src/tx_queue.rs @@ -1,3 +1,4 @@ +use ethereum_types::{H160, U256}; use rand::Rng; use std::{ collections::HashMap, @@ -20,6 +21,9 @@ impl Default for TransactionQueueMap { } } +/// Holds multiple `TransactionQueue`s, each associated with a unique context ID. +/// +/// Context IDs are randomly generated and used to access distinct transaction queues. impl TransactionQueueMap { pub fn new() -> Self { TransactionQueueMap { @@ -27,6 +31,8 @@ impl TransactionQueueMap { } } + /// `get_context` generates a unique random ID, creates a new `TransactionQueue` for that ID, + /// and then returns the ID. pub fn get_context(&self) -> u64 { let mut rng = rand::thread_rng(); loop { @@ -40,10 +46,13 @@ impl TransactionQueueMap { } } + /// Try to remove and return the `TransactionQueue` associated with the provided + /// context ID. pub fn remove(&self, context_id: u64) -> Option { self.queues.write().unwrap().remove(&context_id) } + /// Clears the `TransactionQueue` vector associated with the provided context ID. pub fn clear(&self, context_id: u64) -> Result<(), QueueError> { self.queues .read() @@ -53,6 +62,20 @@ impl TransactionQueueMap { .map(TransactionQueue::clear) } + /// Attempts to add a new transaction to the `TransactionQueue` associated with the + /// provided context ID. If the transaction is a `SignedTx`, it also updates the + /// corresponding account's nonce. + /// Nonces for each account's transactions must be in strictly increasing order. This means that if the last + /// queued transaction for an account has nonce 3, the next one should have nonce 4. If a `SignedTx` with a nonce + /// that is not one more than the previous nonce is added, an error is returned. This helps to ensure the integrity + /// of the transaction queue and enforce correct nonce usage. + /// + /// # Errors + /// + /// Returns `QueueError::NoSuchContext` if no queue is associated with the given context ID. + /// Returns `QueueError::InvalidNonce` if a `SignedTx` is provided with a nonce that is not one more than the + /// previous nonce of transactions from the same sender in the queue. + /// pub fn queue_tx( &self, context_id: u64, @@ -64,9 +87,12 @@ impl TransactionQueueMap { .unwrap() .get(&context_id) .ok_or(QueueError::NoSuchContext) - .map(|queue| queue.queue_tx((tx, hash))) + .map(|queue| queue.queue_tx((tx, hash)))? } + /// `drain_all` returns all transactions from the `TransactionQueue` associated with the + /// provided context ID, removing them from the queue. Transactions are returned in the + /// order they were added. pub fn drain_all(&self, context_id: u64) -> Vec { self.queues .read() @@ -90,6 +116,18 @@ impl TransactionQueueMap { .get(&context_id) .map_or(0, TransactionQueue::len) } + + /// `get_next_valid_nonce` returns the next valid nonce for the account with the provided address + /// in the `TransactionQueue` associated with the provided context ID. This method assumes that + /// only signed transactions (which include a nonce) are added to the queue using `queue_tx` + /// and that their nonces are in increasing order. + pub fn get_next_valid_nonce(&self, context_id: u64, address: H160) -> Option { + self.queues + .read() + .unwrap() + .get(&context_id) + .map_or(None, |queue| queue.get_next_valid_nonce(address)) + } } #[derive(Debug, Clone)] @@ -100,15 +138,20 @@ pub enum QueueTx { type QueueTxWithNativeHash = (QueueTx, NativeTxHash); +/// The `TransactionQueue` holds a queue of transactions and a map of account nonces. +/// It's used to manage and process transactions for different accounts. +/// #[derive(Debug, Default)] pub struct TransactionQueue { transactions: Mutex>, + account_nonces: Mutex>, } impl TransactionQueue { pub fn new() -> Self { Self { transactions: Mutex::new(Vec::new()), + account_nonces: Mutex::new(HashMap::new()), } } @@ -128,13 +171,32 @@ impl TransactionQueue { self.transactions.lock().unwrap().clone() } - pub fn queue_tx(&self, tx: QueueTxWithNativeHash) { + pub fn queue_tx(&self, tx: QueueTxWithNativeHash) -> Result<(), QueueError> { + if let QueueTx::SignedTx(signed_tx) = &tx.0 { + let mut account_nonces = self.account_nonces.lock().unwrap(); + if let Some(nonce) = account_nonces.get(&signed_tx.sender) { + if signed_tx.nonce() != nonce + 1 { + return Err(QueueError::InvalidNonce((signed_tx.clone(), *nonce))); + } + } + account_nonces.insert(signed_tx.sender, signed_tx.nonce()); + } self.transactions.lock().unwrap().push(tx); + Ok(()) } pub fn len(&self) -> usize { self.transactions.lock().unwrap().len() } + + pub fn get_next_valid_nonce(&self, address: H160) -> Option { + self.account_nonces + .lock() + .unwrap() + .get(&address) + .map(ToOwned::to_owned) + .map(|nonce| nonce + 1) + } } impl From for QueueTx { @@ -146,14 +208,99 @@ impl From for QueueTx { #[derive(Debug)] pub enum QueueError { NoSuchContext, + InvalidNonce((Box, U256)), } impl std::fmt::Display for QueueError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { QueueError::NoSuchContext => write!(f, "No transaction queue for this context"), + QueueError::InvalidNonce((tx, nonce)) => write!(f, "Invalid nonce {:x?} for tx {:x?}. Previous queued nonce is {}. TXs should be queued in increasing nonce order.", tx.nonce(), tx.transaction.hash(), nonce), } } } impl std::error::Error for QueueError {} + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use ethereum_types::{H256, U256}; + + use crate::transaction::bridge::BalanceUpdate; + + use super::*; + + #[test] + fn test_invalid_nonce_order() -> Result<(), QueueError> { + let queue = TransactionQueue::new(); + + // Nonce 2, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx1 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa0adb0386f95848d33b49ee6057c34e530f87f696a29b4e1b04ef90b2a58bbedbca02f500cf29c5c4245608545e7d9d35b36ef0365e5c52d96e69b8f07920d32552f").unwrap())); + + // Nonce 2, sender 0x6bc42fd533d6cb9d973604155e1f7197a3b0e703 + let tx2 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa09588b47d2cd3f474d6384309cca5cb8e360cb137679f0a1589a1c184a15cb27ca0453ddbf808b83b279cac3226b61a9d83855aba60ae0d3a8407cba0634da7459d").unwrap())); + + // Nonce 0, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx3 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869808502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa03d28d24808c3de08c606c5544772ded91913f648ad56556f181905208e206c85a00ecd0ba938fb89fc4a17ea333ea842c7305090dee9236e2b632578f9e5045cb3").unwrap())); + + queue.queue_tx((tx1, H256::from_low_u64_be(1).into()))?; + queue.queue_tx((tx2, H256::from_low_u64_be(2).into()))?; + // Should fail as nonce 2 is already queued for this sender + let queued = queue.queue_tx((tx3, H256::from_low_u64_be(3).into())); + assert!(matches!(queued, Err(QueueError::InvalidNonce { .. }))); + Ok(()) + } + + #[test] + fn test_invalid_nonce_order_with_transfer_domain() -> Result<(), QueueError> { + let queue = TransactionQueue::new(); + + // Nonce 2, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx1 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa0adb0386f95848d33b49ee6057c34e530f87f696a29b4e1b04ef90b2a58bbedbca02f500cf29c5c4245608545e7d9d35b36ef0365e5c52d96e69b8f07920d32552f").unwrap())); + + // Nonce 2, sender 0x6bc42fd533d6cb9d973604155e1f7197a3b0e703 + let tx2 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa09588b47d2cd3f474d6384309cca5cb8e360cb137679f0a1589a1c184a15cb27ca0453ddbf808b83b279cac3226b61a9d83855aba60ae0d3a8407cba0634da7459d").unwrap())); + + // sender 0x6bc42fd533d6cb9d973604155e1f7197a3b0e703 + let tx3 = QueueTx::BridgeTx(BridgeTx::EvmIn(BalanceUpdate { + address: H160::from_str("0x6bc42fd533d6cb9d973604155e1f7197a3b0e703").unwrap(), + amount: U256::one(), + })); + + // Nonce 0, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx4 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869808502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa03d28d24808c3de08c606c5544772ded91913f648ad56556f181905208e206c85a00ecd0ba938fb89fc4a17ea333ea842c7305090dee9236e2b632578f9e5045cb3").unwrap())); + + queue.queue_tx((tx1, H256::from_low_u64_be(1).into()))?; + queue.queue_tx((tx2, H256::from_low_u64_be(2).into()))?; + queue.queue_tx((tx3, H256::from_low_u64_be(3).into()))?; + // Should fail as nonce 2 is already queued for this sender + let queued = queue.queue_tx((tx4, H256::from_low_u64_be(4).into())); + assert!(matches!(queued, Err(QueueError::InvalidNonce { .. }))); + Ok(()) + } + + #[test] + fn test_valid_nonce_order() -> Result<(), QueueError> { + let queue = TransactionQueue::new(); + + // Nonce 0, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx1 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869808502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa03d28d24808c3de08c606c5544772ded91913f648ad56556f181905208e206c85a00ecd0ba938fb89fc4a17ea333ea842c7305090dee9236e2b632578f9e5045cb3").unwrap())); + + // Nonce 1, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx2 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869018502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa0dd1fad9a8465969354d567e8a74af3f6de3e56abbe1b71154d7929d0bd5cc170a0353190adb50e3cfae82a77c2ea56b49a86f72bd0071a70d1c25c49827654aa68").unwrap())); + + // Nonce 2, sender 0xe61a3a6eb316d773c773f4ce757a542f673023c6 + let tx3 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa0adb0386f95848d33b49ee6057c34e530f87f696a29b4e1b04ef90b2a58bbedbca02f500cf29c5c4245608545e7d9d35b36ef0365e5c52d96e69b8f07920d32552f").unwrap())); + + // Nonce 2, sender 0x6bc42fd533d6cb9d973604155e1f7197a3b0e703 + let tx4 = QueueTx::SignedTx(Box::new(SignedTx::try_from("f869028502540be400832dc6c0943e338e722607a8c1eab615579ace4f6dedfa19fa80840adb1a9a2aa09588b47d2cd3f474d6384309cca5cb8e360cb137679f0a1589a1c184a15cb27ca0453ddbf808b83b279cac3226b61a9d83855aba60ae0d3a8407cba0634da7459d").unwrap())); + + queue.queue_tx((tx1, H256::from_low_u64_be(1).into()))?; + queue.queue_tx((tx2, H256::from_low_u64_be(2).into()))?; + queue.queue_tx((tx3, H256::from_low_u64_be(3).into()))?; + queue.queue_tx((tx4, H256::from_low_u64_be(4).into()))?; + Ok(()) + } +} diff --git a/lib/ain-grpc/src/rpc/eth.rs b/lib/ain-grpc/src/rpc/eth.rs index a4979ae109b..47ff105ab12 100644 --- a/lib/ain-grpc/src/rpc/eth.rs +++ b/lib/ain-grpc/src/rpc/eth.rs @@ -576,7 +576,7 @@ impl MetachainRPCServer for MetachainRPCModule { Ok(format!("{:#x}", signed_tx.transaction.hash())) } else { - debug!(target:"rpc","[send_raw_transaction] Could not publish raw transaction: {tx}"); + debug!(target:"rpc","[send_raw_transaction] Could not publish raw transaction: {tx} reason: {res_string}"); Err(Error::Custom(format!( "Could not publish raw transaction: {tx} reason: {res_string}" ))) diff --git a/lib/ain-rs-exports/src/lib.rs b/lib/ain-rs-exports/src/lib.rs index 558cb699dd1..9a71afa7edf 100644 --- a/lib/ain-rs-exports/src/lib.rs +++ b/lib/ain-rs-exports/src/lib.rs @@ -12,6 +12,8 @@ use ethereum::{EnvelopedEncodable, TransactionAction, TransactionSignature}; use primitive_types::{H160, H256, U256}; use transaction::{LegacyUnsignedTransaction, TransactionError, LOWER_H256}; +use crate::ffi::RustRes; + pub const WEI_TO_GWEI: u64 = 1_000_000_000; pub const GWEI_TO_SATS: u64 = 10; @@ -34,8 +36,22 @@ pub mod ffi { miner_fee: u64, } + #[derive(Default)] + pub struct ValidateTxResult { + nonce: u64, + sender: [u8; 20], + } + + pub struct RustRes { + ok: bool, + reason: String, + } + extern "Rust" { - fn evm_get_balance(address: &str, block_number: [u8; 32]) -> Result; + fn evm_get_balance(address: [u8; 20]) -> Result; + fn evm_get_nonce(address: [u8; 20]) -> Result; + fn evm_get_next_valid_nonce_in_context(context: u64, address: [u8; 20]) -> u64; + fn evm_add_balance( context: u64, address: &str, @@ -48,11 +64,18 @@ pub mod ffi { amount: [u8; 32], native_tx_hash: [u8; 32], ) -> Result; - fn evm_prevalidate_raw_tx(tx: &str) -> Result; + + fn evm_try_prevalidate_raw_tx(result: &mut RustRes, tx: &str) -> Result; fn evm_get_context() -> u64; fn evm_discard_context(context: u64) -> Result<()>; - fn evm_queue_tx(context: u64, raw_tx: &str, native_tx_hash: [u8; 32]) -> Result; + fn evm_try_queue_tx( + result: &mut RustRes, + context: u64, + raw_tx: &str, + native_tx_hash: [u8; 32], + ) -> Result; + fn evm_finalize( context: u64, update_state: bool, @@ -116,12 +139,11 @@ pub fn create_and_sign_tx(ctx: ffi::CreateTransactionContext) -> Result, Ok(signed.encode().into()) } -/// Retrieves the balance of an EVM account at a specific block number. +/// Retrieves the balance of an EVM account at latest block height. /// /// # Arguments /// /// * `address` - The EVM address of the account. -/// * `block_number` - The block number as a byte array. /// /// # Errors /// @@ -130,18 +152,62 @@ pub fn create_and_sign_tx(ctx: ffi::CreateTransactionContext) -> Result, /// # Returns /// /// Returns the balance of the account as a `u64` on success. -pub fn evm_get_balance(address: &str, block_number: [u8; 32]) -> Result> { - let account = address.parse()?; +pub fn evm_get_balance(address: [u8; 20]) -> Result> { + let account = H160::from(address); + let (_, latest_block_number) = RUNTIME.handlers.block.get_latest_block_hash_and_number(); let mut balance = RUNTIME .handlers .evm - .get_balance(account, U256::from(block_number)) + .get_balance(account, latest_block_number) .unwrap_or_default(); // convert to try_evm_get_balance - Default to 0 for now balance /= WEI_TO_GWEI; balance /= GWEI_TO_SATS; Ok(balance.as_u64()) } +/// Retrieves the nonce of an EVM account at latest block height. +/// +/// # Arguments +/// +/// * `address` - The EVM address of the account. +/// +/// # Errors +/// +/// Throws an Error if the address is not a valid EVM address. +/// +/// # Returns +/// +/// Returns the nonce of the account as a `u64` on success. +pub fn evm_get_nonce(address: [u8; 20]) -> Result> { + let account = H160::from(address); + let (_, latest_block_number) = RUNTIME.handlers.block.get_latest_block_hash_and_number(); + let nonce = RUNTIME + .handlers + .evm + .get_nonce(account, latest_block_number) + .unwrap_or_default(); + Ok(nonce.as_u64()) +} + +/// Retrieves the next valid nonce of an EVM account in a specific context +/// +/// # Arguments +/// +/// * `context` - The context queue number. +/// * `address` - The EVM address of the account. +/// +/// # Returns +/// +/// Returns the next valid nonce of the account in a specific context as a `u64` +pub fn evm_get_next_valid_nonce_in_context(context: u64, address: [u8; 20]) -> u64 { + let address = H160::from(address); + let nonce = RUNTIME + .handlers + .evm + .get_next_valid_nonce_in_context(context, address); + nonce.as_u64() +} + /// EvmIn. Send DFI to an EVM account. /// /// # Arguments @@ -227,13 +293,25 @@ pub fn evm_sub_balance( /// /// # Returns /// -/// Returns `true` if the transaction is valid, logs the error and returns `false` otherwise. -pub fn evm_prevalidate_raw_tx(tx: &str) -> Result> { +/// Returns the transaction nonce and sender address if the transaction is valid, logs and throws the error otherwise. +pub fn evm_try_prevalidate_raw_tx( + result: &mut RustRes, + tx: &str, +) -> Result> { match RUNTIME.handlers.evm.validate_raw_tx(tx) { - Ok(_) => Ok(true), + Ok(signed_tx) => { + result.ok = true; + Ok(ffi::ValidateTxResult { + nonce: signed_tx.nonce().as_u64(), + sender: signed_tx.sender.to_fixed_bytes(), + }) + } Err(e) => { - debug!("evm_prevalidate_raw_tx fails with error: {:?}", e); - Ok(false) + debug!("evm_try_prevalidate_raw_tx fails with error: {e}"); + result.ok = false; + result.reason = e.to_string(); + + Ok(ffi::ValidateTxResult::default()) } } } @@ -283,15 +361,27 @@ fn evm_discard_context(context: u64) -> Result<(), Box> { /// # Returns /// /// Returns `true` if the transaction is successfully queued, `false` otherwise. -fn evm_queue_tx(context: u64, raw_tx: &str, hash: [u8; 32]) -> Result> { +fn evm_try_queue_tx( + result: &mut RustRes, + context: u64, + raw_tx: &str, + hash: [u8; 32], +) -> Result> { let signed_tx: SignedTx = raw_tx.try_into()?; match RUNTIME .handlers .evm .queue_tx(context, signed_tx.into(), hash) { - Ok(_) => Ok(true), - Err(_) => Ok(false), + Ok(_) => { + result.ok = true; + Ok(true) + } + Err(e) => { + result.ok = false; + result.reason = e.to_string(); + Ok(false) + } } } diff --git a/src/masternodes/mn_checks.cpp b/src/masternodes/mn_checks.cpp index d396a04cd2a..83959251f28 100644 --- a/src/masternodes/mn_checks.cpp +++ b/src/masternodes/mn_checks.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -3855,11 +3856,19 @@ class CCustomTxApplyVisitor : public CCustomTxVisitor { if (obj.evmTx.size() > static_cast(EVM_TX_SIZE)) return Res::Err("evm tx size too large"); - if (!evm_prevalidate_raw_tx(HexStr(obj.evmTx))) - return Res::Err("evm tx failed to validate"); + RustRes result; + evm_try_prevalidate_raw_tx(result, HexStr(obj.evmTx)); - if (!evm_queue_tx(evmContext, HexStr(obj.evmTx), tx.GetHash().ToArrayReversed())) - return Res::Err("evm tx failed to queue"); + if (!result.ok) { + LogPrintf("[evm_try_prevalidate_raw_tx] failed, reason : %s\n", result.reason); + return Res::Err("evm tx failed to validate %s", result.reason); + } + + evm_try_queue_tx(result, evmContext, HexStr(obj.evmTx), tx.GetHash().ToArrayReversed()); + if (!result.ok) { + LogPrintf("[evm_try_queue_tx] failed, reason : %s\n", result.reason); + return Res::Err("evm tx failed to queue %s\n", result.reason); + } return Res::Ok(); } diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 3a288a65f84..7ba51609908 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -602,8 +602,9 @@ UniValue gettokenbalances(const JSONRPCRequest& request) { if (eth_lookup) { for (const auto keyID : pwallet->GetEthKeys()) { - const arith_uint256 height = targetHeight; - const auto evmAmount = evm_get_balance(HexStr(keyID.begin(), keyID.end()), ArithToUint256(height).ToArrayReversed()); + std::array address{}; + std::copy(keyID.begin(), keyID.end(), address.begin()); + const auto evmAmount = evm_get_balance(address); totalBalances.Add({{}, static_cast(evmAmount)}); } } diff --git a/src/miner.cpp b/src/miner.cpp index fa68676a4ec..3abddcb06dc 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -636,6 +637,8 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda indexed_modified_transaction_set mapModifiedTx; // Keep track of entries that failed inclusion, to avoid duplicate work CTxMemPool::setEntries failedTx; + // Keep track of EVM entries that failed nonce check + std::multimap failedNonces; // Start by adding all descendants of previously added txs to mapModifiedTx // and modifying them for their already included ancestors @@ -656,7 +659,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda // Copy of the view CCoinsViewCache coinsView(&::ChainstateActive().CoinsTip()); - while (mi != mempool.mapTx.get().end() || !mapModifiedTx.empty()) + while (mi != mempool.mapTx.get().end() || !mapModifiedTx.empty() || !failedNonces.empty()) { // First try to find a new transaction in mapTx to evaluate. if (mi != mempool.mapTx.get().end() && @@ -670,7 +673,11 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda bool fUsingModified = false; modtxscoreiter modit = mapModifiedTx.get().begin(); - if (mi == mempool.mapTx.get().end()) { + if (mi == mempool.mapTx.get().end() && mapModifiedTx.empty()) { + const auto it = failedNonces.begin(); + iter = it->second; + failedNonces.erase(it); + } else if (mi == mempool.mapTx.get().end()) { // We're out of entries in mapTx; use the entry from mapModifiedTx iter = modit->iter; fUsingModified = true; @@ -773,10 +780,37 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda AddCoins(coins, tx, nHeight, false); // do not check std::vector metadata; - CustomTxType txType = GuessCustomTxType(tx, metadata); + CustomTxType txType = GuessCustomTxType(tx, metadata, true); // Only check custom TXs if (txType != CustomTxType::None) { + if (txType == CustomTxType::EvmTx) { + auto txMessage = customTypeToMessage(txType); + if (!CustomMetadataParse(nHeight, Params().GetConsensus(), metadata, txMessage)) { + customTxPassed = false; + break; + } + + const auto obj = std::get(txMessage); + + RustRes result; + const auto txResult = evm_try_prevalidate_raw_tx(result, HexStr(obj.evmTx)); + if (!result.ok) { + customTxPassed = false; + break; + } + + const auto nonce = evm_get_next_valid_nonce_in_context(evmContext, txResult.sender); + if (nonce != txResult.nonce) { + // Only add if not already in failed TXs to prevent adding on second attempt. + if (!failedTx.count(iter)) { + failedNonces.emplace(nonce, iter); + } + customTxPassed = false; + break; + } + } + const auto res = ApplyCustomTx(view, coins, tx, chainparams.GetConsensus(), nHeight, pblock->nTime, nullptr, 0, evmContext); // Not okay invalidate, undo and skip diff --git a/src/validation.cpp b/src/validation.cpp index cc7289d581e..094295d7223 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -540,38 +540,40 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Check for conflicts with in-memory transactions std::set setConflicts; - for (const CTxIn &txin : tx.vin) - { - const CTransaction* ptxConflicting = pool.GetConflictTx(txin.prevout); - if (ptxConflicting) { - if (!setConflicts.count(ptxConflicting->GetHash())) - { - // Allow opt-out of transaction replacement by setting - // nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. - // - // SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by - // non-replaceable transactions. All inputs rather than just one - // is for the sake of multi-party protocols, where we don't - // want a single party to be able to disable replacement. - // - // The opt-out ignores descendants as anyone relying on - // first-seen mempool behavior should be checking all - // unconfirmed ancestors anyway; doing otherwise is hopelessly - // insecure. - bool fReplacementOptOut = true; - for (const CTxIn &_txin : ptxConflicting->vin) + if (!IsEVMTx(tx)) { + for (const CTxIn &txin : tx.vin) + { + const CTransaction* ptxConflicting = pool.GetConflictTx(txin.prevout); + if (ptxConflicting) { + if (!setConflicts.count(ptxConflicting->GetHash())) { - if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE) + // Allow opt-out of transaction replacement by setting + // nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. + // + // SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by + // non-replaceable transactions. All inputs rather than just one + // is for the sake of multi-party protocols, where we don't + // want a single party to be able to disable replacement. + // + // The opt-out ignores descendants as anyone relying on + // first-seen mempool behavior should be checking all + // unconfirmed ancestors anyway; doing otherwise is hopelessly + // insecure. + bool fReplacementOptOut = true; + for (const CTxIn &_txin : ptxConflicting->vin) { - fReplacementOptOut = false; - break; + if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE) + { + fReplacementOptOut = false; + break; + } + } + if (fReplacementOptOut) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_DUPLICATE, "txn-mempool-conflict"); } - } - if (fReplacementOptOut) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_DUPLICATE, "txn-mempool-conflict"); - } - setConflicts.insert(ptxConflicting->GetHash()); + setConflicts.insert(ptxConflicting->GetHash()); + } } } } diff --git a/test/functional/feature_evm.py b/test/functional/feature_evm.py index 9e56acf09ff..11228f5452a 100755 --- a/test/functional/feature_evm.py +++ b/test/functional/feature_evm.py @@ -259,9 +259,6 @@ def run_test(self): self.nodes[0].generate(1) self.sync_blocks() - # Try and send a TX with a high nonce - assert_raises_rpc_error(-32600, "evm tx failed to validate", self.nodes[0].evmtx, ethAddress, 1, 21, 21000, to_address, 1) - # Check Eth balances before transfer assert_equal(int(self.nodes[0].eth_getBalance(ethAddress)[2:], 16), 10000000000000000000) assert_equal(int(self.nodes[0].eth_getBalance(to_address)[2:], 16), 0) @@ -270,35 +267,45 @@ def run_test(self): miner_before = Decimal(self.nodes[0].getaccount(self.nodes[0].get_genesis_keys().ownerAuthAddress)[0].split('@')[0]) # Test EVM Tx - tx = self.nodes[0].evmtx(ethAddress, 0, 21, 21000, to_address, 1) + tx3 = self.nodes[0].evmtx(ethAddress, 2, 21, 21001, to_address, 1) + tx2 = self.nodes[0].evmtx(ethAddress, 1, 21, 21001, to_address, 1) + tx = self.nodes[0].evmtx(ethAddress, 0, 21, 21001, to_address, 1) + tx4 = self.nodes[0].evmtx(ethAddress, 3, 21, 21001, to_address, 1) raw_tx = self.nodes[0].getrawtransaction(tx) self.sync_mempools() # Check the pending TXs result = self.nodes[0].eth_pendingTransactions() - assert_equal(result[0]['blockHash'], '0x0000000000000000000000000000000000000000000000000000000000000000') - assert_equal(result[0]['blockNumber'], 'null') - assert_equal(result[0]['from'], ethAddress) - assert_equal(result[0]['gas'], '0x5208') - assert_equal(result[0]['gasPrice'], '0x4e3b29200') - assert_equal(result[0]['hash'], '0x8c99e9f053e033078e33c2756221f38fd529b914165090a615f27961de687497') - assert_equal(result[0]['input'], '0x') - assert_equal(result[0]['nonce'], '0x0') - assert_equal(result[0]['to'], to_address.lower()) - assert_equal(result[0]['transactionIndex'], '0x0') - assert_equal(result[0]['value'], '0xde0b6b3a7640000') - assert_equal(result[0]['v'], '0x25') - assert_equal(result[0]['r'], '0x37f41c543402c9b02b35b45ef43ac31a63dcbeba0c622249810ecdec00aee376') - assert_equal(result[0]['s'], '0x5eb2be77eb0c7a1875a53ba15fc6afe246fbffe869157edbde64270e41ba045e') - - # Check mempools for TX - assert_equal(self.nodes[0].getrawmempool(), [tx]) - assert_equal(self.nodes[1].getrawmempool(), [tx]) + assert_equal(result[2]['blockHash'], '0x0000000000000000000000000000000000000000000000000000000000000000') + assert_equal(result[2]['blockNumber'], 'null') + assert_equal(result[2]['from'], ethAddress) + assert_equal(result[2]['gas'], '0x5209') + assert_equal(result[2]['gasPrice'], '0x4e3b29200') + assert_equal(result[2]['hash'], '0xadf0fbeb972cdc4a82916d12ffc6019f60005de6dde1bbc7cb4417fe5a7b1bcb') + assert_equal(result[2]['input'], '0x') + assert_equal(result[2]['nonce'], '0x0') + assert_equal(result[2]['to'], to_address.lower()) + assert_equal(result[2]['transactionIndex'], '0x0') + assert_equal(result[2]['value'], '0xde0b6b3a7640000') + assert_equal(result[2]['v'], '0x26') + assert_equal(result[2]['r'], '0x3a0587be1a14bd5e68bc883e627f3c0999cff9458e30ea8049f17bd7369d7d9c') + assert_equal(result[2]['s'], '0x1876f296657bc56499cc6398617f97b2327fa87189c0a49fb671b4361876142a') + + # Check mempools for TXs + assert_equal(self.nodes[0].getrawmempool(), [tx3, tx2, tx4, tx]) + assert_equal(self.nodes[1].getrawmempool(), [tx3, tx2, tx4, tx]) self.nodes[0].generate(1) + # Check TXs in block in correct order + block_txs = self.nodes[0].getblock(self.nodes[0].getblockhash(self.nodes[0].getblockcount()))['tx'] + assert_equal(block_txs[1], tx) + assert_equal(block_txs[2], tx2) + assert_equal(block_txs[3], tx3) + assert_equal(block_txs[4], tx4) + # Check Eth balances before transfer - assert_equal(int(self.nodes[0].eth_getBalance(ethAddress)[2:], 16), 9000000000000000000) - assert_equal(int(self.nodes[0].eth_getBalance(to_address)[2:], 16), 1000000000000000000) + assert_equal(int(self.nodes[0].eth_getBalance(ethAddress)[2:], 16), 6000000000000000000) + assert_equal(int(self.nodes[0].eth_getBalance(to_address)[2:], 16), 4000000000000000000) # Check miner account balance after transfer miner_after = Decimal(self.nodes[0].getaccount(self.nodes[0].get_genesis_keys().ownerAuthAddress)[0].split('@')[0]) @@ -310,7 +317,12 @@ def run_test(self): # Check EVM Tx shows in block on EVM side block = self.nodes[0].eth_getBlockByNumber("latest", False) - assert_equal(block['transactions'], ['0x8c99e9f053e033078e33c2756221f38fd529b914165090a615f27961de687497']) + assert_equal(block['transactions'], [ + '0xadf0fbeb972cdc4a82916d12ffc6019f60005de6dde1bbc7cb4417fe5a7b1bcb', + '0x66c380af8f76295bab799d1228af75bd3c436b7bbeb9d93acd8baac9377a851a', + '0x02b05a6646feb65bf9491f9551e02678263239dc2512d73c9ad6bc80dc1c13ff', + '0x1d4c8a49ad46d9362c805d6cdf9a8937ba115eec9def17b3efe23a09ee694e5c' + ]) # Check pending TXs now empty assert_equal(self.nodes[0].eth_pendingTransactions(), [])