From f030bd5a2b366d37ec4e5c53255c0c1a251e0770 Mon Sep 17 00:00:00 2001 From: Niven Date: Fri, 15 Sep 2023 12:35:39 +0800 Subject: [PATCH] EVM: Add optional nonce parameter for transfer domain RPC (#2451) * Move RBF from mempool to miner * Reformat Python * lint: remove unused variable * Fix feature_evm_miner test * Add nonce option for transferdomain rpc * Fix feature_dst20 test --------- Co-authored-by: Bushstar --- lib/ain-evm/src/core.rs | 9 +- lib/ain-rs-exports/src/evm.rs | 18 +- lib/ain-rs-exports/src/lib.rs | 7 +- src/consensus/tx_verify.cpp | 2 +- src/masternodes/consensus/txvisitor.cpp | 6 +- src/masternodes/consensus/txvisitor.h | 4 +- src/masternodes/consensus/xvm.cpp | 2 +- src/masternodes/mn_checks.cpp | 19 +- src/masternodes/mn_checks.h | 6 +- src/masternodes/mn_rpc.cpp | 3 +- src/masternodes/rpc_accounts.cpp | 25 +- src/masternodes/rpc_tokens.cpp | 2 +- src/miner.cpp | 174 +++++++++-- src/test/applytx_tests.cpp | 8 +- src/txmempool.cpp | 288 ++---------------- src/txmempool.h | 6 +- src/validation.cpp | 33 +- test/functional/feature_dst20.py | 8 +- test/functional/feature_evm.py | 63 +--- test/functional/feature_evm_miner.py | 16 +- test/functional/feature_evm_rollback.py | 8 +- .../feature_evm_transaction_replacement.py | 21 +- test/functional/rpc_mn_basic.py | 11 +- 23 files changed, 271 insertions(+), 468 deletions(-) mode change 100644 => 100755 test/functional/feature_evm_miner.py diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index 92858ef8e00..4b3fc86fcec 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -216,7 +216,6 @@ impl EVMCoreService { tx: &str, queue_id: u64, pre_validate: bool, - test_tx: bool, ) -> Result { debug!("[validate_raw_tx] queue_id {}", queue_id); debug!("[validate_raw_tx] raw transaction : {:#?}", tx); @@ -333,11 +332,9 @@ impl EVMCoreService { .get_total_gas_used_in(queue_id) .unwrap_or_default(); - if !test_tx { - let block_gas_limit = self.storage.get_attributes_or_default()?.block_gas_limit; - if total_current_gas_used + U256::from(used_gas) > U256::from(block_gas_limit) { - return Err(format_err!("Tx can't make it in block. Block size limit {}, pending block gas used : {:x?}, tx used gas : {:x?}, total : {:x?}", block_gas_limit, total_current_gas_used, U256::from(used_gas), total_current_gas_used + U256::from(used_gas)).into()); - } + let block_gas_limit = self.storage.get_attributes_or_default()?.block_gas_limit; + if total_current_gas_used + U256::from(used_gas) > U256::from(block_gas_limit) { + return Err(format_err!("Tx can't make it in block. Block size limit {}, pending block gas used : {:x?}, tx used gas : {:x?}, total : {:x?}", block_gas_limit, total_current_gas_used, U256::from(used_gas), total_current_gas_used + U256::from(used_gas)).into()); } Ok(ValidateTxInfo { diff --git a/lib/ain-rs-exports/src/evm.rs b/lib/ain-rs-exports/src/evm.rs index 00f69f73108..3190398f7a6 100644 --- a/lib/ain-rs-exports/src/evm.rs +++ b/lib/ain-rs-exports/src/evm.rs @@ -235,11 +235,16 @@ pub fn evm_try_create_and_sign_transfer_domain_tx( } } }; - let Ok(nonce) = SERVICES.evm.get_nonce(from_address, state_root) else { - return cross_boundary_error_return( - result, - format!("Could not get nonce for {from_address:x?}"), - ); + let nonce = if ctx.use_nonce { + U256::from(ctx.nonce) + } else { + let Ok(nonce) = SERVICES.evm.get_nonce(from_address, state_root) else { + return cross_boundary_error_return( + result, + format!("Could not get nonce for {from_address:x?}"), + ); + }; + nonce }; let t = LegacyUnsignedTransaction { @@ -498,7 +503,6 @@ pub fn evm_unsafe_try_validate_raw_tx_in_q( queue_id: u64, raw_tx: &str, pre_validate: bool, - test_tx: bool, ) -> ffi::ValidateTxMiner { debug!("[evm_unsafe_try_validate_raw_tx_in_q]"); match SERVICES.evm.verify_tx_fees(raw_tx) { @@ -512,7 +516,7 @@ pub fn evm_unsafe_try_validate_raw_tx_in_q( match SERVICES .evm .core - .validate_raw_tx(raw_tx, queue_id, pre_validate, test_tx) + .validate_raw_tx(raw_tx, queue_id, pre_validate) { Ok(ValidateTxInfo { signed_tx, diff --git a/lib/ain-rs-exports/src/lib.rs b/lib/ain-rs-exports/src/lib.rs index b3da24cdd25..f9c0dec361f 100644 --- a/lib/ain-rs-exports/src/lib.rs +++ b/lib/ain-rs-exports/src/lib.rs @@ -43,8 +43,8 @@ pub mod ffi { #[derive(Default)] pub struct TxSenderInfo { - address: String, - nonce: u64, + pub address: String, + pub nonce: u64, } // ========== Governance Variable ========== @@ -99,6 +99,8 @@ pub mod ffi { pub chain_id: u64, pub priv_key: [u8; 32], pub queue_id: u64, + pub use_nonce: bool, + pub nonce: u64, } #[derive(Default)] @@ -169,7 +171,6 @@ pub mod ffi { queue_id: u64, raw_tx: &str, pre_validate: bool, - test_tx: bool, ) -> ValidateTxMiner; fn evm_unsafe_try_push_tx_in_q( result: &mut CrossBoundaryResult, diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index bd08718f911..8e506016097 100644 --- a/src/consensus/tx_verify.cpp +++ b/src/consensus/tx_verify.cpp @@ -183,7 +183,7 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c // Note: TXs are already filtered. So we pass isEVMEnabled to false, but for future proof, refactor this enough, // that it's propagated. uint64_t gasUsed{}; - auto res = ApplyCustomTx(discardCache, inputs, tx, chainparams.GetConsensus(), nSpendHeight, gasUsed, 0, &canSpend, 0, 0, false); + auto res = ApplyCustomTx(discardCache, inputs, tx, chainparams.GetConsensus(), nSpendHeight, gasUsed, 0, &canSpend, 0, 0, false, false); if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) { return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-customtx", res.msg); } diff --git a/src/masternodes/consensus/txvisitor.cpp b/src/masternodes/consensus/txvisitor.cpp index da75f4dd535..e440eaabc3c 100644 --- a/src/masternodes/consensus/txvisitor.cpp +++ b/src/masternodes/consensus/txvisitor.cpp @@ -63,8 +63,7 @@ CCustomTxVisitor::CCustomTxVisitor(const CTransaction &tx, const uint64_t evmQueueId, const bool isEvmEnabledForBlock, uint64_t &gasUsed, - const bool evmPreValidate, - const bool testTx) + const bool evmPreValidate) : height(height), mnview(mnview), tx(tx), @@ -75,8 +74,7 @@ CCustomTxVisitor::CCustomTxVisitor(const CTransaction &tx, evmQueueId(evmQueueId), isEvmEnabledForBlock(isEvmEnabledForBlock), gasUsed(gasUsed), - evmPreValidate(evmPreValidate), - testTx(testTx) {} + evmPreValidate(evmPreValidate) {} Res CCustomTxVisitor::HasAuth(const CScript &auth) const { return ::HasAuth(tx, coins, auth); diff --git a/src/masternodes/consensus/txvisitor.h b/src/masternodes/consensus/txvisitor.h index 2b334a9cc6a..5d964d810e8 100644 --- a/src/masternodes/consensus/txvisitor.h +++ b/src/masternodes/consensus/txvisitor.h @@ -54,7 +54,6 @@ class CCustomTxVisitor { bool isEvmEnabledForBlock; uint64_t &gasUsed; bool evmPreValidate; - bool testTx; public: CCustomTxVisitor(const CTransaction &tx, @@ -67,8 +66,7 @@ class CCustomTxVisitor { const uint64_t evmQueueId, const bool isEvmEnabledForBlock, uint64_t &gasUsed, - const bool evmPreValidate, - const bool testTx); + const bool evmPreValidate); protected: Res HasAuth(const CScript &auth) const; diff --git a/src/masternodes/consensus/xvm.cpp b/src/masternodes/consensus/xvm.cpp index d9925dacc79..caca897c83a 100644 --- a/src/masternodes/consensus/xvm.cpp +++ b/src/masternodes/consensus/xvm.cpp @@ -327,7 +327,7 @@ Res CXVMConsensus::operator()(const CEvmTxMessage &obj) const { return Res::Err("evm tx size too large"); CrossBoundaryResult result; - auto validateResults = evm_unsafe_try_validate_raw_tx_in_q(result, evmQueueId, HexStr(obj.evmTx), evmPreValidate, testTx); + auto validateResults = evm_unsafe_try_validate_raw_tx_in_q(result, evmQueueId, HexStr(obj.evmTx), evmPreValidate); if (!result.ok) { LogPrintf("[evm_try_validate_raw_tx] failed, reason : %s\n", result.reason); return Res::Err("evm tx failed to validate %s", result.reason); diff --git a/src/masternodes/mn_checks.cpp b/src/masternodes/mn_checks.cpp index 6a29d1e59e8..8ff632a08b8 100644 --- a/src/masternodes/mn_checks.cpp +++ b/src/masternodes/mn_checks.cpp @@ -333,7 +333,6 @@ class CCustomTxApplyVisitor { bool isEvmEnabledForBlock; uint64_t &gasUsed; bool evmPreValidate; - bool testTx; template Res ConsensusHandler(const T& obj) const { @@ -341,7 +340,7 @@ class CCustomTxApplyVisitor { static_assert(std::is_base_of_v, "CCustomTxVisitor base required"); if constexpr (std::is_invocable_v) - return T1{tx, height, coins, mnview, consensus, time, txn, evmQueueId, isEvmEnabledForBlock, gasUsed, evmPreValidate, testTx}(obj); + return T1{tx, height, coins, mnview, consensus, time, txn, evmQueueId, isEvmEnabledForBlock, gasUsed, evmPreValidate}(obj); else if constexpr (sizeof...(Args) != 0) return ConsensusHandler(obj); else @@ -361,8 +360,7 @@ class CCustomTxApplyVisitor { const uint64_t evmQueueId, const bool isEvmEnabledForBlock, uint64_t &gasUsed, - const bool evmPreValidate, - const bool testTx) + const bool evmPreValidate) : tx(tx), height(height), @@ -374,8 +372,7 @@ class CCustomTxApplyVisitor { evmQueueId(evmQueueId), isEvmEnabledForBlock(isEvmEnabledForBlock), gasUsed(gasUsed), - evmPreValidate(evmPreValidate), - testTx(testTx) {} + evmPreValidate(evmPreValidate) {} template Res operator()(const T& obj) const { @@ -454,8 +451,7 @@ Res CustomTxVisit(CCustomCSView &mnview, const uint32_t txn, const uint64_t evmQueueId, const bool isEvmEnabledForBlock, - const bool evmPreValidate, - const bool testTx) { + const bool evmPreValidate) { if (IsDisabledTx(height, tx, consensus)) { return Res::ErrCode(CustomTxErrCodes::Fatal, "Disabled custom transaction"); } @@ -470,7 +466,7 @@ Res CustomTxVisit(CCustomCSView &mnview, try { auto res = std::visit( - CCustomTxApplyVisitor(tx, height, coins, mnview, consensus, time, txn, q, isEvmEnabledForBlock, gasUsed, evmPreValidate, testTx), + CCustomTxApplyVisitor(tx, height, coins, mnview, consensus, time, txn, q, isEvmEnabledForBlock, gasUsed, evmPreValidate), txMessage); if (wipeQueue) { XResultStatusLogged(evm_unsafe_try_remove_queue(result, q)); @@ -569,7 +565,8 @@ Res ApplyCustomTx(CCustomCSView &mnview, uint256 *canSpend, uint32_t txn, const uint64_t evmQueueId, - const bool isEvmEnabledForBlock) { + const bool isEvmEnabledForBlock, + const bool evmPreValidate) { auto res = Res::Ok(); if (tx.IsCoinBase() && height > 0) { // genesis contains custom coinbase txs return res; @@ -608,7 +605,7 @@ Res ApplyCustomTx(CCustomCSView &mnview, PopulateVaultHistoryData(mnview.GetHistoryWriters(), view, txMessage, txType, height, txn, tx.GetHash()); } - res = CustomTxVisit(view, coins, tx, height, consensus, txMessage, time, gasUsed, txn, evmQueueId, isEvmEnabledForBlock, false, false); + res = CustomTxVisit(view, coins, tx, height, consensus, txMessage, time, gasUsed, txn, evmQueueId, isEvmEnabledForBlock, evmPreValidate); if (res) { if (canSpend && txType == CustomTxType::UpdateMasternode) { diff --git a/src/masternodes/mn_checks.h b/src/masternodes/mn_checks.h index 98c42efac9c..2f053f9b4b5 100644 --- a/src/masternodes/mn_checks.h +++ b/src/masternodes/mn_checks.h @@ -175,7 +175,8 @@ Res ApplyCustomTx(CCustomCSView &mnview, uint256 *canSpend, uint32_t txn, const uint64_t evmQueueId, - const bool isEvmEnabledForBlock); + const bool isEvmEnabledForBlock, + const bool evmPreValidate); Res CustomTxVisit(CCustomCSView &mnview, const CCoinsViewCache &coins, @@ -188,8 +189,7 @@ Res CustomTxVisit(CCustomCSView &mnview, const uint32_t txn, const uint64_t evmQueueId, const bool isEvmEnabledForBlock, - const bool evmPreValidate, - const bool testTx); + const bool evmPreValidate); ResVal ApplyAnchorRewardTx(CCustomCSView &mnview, diff --git a/src/masternodes/mn_rpc.cpp b/src/masternodes/mn_rpc.cpp index ed3e230e19a..08ff4980b58 100644 --- a/src/masternodes/mn_rpc.cpp +++ b/src/masternodes/mn_rpc.cpp @@ -459,7 +459,7 @@ void execTestTx(const CTransaction& tx, uint32_t height, CTransactionRef optAuth auto consensus = Params().GetConsensus(); auto isEvmEnabledForBlock = IsEVMEnabled(height, view, consensus); uint64_t gasUsed{}; - res = CustomTxVisit(view, coins, tx, height, consensus, txMessage, ::ChainActive().Tip()->nTime, gasUsed, 0, 0, isEvmEnabledForBlock, true, true); + res = CustomTxVisit(view, coins, tx, height, consensus, txMessage, ::ChainActive().Tip()->nTime, gasUsed, 0, 0, isEvmEnabledForBlock, true); } if (!res) { if (res.code == CustomTxErrCodes::NotEnoughBalance) { @@ -1132,7 +1132,6 @@ static UniValue clearmempool(const JSONRPCRequest& request) LOCK(mempool.cs); mempool.queryHashes(vtxid); mempool.clear(); - mempool.wipeEvmQueueId(); } { diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 13bddda6391..94907bbb426 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -2018,7 +2018,8 @@ UniValue transferdomain(const JSONRPCRequest& request) { {"domain", RPCArg::Type::NUM, RPCArg::Optional::NO, "Domain of source: 2 - DVM, 3 - EVM"}, // {"data", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Optional data"}, }, - } + }, + {"nonce", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Optional parameter to specify the transaction nonce"}, }, }, }, @@ -2050,10 +2051,11 @@ UniValue transferdomain(const JSONRPCRequest& request) { try { for (unsigned int i=0; i < srcDstArray.size(); i++) { const UniValue& elem = srcDstArray[i]; - RPCTypeCheck(elem, {UniValue::VOBJ, UniValue::VOBJ}, false); + RPCTypeCheck(elem, {UniValue::VOBJ, UniValue::VOBJ, UniValue::VNUM}, false); const UniValue& srcObj = elem["src"].get_obj(); const UniValue& dstObj = elem["dst"].get_obj(); + const UniValue& nonceObj = elem["nonce"]; CTransferDomainItem src, dst; @@ -2136,7 +2138,15 @@ UniValue transferdomain(const JSONRPCRequest& request) { std::string from = ScriptToString(script); CrossBoundaryResult result; - auto evmQueueId = mempool.getEvmQueueId(); + auto evmQueueId = evm_unsafe_try_create_queue(result); + if (!result.ok) { + throw JSONRPCError(RPC_MISC_ERROR, "Unable to create EVM queue"); + } + uint64_t nonce = 0; + bool useNonce = !nonceObj.isNull(); + if (useNonce) { + nonce = nonceObj.get_int64(); + } const auto signedTx = evm_try_create_and_sign_transfer_domain_tx(result, CreateTransferDomainContext{std::move(from), std::move(to), nativeAddress, @@ -2145,12 +2155,19 @@ UniValue transferdomain(const JSONRPCRequest& request) { dst.amount.nTokenId.v, Params().GetConsensus().evmChainId, privKey, - evmQueueId + evmQueueId, + useNonce, + nonce }); if (!result.ok) { throw JSONRPCError(RPC_MISC_ERROR, strprintf("Failed to create and sign TX: %s", result.reason.c_str())); } + evm_unsafe_try_remove_queue(result, evmQueueId); + if (!result.ok) { + throw JSONRPCError(RPC_MISC_ERROR, "Unable to destroy EVM queue"); + } + std::vector evmTx(signedTx.size()); std::copy(signedTx.begin(), signedTx.end(), evmTx.begin()); if (isEVMIn) { diff --git a/src/masternodes/rpc_tokens.cpp b/src/masternodes/rpc_tokens.cpp index e833403ec85..6c6a189e9f5 100644 --- a/src/masternodes/rpc_tokens.cpp +++ b/src/masternodes/rpc_tokens.cpp @@ -608,7 +608,7 @@ UniValue getcustomtx(const JSONRPCRequest& request) auto isEvmEnabledForBlock = IsEVMEnabled(nHeight, mnview, consensus); uint64_t gasUsed{}; - auto res = ApplyCustomTx(mnview, view, *tx, consensus, nHeight, gasUsed, 0, nullptr, 0, 0, isEvmEnabledForBlock); + auto res = ApplyCustomTx(mnview, view, *tx, consensus, nHeight, gasUsed, 0, nullptr, 0, 0, isEvmEnabledForBlock, false); result.pushKV("valid", res.ok); } else { diff --git a/src/miner.cpp b/src/miner.cpp index d1c18c73f47..7493c1a0384 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -40,14 +40,34 @@ #include #include +struct EvmPackageContext { + // Used to track EVM TX fee and TX hash by sender and nonce. + std::map> evmFeeMap; + // Quick lookup for EVM fee map + using EVMFeeMapIterator = std::map>::iterator; + std::map feeMapLookup{}; + // Quick lookup for mempool time order map + using TimeMapIterator = std::multimap::iterator; + std::map timeOrderLookup{}; + // Keep track of EVM entries that failed nonce check + std::multimap failedNonces; + // Used for replacement Eth TXs ordered by time and nonce + std::map, CTxMemPool::txiter> replaceByFee; + // Variable to tally total gas used in the block + uint64_t totalGas{}; + // Used to track gas fees of EVM TXs + std::map evmGasFees; +}; + struct EvmTxPreApplyContext { CTxMemPool::txiter& txIter; std::vector& txMetadata; CustomTxType txType; int height; uint64_t evmQueueId; - std::multimap &failedNonces; - CTxMemPool::setEntries& failedTxSet; + EvmPackageContext& pkgCtx; + std::set& checkedTXEntries; + CTxMemPool::setEntries& failedTxEntries; }; int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) @@ -598,13 +618,19 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve bool BlockAssembler::EvmTxPreapply(const EvmTxPreApplyContext& ctx) { - auto txMessage = customTypeToMessage(ctx.txType); + auto& txType = ctx.txType; + auto txMessage = customTypeToMessage(txType); auto& txIter = ctx.txIter; auto& evmQueueId = ctx.evmQueueId; auto& metadata = ctx.txMetadata; auto& height = ctx.height; - auto& failedNonces = ctx.failedNonces; - auto& failedTxSet = ctx.failedTxSet; + auto& checkedDfTxHashSet = ctx.checkedTXEntries; + auto& failedTxSet = ctx.failedTxEntries; + auto& pkgCtx = ctx.pkgCtx; + auto& failedNonces = pkgCtx.failedNonces; + auto& replaceByFee = pkgCtx.replaceByFee; + auto& totalGas = pkgCtx.totalGas; + auto& timeOrderLookup = pkgCtx.timeOrderLookup; if (!CustomMetadataParse(height, Params().GetConsensus(), metadata, txMessage)) { return false; @@ -614,15 +640,14 @@ bool BlockAssembler::EvmTxPreapply(const EvmTxPreApplyContext& ctx) ValidateTxMiner txResult; if (ctx.txType == CustomTxType::EvmTx) { const auto obj = std::get(txMessage); - txResult = evm_unsafe_try_validate_raw_tx_in_q(result, evmQueueId, HexStr(obj.evmTx), true, false); + txResult = evm_unsafe_try_validate_raw_tx_in_q(result, evmQueueId, HexStr(obj.evmTx), true); if (!result.ok) { return false; } - if (txResult.higher_nonce && !failedTxSet.count(txIter)) { - failedNonces.emplace(txResult.nonce, txIter); - } - if (txResult.higher_nonce || txResult.lower_nonce) { - failedTxSet.insert(txIter); + if (txResult.higher_nonce) { + if (!failedTxSet.count(txIter)) { + failedNonces.emplace(txResult.nonce, txIter); + } return false; } } else if (ctx.txType == CustomTxType::TransferDomain) { @@ -631,7 +656,7 @@ bool BlockAssembler::EvmTxPreapply(const EvmTxPreApplyContext& ctx) return false; } - std::string evmTx = ""; + std::string evmTx; if (obj.transfers[0].first.domain == static_cast(VMDomain::DVM) && obj.transfers[0].second.domain == static_cast(VMDomain::EVM)) { evmTx = HexStr(obj.transfers[0].second.data); } @@ -642,21 +667,95 @@ bool BlockAssembler::EvmTxPreapply(const EvmTxPreApplyContext& ctx) if (!result.ok) { return false; } - const auto nonce = evm_unsafe_try_get_next_valid_nonce_in_q(result, evmQueueId, senderInfo.address); - if (!result.ok || nonce > senderInfo.nonce) { + const auto expectedNonce = evm_unsafe_try_get_next_valid_nonce_in_q(result, evmQueueId, senderInfo.address); + if (!result.ok) { return false; } - if (nonce < senderInfo.nonce) { + if (senderInfo.nonce < expectedNonce) { // Pays 0 so cannot be RBF + return false; + } else if (senderInfo.nonce > expectedNonce) { if (!failedTxSet.count(txIter)) { failedNonces.emplace(senderInfo.nonce, txIter); } - failedTxSet.insert(txIter); return false; } + txResult.nonce = senderInfo.nonce; + txResult.sender = senderInfo.address; + txResult.prepay_fee = 0; + txResult.higher_nonce = false; + txResult.lower_nonce = false; } else { return false; } + auto& evmFeeMap = pkgCtx.evmFeeMap; + auto& feeMapLookup = pkgCtx.feeMapLookup; + + const std::string txResultSender{txResult.sender.data(), txResult.sender.length()}; + const EvmAddressWithNonce addrKey{txResult.nonce, txResultSender}; + + if (const auto feeEntry = evmFeeMap.find(addrKey); feeEntry != evmFeeMap.end()) { + // Key already exists. We check to see if we need to prioritize higher fee tx + const auto& [lastFee, prevHash] = feeEntry->second; + if (txResult.prepay_fee > lastFee) { + // Higher paying fee. Remove all TXs above the TX to be replaced. + auto hashes = evm_unsafe_try_remove_txs_above_hash_in_q(result, evmQueueId, prevHash.ToString()); + if (!result.ok) { + return Res::Err("%s: Unable to remove TXs from queue\n", __func__); + } + + // Set of TXs to remove from the block + CTxMemPool::setEntries txsForRemoval; + + // Loop through hashes of removed TXs and remove from block. + for (const auto &rustStr : hashes) { + const auto txHash = uint256S(std::string{rustStr.data(), rustStr.length()}); + + // Check fee map entry and get nonce + assert(feeMapLookup.count(txHash)); + auto it = feeMapLookup.at(txHash); + auto &[nonce, hash] = it->first; + + // Check TX time order entry and get time and TX + assert(timeOrderLookup.count(txHash)); + const auto &timeIt = timeOrderLookup.at(txHash); + const auto& [txTime, transaction] = *timeIt; + + // Add entry to removal set + txsForRemoval.insert(transaction); + + // Reduce fee + totalGas -= transaction->GetFee(); + + // This is the entry to be replaced + if (prevHash == txHash) { + replaceByFee.emplace(std::make_pair(txTime, nonce), txIter); + } else { // Add in previous mempool entry + replaceByFee.emplace(std::make_pair(txTime, nonce), transaction); + } + + // Remove from checked entries + checkedDfTxHashSet.erase(txHash); + + // Remove replace fee entries + evmFeeMap.erase(feeMapLookup.at(txHash)); + feeMapLookup.erase(txHash); + } + + RemoveFromBlock(txsForRemoval, true); + } + + return false; + } + + // If not RBF for a mempool TX then return false + if (txResult.lower_nonce) { + return false; + } + + auto feeMapRes = evmFeeMap.emplace(addrKey, std::make_pair(txResult.prepay_fee, txIter->GetTx().GetHash())); + feeMapLookup.emplace(txIter->GetTx().GetHash(), feeMapRes.first); + return true; } @@ -698,13 +797,13 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda // Copy of the view CCoinsViewCache coinsView(&::ChainstateActive().CoinsTip()); - // Keep track of EVM entries that failed nonce check - std::multimap failedNonces; + // Track mempool time order + std::multimap mempoolTimeOrder{}; - // Track total gas used in the block - uint64_t totalGas{}; + // EVM related context for this package + EvmPackageContext evmPackageContext; - while (mi != mempool.mapTx.get().end() || !mapModifiedTxSet.empty() || !failedNonces.empty()) { + while (mi != mempool.mapTx.get().end() || !mapModifiedTxSet.empty() || !evmPackageContext.failedNonces.empty() || !evmPackageContext.replaceByFee.empty()) { // First try to find a new transaction in mapTx to evaluate. if (mi != mempool.mapTx.get().end() && SkipMapTxEntry(mempool.mapTx.project<0>(mi), mapModifiedTxSet, failedTxSet)) { @@ -716,8 +815,19 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda // the next entry from mapTx, or the best from mapModifiedTxSet? bool fUsingModified = false; + auto& replaceByFee = evmPackageContext.replaceByFee; + + // Check whether we are using Eth replaceByFee + bool usingReplaceByFee = !replaceByFee.empty(); + modtxscoreiter modit = mapModifiedTxSet.get().begin(); - if (mi == mempool.mapTx.get().end() && mapModifiedTxSet.empty()) { + if (!replaceByFee.empty()) { + const auto it = replaceByFee.begin(); + iter = it->second; + replaceByFee.erase(it); + usingReplaceByFee = true; + } else if (mi == mempool.mapTx.get().end() && mapModifiedTxSet.empty()) { + auto& failedNonces = evmPackageContext.failedNonces; const auto it = failedNonces.begin(); iter = it->second; failedNonces.erase(it); @@ -740,6 +850,8 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda // Increment mi for the next loop iteration. ++mi; } + auto timeIt = mempoolTimeOrder.emplace(iter->GetTime(), iter); + evmPackageContext.timeOrderLookup.emplace(iter->GetTx().GetHash(), timeIt); } // We skip mapTx entries that are inBlock, and mapModifiedTxSet shouldn't @@ -839,20 +951,22 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda txType, nHeight, evmQueueId, - failedNonces, - failedTxSet + evmPackageContext, + checkedDfTxHashSet, + failedTxSet, }; auto res = EvmTxPreapply(evmTxCtx); if (res) { customTxPassed = true; } else { + failedTxSet.insert(sortedEntries[i]); customTxPassed = false; break; } } uint64_t gasUsed{}; - const auto res = ApplyCustomTx(view, coins, tx, chainparams.GetConsensus(), nHeight, gasUsed, pblock->nTime, nullptr, 0, evmQueueId, isEvmEnabledForBlock); + const auto res = ApplyCustomTx(view, coins, tx, chainparams.GetConsensus(), nHeight, gasUsed, pblock->nTime, nullptr, 0, evmQueueId, isEvmEnabledForBlock, false); // Not okay invalidate, undo and skip if (!res.ok) { customTxPassed = false; @@ -860,11 +974,13 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda break; } - if (totalGas + gasUsed > MAX_BLOCK_GAS_LIMIT) { + evmPackageContext.evmGasFees.emplace(tx.GetHash(), gasUsed); + + if (evmPackageContext.totalGas + gasUsed > MAX_BLOCK_GAS_LIMIT) { customTxPassed = false; break; } - totalGas += gasUsed; + evmPackageContext.totalGas += gasUsed; // Track checked TXs to avoid double applying checkedDfTxHashSet.insert(tx.GetHash()); @@ -877,6 +993,10 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda if (fUsingModified) { mapModifiedTxSet.get().erase(modit); } + if (usingReplaceByFee) { + // TX failed in chain so remove the rest of the items + replaceByFee.clear(); + } failedTxSet.insert(iter); continue; } diff --git a/src/test/applytx_tests.cpp b/src/test/applytx_tests.cpp index 1f1cadeaa27..748f9ed30d5 100644 --- a/src/test/applytx_tests.cpp +++ b/src/test/applytx_tests.cpp @@ -118,7 +118,7 @@ BOOST_AUTO_TEST_CASE(apply_a2a_neg) rawTx.vout[0].scriptPubKey = CreateMetaA2A(msg); - res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false); + res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false, false); BOOST_CHECK(!res.ok); BOOST_CHECK_NE(res.msg.find("negative amount"), std::string::npos); // check that nothing changes: @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(apply_a2a_neg) rawTx.vout[0].scriptPubKey = CreateMetaA2A(msg); uint64_t gasUsed{}; - res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false); + res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false, false); BOOST_CHECK(!res.ok); BOOST_CHECK_EQUAL(res.code, (uint32_t) CustomTxErrCodes::NotEnoughBalance); // check that nothing changes: @@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE(apply_a2a_neg) rawTx.vout[0].scriptPubKey = CreateMetaA2A(msg); - res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false); + res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false, false); BOOST_CHECK(!res.ok); BOOST_CHECK_NE(res.msg.find("negative amount"), std::string::npos); // check that nothing changes: @@ -169,7 +169,7 @@ BOOST_AUTO_TEST_CASE(apply_a2a_neg) rawTx.vout[0].scriptPubKey = CreateMetaA2A(msg); - res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false); + res = ApplyCustomTx(mnview, coinview, CTransaction(rawTx), amkCheated, 1, gasUsed, 0, nullptr, 0, 0, false, false); BOOST_CHECK(res.ok); // check result balances: auto const dfi90 = CTokenAmount{DFI, 90}; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 3697ce3a898..7920c53ce33 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -559,7 +559,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem CCoinsViewMemPool viewMemPool(&coins_cache, *this); view.SetBackend(viewMemPool); - rebuildAccountsView(nMemPoolHeight, view, ptx); + rebuildAccountsView(nMemPoolHeight, view); } } @@ -633,7 +633,7 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne CCoinsViewMemPool viewMemPool(&coins_cache, *this); view.SetBackend(viewMemPool); - rebuildAccountsView(nBlockHeight, view, ptx); + rebuildAccountsView(nBlockHeight, view); } lastRollingFeeUpdate = GetTime(); @@ -1160,39 +1160,12 @@ bool CTxMemPool::getAccountViewDirty() const { return accountsViewDirty; } -uint64_t CTxMemPool::getEvmQueueId() { - if (!evmQueueId) { - CrossBoundaryResult result; - evmQueueId = evm_unsafe_try_create_queue(result); - } - return evmQueueId; -} - -void CTxMemPool::wipeEvmQueueId() { - evmQueueId = 0; -} - -Res CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCache, const CTransactionRef& ptx, const int64_t time) +void CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCache) { - if (!pcustomcsview || (!accountsViewDirty && evmQueueId)) { - return Res::Ok(); - } - - CrossBoundaryResult result; - if (evmQueueId) { - evm_unsafe_try_remove_queue(result, evmQueueId); - if (!result.ok) { - return Res::Err("%s: Unable to remove previous EVM queue\n", __func__); - } - } - - evmQueueId = evm_unsafe_try_create_queue(result); - if (!result.ok) { - return Res::Err("%s: Unable to create EVM queue\n", __func__); + if (!pcustomcsview || !accountsViewDirty) { + return; } - auto newEntryRes = Res::Ok(); - CAmount txfee{}; accountsView().Discard(); CCustomCSView viewDuplicate(accountsView()); @@ -1204,247 +1177,22 @@ Res CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCach auto isEvmEnabledForBlock = IsEVMEnabled(height, viewDuplicate, consensus); // Check custom TX consensus types are now not in conflict with account layer - auto mi = mapTx.get().begin(); - - // Track TX hashes to mempool iterators - std::map mempoolIterMap{}; - - // Track mempool time order - std::multimap mempoolTimeOrder{}; - - // Quick lookup for mempool time order map - using TimeMapIterator = std::multimap::iterator; - std::map timeOrderLookup{}; - - // Used to track EVM TX fee and TX hash by sender and nonce. - std::map> evmFeeMap; - - // Quick lookup for EVM fee map - using EVMFeeMapIterator = std::map>::iterator; - std::map feeMapLookup{}; - - // Used for replacement Eth TXs ordered by time and nonce - std::map, CTransactionRef> replaceByFee; - - // Keep track of EVM entries that failed nonce check - std::multimap failedNonces; - - // Keep track of entries that failed inclusion, to avoid duplicate work - std::set failedTxSet; - - // Temp copy to loop over new TX - auto newTx = ptx; - - while (newTx || mi != mempool.mapTx.get().end() || !replaceByFee.empty() || !failedNonces.empty()) { - - // Whether we are looping on the transaction being added to the mempool - bool newTxLoop{}; - - CTransactionRef tx; - if (!replaceByFee.empty()) { - const auto it = replaceByFee.begin(); - tx = it->second; - replaceByFee.erase(it); - } else if (mi == mempool.mapTx.get().end() && newTx) { - tx = newTx; - auto timeIt = mempoolTimeOrder.emplace(time, tx); - timeOrderLookup.emplace(tx->GetHash(), timeIt); - newTx = nullptr; - newTxLoop = true; - } else if (mi == mempool.mapTx.get().end()) { - const auto it = failedNonces.begin(); - tx = it->second; - failedNonces.erase(it); - } else { - auto iter = mempool.mapTx.project<0>(mi); - mempoolIterMap.emplace(iter->GetTx().GetHash(), iter); - tx = iter->GetSharedTx(); - auto timeIt = mempoolTimeOrder.emplace(iter->GetTime(), tx); - timeOrderLookup.emplace(iter->GetTx().GetHash(), timeIt); - ++mi; - } - + auto& txsByEntryTime = mapTx.get(); + for (auto it = txsByEntryTime.begin(); it != txsByEntryTime.end(); ++it) { CValidationState state; - if (!Consensus::CheckTxInputs(*tx, state, coinsCache, viewDuplicate, height, txfee, Params())) { - if (newTxLoop) { - newEntryRes = Res::Err(state.GetDebugMessage()); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } + const auto& tx = it->GetTx(); + if (!Consensus::CheckTxInputs(tx, state, coinsCache, viewDuplicate, height, txfee, Params())) { + LogPrintf("%s: Remove conflicting TX: %s\n", __func__, tx.GetHash().GetHex()); + staged.insert(mapTx.project<0>(it)); + vtx.push_back(it->GetSharedTx()); continue; } - uint64_t gasUsed{}; - std::vector metadata; - CustomTxType txType = GuessCustomTxType(*tx, metadata, true); - - if (txType == CustomTxType::EvmTx || txType == CustomTxType::TransferDomain) { - auto txMessage = customTypeToMessage(txType); - auto res = CustomMetadataParse(height, consensus, metadata, txMessage); - if (!res) { - if (newTxLoop) { - newEntryRes = Res::Err(res.msg); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } - ValidateTxMiner txResult; - if (txType == CustomTxType::EvmTx) { - const auto obj = std::get(txMessage); - txResult = evm_unsafe_try_validate_raw_tx_in_q(result, evmQueueId, HexStr(obj.evmTx), true, true); - if (!result.ok) { - if (newTxLoop) { - newEntryRes = Res::Err(result.reason.c_str()); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } - if (txResult.higher_nonce) { - if (!failedTxSet.count(tx->GetHash())) { - failedNonces.emplace(txResult.nonce, tx); - } - failedTxSet.insert(tx->GetHash()); - continue; - } - } else { - const auto obj = std::get(txMessage); - if (obj.transfers.size() != 1) { - if (newTxLoop) { - newEntryRes = DeFiErrors::TransferDomainMultipleTransfers(); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } - - std::string evmTx = ""; - if (obj.transfers[0].first.domain == static_cast(VMDomain::DVM) && obj.transfers[0].second.domain == static_cast(VMDomain::EVM)) { - evmTx = HexStr(obj.transfers[0].second.data); - } - else if (obj.transfers[0].first.domain == static_cast(VMDomain::EVM) && obj.transfers[0].second.domain == static_cast(VMDomain::DVM)) { - evmTx = HexStr(obj.transfers[0].first.data); - } - auto senderInfo = evm_try_get_tx_sender_info_from_raw_tx(result, evmTx); - if (!result.ok) { - if (newTxLoop) { - newEntryRes = Res::Err(result.reason.c_str()); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } - const auto nonce = evm_unsafe_try_get_next_valid_nonce_in_q(result, evmQueueId, senderInfo.address); - if (!result.ok) { - if (newTxLoop) { - newEntryRes = Res::Err(result.reason.c_str()); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } - if (nonce > senderInfo.nonce) { - if (newTxLoop) { - newEntryRes = Res::Err(result.reason.c_str()); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - continue; - } else if (nonce < senderInfo.nonce) { - if (!failedTxSet.count(tx->GetHash())) { - failedNonces.emplace(senderInfo.nonce, tx); - } - failedTxSet.insert(tx->GetHash()); - continue; - } else if (nonce > senderInfo.nonce) { - failedTxSet.insert(tx->GetHash()); - continue; - } - txResult.nonce = senderInfo.nonce; - txResult.sender = senderInfo.address; - txResult.prepay_fee = 0; - txResult.higher_nonce = false; - txResult.lower_nonce = false; - } - - const std::string txResultSender{txResult.sender.data(), txResult.sender.length()}; - const EvmAddressWithNonce addrKey{txResult.nonce, txResultSender}; - - if (const auto feeEntry = evmFeeMap.find(addrKey); feeEntry != evmFeeMap.end()) { - // Key already exists. We check to see if we need to prioritize higher fee tx - const auto& [lastFee, prevHash] = feeEntry->second; - if (txResult.prepay_fee > lastFee) { - // Higher paying fee. Remove all TXs above the TX to be replaced. - auto hashes = evm_unsafe_try_remove_txs_above_hash_in_q(result, evmQueueId, prevHash.ToString()); - if (!result.ok) { - return Res::Err("%s: Unable to remove TXs from queue\n", __func__); - } - - // Loop through hashes of removed TXs and remove from block. - for (const auto &rustStr : hashes) { - const auto txHash = uint256S(std::string{rustStr.data(), rustStr.length()}); - - // Check fee map entry and get nonce - assert(feeMapLookup.count(txHash)); - auto it = feeMapLookup.at(txHash); - auto &[nonce, hash] = it->first; - - // Check TX time order entry and get time and TX - assert(timeOrderLookup.count(txHash)); - const auto &timeIt = timeOrderLookup.at(txHash); - const auto& [txTime, transaction] = *timeIt; - - // This is the entry to be replaced - if (prevHash == txHash) { - AddToStaged(staged, vtx, transaction, mempoolIterMap); - replaceByFee.emplace(std::make_pair(txTime, nonce), tx); - } - else // Add in previous mempool entry - { - replaceByFee.emplace(std::make_pair(txTime, nonce), transaction); - } - - // Remove replace fee entries - evmFeeMap.erase(feeMapLookup.at(txHash)); - feeMapLookup.erase(txHash); - } - } else { - if (newTxLoop) { - newEntryRes = Res::Err("Rejected due to same or lower fee as existing mempool entry"); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - } - - continue; - } - - // If not RBF at this point then remove - if (txResult.lower_nonce) { - if (newTxLoop) { - newEntryRes = Res::Err("Nonce lower than expected"); - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - - continue; - } - - auto feeMapRes = evmFeeMap.emplace(addrKey, std::make_pair(txResult.prepay_fee, tx->GetHash())); - feeMapLookup.emplace(tx->GetHash(), feeMapRes.first); - } - - auto res = ApplyCustomTx(viewDuplicate, coinsCache, *tx, consensus, height, gasUsed, 0, nullptr, 0, evmQueueId, isEvmEnabledForBlock); - if (!res) { - failedTxSet.insert(tx->GetHash()); - if ((res.code & CustomTxErrCodes::Fatal)) { - if (newTxLoop) { - newEntryRes = res; - } else { - AddToStaged(staged, vtx, tx, mempoolIterMap); - } - } + auto res = ApplyCustomTx(viewDuplicate, coinsCache, tx, consensus, height, gasUsed, 0, nullptr, 0, 0, isEvmEnabledForBlock, true); + if (!res && (res.code & CustomTxErrCodes::Fatal)) { + LogPrintf("%s: Remove conflicting custom TX: %s\n", __func__, tx.GetHash().GetHex()); + staged.insert(mapTx.project<0>(it)); + vtx.push_back(it->GetSharedTx()); } } @@ -1458,8 +1206,6 @@ Res CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCach viewDuplicate.Flush(); accountsViewDirty = false; forceRebuildForReorg = false; - - return newEntryRes; } void CTxMemPool::AddToStaged(setEntries &staged, std::vector &vtx, const CTransactionRef tx, std::map &mempoolIterMap) { diff --git a/src/txmempool.h b/src/txmempool.h index b838539de60..dbdf99f71d0 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -564,7 +564,6 @@ class CTxMemPool bool accountsViewDirty; bool forceRebuildForReorg; std::unique_ptr acview; - uint64_t evmQueueId{}; static void AddToStaged(setEntries &staged, std::vector &vtx, const CTransactionRef tx, std::map &mempoolIterMap); public: @@ -723,11 +722,10 @@ class CTxMemPool boost::signals2::signal NotifyEntryRemoved; CCustomCSView& accountsView(); - Res rebuildAccountsView(int height, const CCoinsViewCache& coinsCache, const CTransactionRef& ptx, const int64_t time = 0); + void rebuildAccountsView(int height, const CCoinsViewCache& coinsCache); void setAccountViewDirty(); bool getAccountViewDirty() const; - uint64_t getEvmQueueId(); - void wipeEvmQueueId(); + private: /** UpdateForDescendants is used by UpdateTransactionsFromBlock to update * the descendants for a single transaction that has been added to the diff --git a/src/validation.cpp b/src/validation.cpp index b30376a54d6..10bb29654d9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -633,29 +633,14 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool const auto& consensus = chainparams.GetConsensus(); auto isEvmEnabledForBlock = IsEVMEnabled(height, mnview, consensus); - std::vector metadata; - CustomTxType txType = GuessCustomTxType(tx, metadata, true); - if (txType == CustomTxType::EvmTx || txType == CustomTxType::TransferDomain) { - pool.setAccountViewDirty(); - } + // rebuild accounts view if dirty + pool.rebuildAccountsView(height, view); CAmount nFees = 0; if (!Consensus::CheckTxInputs(tx, state, view, mnview, height, nFees, chainparams)) { return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); } - // If account view is dirty or there is no EVM queue ID Then ApplyCustomTx will - // be tested against the current TX in rebuildAccountsView. - if (!pool.getAccountViewDirty() && pool.getEvmQueueId()) { - uint64_t gasUsed{}; - auto res = ApplyCustomTx(mnview, view, tx, consensus, height, gasUsed, nAcceptTime, nullptr, 0, 0, isEvmEnabledForBlock); - if (!res.ok || (res.code & CustomTxErrCodes::Fatal)) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, res.msg); - } - } else if (auto res = pool.rebuildAccountsView(height, view, ptx, nAcceptTime); !res) { - return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, res.msg); - } - if (nAbsurdFee && nFees > nAbsurdFee) { return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_HIGHFEE, "absurdly-high-fee", strprintf("%d > %d", nFees, nAbsurdFee)); } @@ -665,6 +650,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, "bad-txns-inputs-below-tx-fee"); } + uint64_t gasUsed{}; + auto res = ApplyCustomTx(mnview, view, tx, consensus, height, gasUsed, nAcceptTime, nullptr, 0, 0, isEvmEnabledForBlock, true); + if (!res.ok || (res.code & CustomTxErrCodes::Fatal)) { + return state.Invalid(ValidationInvalidReason::TX_MEMPOOL_POLICY, false, REJECT_INVALID, res.msg); + } + // we have all inputs cached now, so switch back to dummy, so we don't need to keep lock on mempool view.SetBackend(dummy); @@ -913,6 +904,8 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // TODO: We do multiple guess and parses. Streamline this later. // We also have IsEvmTx, but we need the metadata here. + std::vector metadata; + CustomTxType txType = GuessCustomTxType(tx, metadata, true); auto isEvmTx = txType == CustomTxType::EvmTx; if (!isEvmTx && !CheckInputsFromMempoolAndCache(tx, state, view, pool, currentBlockScriptVerifyFlags, true, txdata)) { @@ -924,7 +917,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool if (isEvmTx) { auto txMessage = customTypeToMessage(txType); - auto res = CustomMetadataParse(height, consensus, metadata, txMessage); + res = CustomMetadataParse(height, consensus, metadata, txMessage); if (!res) { return state.Invalid(ValidationInvalidReason::TX_NOT_STANDARD, error("Failed to parse EVM tx metadata"), REJECT_INVALID, "failed-to-parse-evm-tx-metadata"); } @@ -2414,7 +2407,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl mnview.GetHistoryWriters().GetBurnView() = nullptr; for (size_t i = 0; i < block.vtx.size(); ++i) { uint64_t gasUsed{}; - const auto res = ApplyCustomTx(mnview, view, *block.vtx[i], chainparams.GetConsensus(), pindex->nHeight, gasUsed, pindex->GetBlockTime(), nullptr, i, 0, false); + const auto res = ApplyCustomTx(mnview, view, *block.vtx[i], chainparams.GetConsensus(), pindex->nHeight, gasUsed, pindex->GetBlockTime(), nullptr, i, 0, false, false); if (!res.ok) { return error("%s: Genesis block ApplyCustomTx failed. TX: %s Error: %s", __func__, block.vtx[i]->GetHash().ToString(), res.msg); @@ -2698,7 +2691,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl const auto applyCustomTxTime = GetTimeMicros(); uint64_t gasUsed{}; - const auto res = ApplyCustomTx(accountsView, view, tx, consensus, pindex->nHeight, gasUsed, pindex->GetBlockTime(), nullptr, i, evmQueueId, isEvmEnabledForBlock); + const auto res = ApplyCustomTx(accountsView, view, tx, consensus, pindex->nHeight, gasUsed, pindex->GetBlockTime(), nullptr, i, evmQueueId, isEvmEnabledForBlock, false); LogApplyCustomTx(tx, applyCustomTxTime); if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) { diff --git a/test/functional/feature_dst20.py b/test/functional/feature_dst20.py index 06ad2bfbafa..c1fdbdcc8e4 100755 --- a/test/functional/feature_dst20.py +++ b/test/functional/feature_dst20.py @@ -446,6 +446,7 @@ def test_dst20_evm_to_dvm_bridge(self): assert_equal(amountBTC, "9.50000000@BTC") def test_multiple_dvm_evm_bridge(self): + nonce = self.nodes[0].w3.eth.get_transaction_count(self.erc55_address) self.nodes[0].transferdomain( [ { @@ -455,6 +456,7 @@ def test_multiple_dvm_evm_bridge(self): "amount": "1@BTC", "domain": 3, }, + "nonce": nonce, } ] ) @@ -467,6 +469,7 @@ def test_multiple_dvm_evm_bridge(self): "amount": "2@BTC", "domain": 3, }, + "nonce": nonce + 1, } ] ) @@ -517,7 +520,9 @@ def test_conflicting_bridge(self): [afterAmount] = [x for x in self.node.getaccount(self.address) if "BTC" in x] assert_equal( - self.btc.functions.balanceOf(self.key_pair2.address).call(), Decimal(0) + self.btc.functions.balanceOf(self.key_pair2.address).call() + / math.pow(10, self.btc.functions.decimals().call()), + Decimal(0), ) assert_equal( self.btc.functions.totalSupply().call() @@ -749,6 +754,7 @@ def run_test(self): self.node = self.nodes[0] self.w0 = self.node.w3 self.address = self.node.get_genesis_keys().ownerAuthAddress + self.erc55_address = self.node.addressmap(self.address, 1)["format"]["erc55"] # Contract addresses self.contract_address_usdt = self.w0.to_checksum_address( diff --git a/test/functional/feature_evm.py b/test/functional/feature_evm.py index 47ed6fe2b2c..ea56fa3b887 100755 --- a/test/functional/feature_evm.py +++ b/test/functional/feature_evm.py @@ -809,7 +809,6 @@ def nonce_order_and_rbf(self): tx2 = self.nodes[0].evmtx(self.eth_address, 2, 21, 21001, self.to_address, 1) tx1 = self.nodes[0].evmtx(self.eth_address, 1, 21, 21001, self.to_address, 1) tx3 = self.nodes[0].evmtx(self.eth_address, 3, 21, 21001, self.to_address, 1) - raw_tx = self.nodes[0].getrawtransaction(tx5) self.sync_mempools() # Check the pending TXs @@ -954,11 +953,6 @@ def nonce_order_and_rbf(self): ], ) - # Try and send EVM TX a second time - assert_raises_rpc_error( - -26, "Nonce lower than expected", self.nodes[0].sendrawtransaction, raw_tx - ) - def validate_xvm_coinbase(self): # Check EVM blockhash eth_block = self.nodes[0].eth_getBlockByNumber("latest") @@ -1130,62 +1124,13 @@ def multiple_eth_rbf(self): self.nodes[0].evmtx(self.eth_address, 65, 22, 21001, self.to_address, 1) self.nodes[0].evmtx(self.eth_address, 65, 23, 21001, self.to_address, 1) tx0 = self.nodes[0].evmtx(self.eth_address, 65, 25, 21001, self.to_address, 1) - assert_raises_rpc_error( - -26, - "Rejected due to same or lower fee as existing mempool entry", - self.nodes[0].evmtx, - self.eth_address, - 65, - 21, - 21001, - self.to_address, - 1, - ) - assert_raises_rpc_error( - -26, - "Rejected due to same or lower fee as existing mempool entry", - self.nodes[0].evmtx, - self.eth_address, - 65, - 24, - 21001, - self.to_address, - 1, - ) + self.nodes[0].evmtx(self.eth_address, 65, 21, 21001, self.to_address, 1) + self.nodes[0].evmtx(self.eth_address, 65, 24, 21001, self.to_address, 1) self.nodes[0].evmtx(self.to_address, 0, 22, 21001, self.eth_address, 1) self.nodes[0].evmtx(self.to_address, 0, 23, 21001, self.eth_address, 1) tx1 = self.nodes[0].evmtx(self.to_address, 0, 25, 21001, self.eth_address, 1) - assert_raises_rpc_error( - -26, - "Rejected due to same or lower fee as existing mempool entry", - self.nodes[0].evmtx, - self.to_address, - 0, - 21, - 21001, - self.eth_address, - 1, - ) - assert_raises_rpc_error( - -26, - "Rejected due to same or lower fee as existing mempool entry", - self.nodes[0].evmtx, - self.to_address, - 0, - 24, - 21001, - self.eth_address, - 1, - ) - - # Check mempool only contains two entries - assert_equal( - sorted(self.nodes[0].getrawmempool()), - [ - "2b13a48b2af32206a2d60d535ad46d4958c25b4ddd4c30f3a2da32f092c23916", - "6a6b53538b66e0eb477ce923901e6fa1714c4f52a83f8f1793c92c14ebc0f910", - ], - ) + self.nodes[0].evmtx(self.to_address, 0, 21, 21001, self.eth_address, 1) + self.nodes[0].evmtx(self.to_address, 0, 24, 21001, self.eth_address, 1) self.nodes[0].generate(1) # Check accounting of EVM fees diff --git a/test/functional/feature_evm_miner.py b/test/functional/feature_evm_miner.py old mode 100644 new mode 100755 index f0c75567748..3aed59b64d8 --- a/test/functional/feature_evm_miner.py +++ b/test/functional/feature_evm_miner.py @@ -274,7 +274,7 @@ def invalid_evm_tx_in_block_creation(self): block_info = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 4) assert_equal(len(block_info["tx"]) - 1, 20) - def test_for_fee_mismatch_between_block_and_queue(self): + def state_dependent_txs_in_block_and_queue(self): self.rollback_and_clear_mempool() before_balance = Decimal( self.nodes[0].getaccount(self.ethAddress)[0].split("@")[0] @@ -382,7 +382,6 @@ def test_for_fee_mismatch_between_block_and_queue(self): hash = self.nodes[0].w3.eth.send_raw_transaction(signed.rawTransaction) self.nodes[0].generate(1) - # Only the first 10 txs should have gas used = gas_used_when_true hashes = [] count = self.nodes[0].w3.eth.get_transaction_count(self.ethAddress) for idx in range(10): @@ -411,9 +410,6 @@ def test_for_fee_mismatch_between_block_and_queue(self): hash = self.nodes[0].w3.eth.send_raw_transaction(signed.rawTransaction) hashes.append(signed.hash.hex()) - # Only the 8 of the 15 txs should be able to be minted before reaching block limit - # calculated in txqueue. - # All of these txs should have gas used = gas_used_when_false for idx in range(15): tx = contract.functions.loop(9_000).build_transaction( { @@ -429,9 +425,8 @@ def test_for_fee_mismatch_between_block_and_queue(self): self.nodes[0].generate(1) - # Check that only 19 txs are minted block = self.nodes[0].eth_getBlockByNumber("latest") - assert_equal(len(block["transactions"]), 19) + assert_equal(len(block["transactions"]), 26) # Check first 10 txs should have gas used when true for idx in range(10): @@ -463,7 +458,7 @@ def test_for_fee_mismatch_between_block_and_queue(self): # But the minted block is only of size 16111252. correct_gas_used = ( gas_used_when_true * 10 - + gas_used_when_false * 8 + + gas_used_when_false * 15 + gas_used_when_change_state ) block_info = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 4) @@ -471,9 +466,6 @@ def test_for_fee_mismatch_between_block_and_queue(self): block_info["tx"][0]["vm"]["xvmHeader"]["gasUsed"], correct_gas_used ) - # Check that the remaining 7 evm txs are still in mempool - assert_equal(Decimal(self.nodes[0].getmempoolinfo()["size"]), Decimal("7")) - def run_test(self): self.setup() @@ -484,7 +476,7 @@ def run_test(self): self.invalid_evm_tx_in_block_creation() # Test for block size overflow from fee mismatch between tx queue and block - self.test_for_fee_mismatch_between_block_and_queue() + self.state_dependent_txs_in_block_and_queue() if __name__ == "__main__": diff --git a/test/functional/feature_evm_rollback.py b/test/functional/feature_evm_rollback.py index f6c5b5eb9b9..2723da08c0e 100755 --- a/test/functional/feature_evm_rollback.py +++ b/test/functional/feature_evm_rollback.py @@ -87,7 +87,7 @@ def test_rollback_block(self): blockPreInvalidation = self.nodes[0].eth_getBlockByNumber( blockNumberPreInvalidation ) - assert_equal(blockNumberPreInvalidation, "0x2") + assert_equal(blockNumberPreInvalidation, "0x3") assert_equal(blockPreInvalidation["number"], blockNumberPreInvalidation) self.nodes[0].invalidateblock(initialBlockHash) @@ -101,7 +101,7 @@ def test_rollback_block(self): blockByHash = self.nodes[0].eth_getBlockByHash(blockPreInvalidation["hash"]) assert_equal(blockByHash, None) block = self.nodes[0].eth_getBlockByNumber("latest") - assert_equal(block["number"], "0x1") + assert_equal(block["number"], "0x2") self.nodes[0].reconsiderblock(initialBlockHash) blockNumber = self.nodes[0].eth_blockNumber() @@ -160,7 +160,7 @@ def test_rollback_transactions(self): blockPreInvalidation = self.nodes[0].eth_getBlockByNumber( blockNumberPreInvalidation ) - assert_equal(blockNumberPreInvalidation, "0x3") + assert_equal(blockNumberPreInvalidation, "0x4") assert_equal(blockPreInvalidation["number"], blockNumberPreInvalidation) txPreInvalidation = self.nodes[0].eth_getTransactionByHash(hash) @@ -201,6 +201,8 @@ def run_test(self): } ] ) + self.nodes[0].generate(1) + self.nodes[0].transferdomain( [ { diff --git a/test/functional/feature_evm_transaction_replacement.py b/test/functional/feature_evm_transaction_replacement.py index 4e69b5ca6e7..193676a55c5 100755 --- a/test/functional/feature_evm_transaction_replacement.py +++ b/test/functional/feature_evm_transaction_replacement.py @@ -120,28 +120,21 @@ def send_transaction(self, gasPrice, count): def should_prioritize_transaction_with_the_higher_gas_price(self): gasPrices = [ - "0x2540be400", "0x2540be401", - "0x2540be402", - "0x2540be403", + "0x2540be400", "0x2540be404", - "0x2540be405", "0x2540be406", + "0x2540be401", "0x2540be407", + "0x2540be402", + "0x2540be405", + "0x2540be403", ] count = self.nodes[0].eth_getTransactionCount(self.ethAddress) for gasPrice in gasPrices: self.send_transaction(gasPrice, count) - assert_raises_rpc_error( - -32001, - "lower fee as existing mempool entry", - self.send_transaction, - gasPrices[0], - count, - ) - self.nodes[0].generate(1) block = self.nodes[0].eth_getBlockByNumber("latest", True) @@ -150,8 +143,8 @@ def should_prioritize_transaction_with_the_higher_gas_price(self): def should_replace_pending_transaction_0(self): gasPrices = [ - "0x2540be400", "0x2540be401", + "0x2540be400", # '0x2540be404', # '0x2540be406', # '0x2540be401', @@ -221,9 +214,9 @@ def should_replace_pending_transaction_3(self): gasPrices = [ # '0x2540be401', "0x2540be400", - "0x2540be401", "0x2540be404", "0x2540be406", + "0x2540be401", # '0x2540be407', # '0x2540be402', # '0x2540be405', diff --git a/test/functional/rpc_mn_basic.py b/test/functional/rpc_mn_basic.py index 725d7f8cd96..a78dbe43608 100755 --- a/test/functional/rpc_mn_basic.py +++ b/test/functional/rpc_mn_basic.py @@ -140,7 +140,7 @@ def run_test(self): fundingTx = self.nodes[0].sendtoaddress(collateral0, 1) self.nodes[0].generate(1) # resignTx - self.nodes[0].resignmasternode(idnode0) + resignTx = self.nodes[0].resignmasternode(idnode0) self.nodes[0].generate(1) assert_equal(self.nodes[0].listmasternodes()[idnode0]["state"], "PRE_RESIGNED") self.nodes[0].generate(10) @@ -167,12 +167,9 @@ def run_test(self): self.sync_blocks(self.nodes[0:2]) # Check that collateral spending tx was deleted - # print ("CreateTx", idnode0) - # print ("ResignTx", resignTx) - # print ("FundingTx", fundingTx) - # print ("SpendTx", sendedTxHash) - # resignTx is removed for a block - assert_equal(self.nodes[0].getrawmempool(), [fundingTx]) + assert_equal( + sorted(self.nodes[0].getrawmempool()), sorted([resignTx, fundingTx]) + ) assert_equal(self.nodes[0].listmasternodes()[idnode0]["state"], "ENABLED") # Revert creation!