From 317a0360a340b6998af73f63902a8d0774049d7e Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 13 Sep 2023 21:53:25 +0800 Subject: [PATCH 1/3] Fix transfer domain raw evmtx data --- src/masternodes/consensus/xvm.cpp | 73 ++++++++----------- src/masternodes/rpc_accounts.cpp | 6 +- src/miner.cpp | 10 ++- src/txmempool.cpp | 10 ++- test/functional/feature_evm_transferdomain.py | 1 - 5 files changed, 55 insertions(+), 45 deletions(-) diff --git a/src/masternodes/consensus/xvm.cpp b/src/masternodes/consensus/xvm.cpp index e468d1bc52..3d7cdc3f1a 100644 --- a/src/masternodes/consensus/xvm.cpp +++ b/src/masternodes/consensus/xvm.cpp @@ -192,29 +192,25 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { // Iterate over array of transfers for (const auto &[src, dst] : obj.transfers) { CrossBoundaryResult result; - auto hash = evm_try_get_tx_hash(result, HexStr(dst.data)); - if (!result.ok) { - return Res::Err("Error bridging DFI: %s", result.reason); - } - evmTxHash = std::string(hash.data(), hash.length()).substr(2); - if (src.domain == static_cast(VMDomain::DVM) && dst.domain == static_cast(VMDomain::EVM)) { - // Check if destination address is a contract - { - CTxDestination dest; - ExtractDestination(dst.address, dest); - const auto toAddress = std::get(dest); + CTxDestination dest; + ExtractDestination(dst.address, dest); + const auto toAddress = std::get(dest); - CrossBoundaryResult result; - auto isSmartContract = evm_is_smart_contract_in_q(result, toAddress.GetHex(), evmQueueId); + // Check if destination address is a contract + auto isSmartContract = evm_is_smart_contract_in_q(result, toAddress.GetHex(), evmQueueId); + if (!result.ok) { + return Res::Err("transferdomain error: %s", result.reason); + } + if (isSmartContract) { + return Res::Err("transferdomain error: EVM destination is a smart contract"); + } - if (!result.ok) { - return Res::Err("transferdomain error: %s", result.reason); - } - if (isSmartContract) { - return Res::Err("transferdomain error: EVM destination is a smart contract"); - } + auto hash = evm_try_get_tx_hash(result, HexStr(dst.data)); + if (!result.ok) { + return Res::Err("Error bridging DFI: %s", result.reason); } + evmTxHash = std::string(hash.data(), hash.length()).substr(2); // Subtract balance from DFI address res = mnview.SubBalance(src.address, src.amount); @@ -225,9 +221,6 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { stats.dvmCurrent.Sub(src.amount); // Add balance to ERC55 address - CTxDestination dest; - ExtractDestination(dst.address, dest); - auto tokenId = dst.amount.nTokenId; if (tokenId == DCT_ID{0}) { evm_unsafe_try_add_balance_in_q(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex()); @@ -245,31 +238,29 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { stats.evmIn.Add(tokenAmount); stats.evmCurrent.Add(tokenAmount); } else if (src.domain == static_cast(VMDomain::EVM) && dst.domain == static_cast(VMDomain::DVM)) { - // Check if source address is a contract - { - CTxDestination dest; - ExtractDestination(src.address, dest); - const auto fromAddress = std::get(dest); + CTxDestination dest; + ExtractDestination(src.address, dest); + const auto fromAddress = std::get(dest); - CrossBoundaryResult result; - auto isSmartContract = evm_is_smart_contract_in_q(result, fromAddress.GetHex(), evmQueueId); + // Check if source address is a contract + auto isSmartContract = evm_is_smart_contract_in_q(result, fromAddress.GetHex(), evmQueueId); + if (!result.ok) { + return Res::Err("transferdomain error: %s", result.reason); + } + if (isSmartContract) { + return Res::Err("transferdomain error: EVM source is a smart contract"); + } - if (!result.ok) { - return Res::Err("transferdomain error: %s", result.reason); - } - if (isSmartContract) { - return Res::Err("transferdomain error: EVM source is a smart contract"); - } + auto hash = evm_try_get_tx_hash(result, HexStr(src.data)); + if (!result.ok) { + return Res::Err("Error bridging DFI: %s", result.reason); } + evmTxHash = std::string(hash.data(), hash.length()).substr(2); // Subtract balance from ERC55 address - CTxDestination dest; - ExtractDestination(src.address, dest); - auto tokenId = dst.amount.nTokenId; - CrossBoundaryResult result; if (tokenId == DCT_ID{0}) { - if (!evm_unsafe_try_sub_balance_in_q(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex())) { + if (!evm_unsafe_try_sub_balance_in_q(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex())) { return DeFiErrors::TransferDomainNotEnoughBalance(EncodeDestination(dest)); } if (!result.ok) { @@ -277,7 +268,7 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { } } else { - evm_try_bridge_dst20(result, evmQueueId, HexStr(dst.data), tx.GetHash().GetHex(), tokenId.v, false); + evm_try_bridge_dst20(result, evmQueueId, HexStr(src.data), tx.GetHash().GetHex(), tokenId.v, false); if (!result.ok) { return Res::Err("Error bridging DST20: %s", result.reason); } diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 6788e47089..13bddda639 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -2153,7 +2153,11 @@ UniValue transferdomain(const JSONRPCRequest& request) { std::vector evmTx(signedTx.size()); std::copy(signedTx.begin(), signedTx.end(), evmTx.begin()); - dst.data = evmTx; + if (isEVMIn) { + dst.data = evmTx; + } else { + src.data = evmTx; + } msg.transfers.push_back({src, dst}); } diff --git a/src/miner.cpp b/src/miner.cpp index 4473f9855f..d1c18c73f4 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -630,7 +630,15 @@ bool BlockAssembler::EvmTxPreapply(const EvmTxPreApplyContext& ctx) if (obj.transfers.size() != 1) { return false; } - auto senderInfo = evm_try_get_tx_sender_info_from_raw_tx(result, HexStr(obj.transfers[0].second.data)); + + 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) { return false; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 4ba0f0e06f..c4757d3e52 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1304,7 +1304,15 @@ Res CTxMemPool::rebuildAccountsView(int height, const CCoinsViewCache& coinsCach } continue; } - auto senderInfo = evm_try_get_tx_sender_info_from_raw_tx(result, HexStr(obj.transfers[0].second.data)); + + 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()); diff --git a/test/functional/feature_evm_transferdomain.py b/test/functional/feature_evm_transferdomain.py index 6a730c8409..72887f17db 100755 --- a/test/functional/feature_evm_transferdomain.py +++ b/test/functional/feature_evm_transferdomain.py @@ -736,7 +736,6 @@ def invalid_transfer_sc(self): tx, self.evm_key_pair.privkey ) hash = self.nodes[0].w3.eth.send_raw_transaction(signed.rawTransaction) - self.nodes[0].generate(1) receipt = self.nodes[0].w3.eth.wait_for_transaction_receipt(hash) From dcaaa59dc39df55f6ea2823bb6484d35e09af4be Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 13 Sep 2023 21:55:28 +0800 Subject: [PATCH 2/3] fix py lint --- test/functional/feature_evm_transferdomain.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_evm_transferdomain.py b/test/functional/feature_evm_transferdomain.py index 72887f17db..76c53ae210 100755 --- a/test/functional/feature_evm_transferdomain.py +++ b/test/functional/feature_evm_transferdomain.py @@ -775,7 +775,9 @@ def invalid_transfer_sc_mempool(self): "gas": 1_000_000, } ) - signed = self.nodes[0].w3.eth.account.sign_transaction(tx, self.evm_key_pair.privkey) + signed = self.nodes[0].w3.eth.account.sign_transaction( + tx, self.evm_key_pair.privkey + ) self.nodes[0].w3.eth.send_raw_transaction(signed.rawTransaction) contract_address = web3.utils.get_create_address( From 7f924f2aebc80f8ef44c09c5b5ef07f6fa4565f5 Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 13 Sep 2023 23:56:46 +0800 Subject: [PATCH 3/3] Add checks to transfer domain validation --- src/masternodes/consensus/xvm.cpp | 30 ++++++++++++++++++++---------- src/masternodes/errors.h | 8 ++++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/masternodes/consensus/xvm.cpp b/src/masternodes/consensus/xvm.cpp index 3d7cdc3f1a..00d789f867 100644 --- a/src/masternodes/consensus/xvm.cpp +++ b/src/masternodes/consensus/xvm.cpp @@ -194,16 +194,21 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { CrossBoundaryResult result; if (src.domain == static_cast(VMDomain::DVM) && dst.domain == static_cast(VMDomain::EVM)) { CTxDestination dest; - ExtractDestination(dst.address, dest); - const auto toAddress = std::get(dest); + if (!ExtractDestination(dst.address, dest)) { + return DeFiErrors::TransferDomainETHDestAddress(); + } + const auto toAddress = std::get_if(&dest); + if (!toAddress) { + return DeFiErrors::TransferDomainETHSourceAddress(); + } // Check if destination address is a contract - auto isSmartContract = evm_is_smart_contract_in_q(result, toAddress.GetHex(), evmQueueId); + auto isSmartContract = evm_is_smart_contract_in_q(result, toAddress->GetHex(), evmQueueId); if (!result.ok) { - return Res::Err("transferdomain error: %s", result.reason); + return Res::Err("Error checking contract address: %s", result.reason); } if (isSmartContract) { - return Res::Err("transferdomain error: EVM destination is a smart contract"); + return DeFiErrors::TransferDomainSmartContractDestAddress(); } auto hash = evm_try_get_tx_hash(result, HexStr(dst.data)); @@ -239,16 +244,21 @@ Res CXVMConsensus::operator()(const CTransferDomainMessage &obj) const { stats.evmCurrent.Add(tokenAmount); } else if (src.domain == static_cast(VMDomain::EVM) && dst.domain == static_cast(VMDomain::DVM)) { CTxDestination dest; - ExtractDestination(src.address, dest); - const auto fromAddress = std::get(dest); + if (!ExtractDestination(src.address, dest)) { + return DeFiErrors::TransferDomainETHSourceAddress(); + } + const auto fromAddress = std::get_if(&dest); + if (!fromAddress) { + return DeFiErrors::TransferDomainETHSourceAddress(); + } // Check if source address is a contract - auto isSmartContract = evm_is_smart_contract_in_q(result, fromAddress.GetHex(), evmQueueId); + auto isSmartContract = evm_is_smart_contract_in_q(result, fromAddress->GetHex(), evmQueueId); if (!result.ok) { - return Res::Err("transferdomain error: %s", result.reason); + return Res::Err("Error checking contract address: %s", result.reason); } if (isSmartContract) { - return Res::Err("transferdomain error: EVM source is a smart contract"); + return DeFiErrors::TransferDomainSmartContractSourceAddress(); } auto hash = evm_try_get_tx_hash(result, HexStr(src.data)); diff --git a/src/masternodes/errors.h b/src/masternodes/errors.h index 4f50f30cf9..d81dfec413 100644 --- a/src/masternodes/errors.h +++ b/src/masternodes/errors.h @@ -518,6 +518,14 @@ class DeFiErrors { return Res::Err("TransferDomain currently only supports a single transfer per transaction"); } + static Res TransferDomainSmartContractSourceAddress() { + return Res::Err("TransferDomain EVM source is a smart contract"); + } + + static Res TransferDomainSmartContractDestAddress() { + return Res::Err("TransferDomain EVM destination is a smart contract"); + } + static Res SettingEVMAttributeFailure() { return Res::Err("Failed to set EVM attribute"); }