From 30399f6f04e532fb2dc2c9c462550f8e53a64386 Mon Sep 17 00:00:00 2001 From: Peter John Bushnell Date: Mon, 26 Jun 2023 15:50:24 +0100 Subject: [PATCH] In TransferDomain handle ExtractDestination failure (#2117) * Handle ExtractDestination failure * Fork guard ExtractDestination failure * Limit DVM transfers to PKHash and Bech32 * Correct error messages and test --- src/masternodes/errors.h | 8 ++-- src/masternodes/mn_checks.cpp | 70 ++++++++++++++++++++++++++++------ test/functional/feature_evm.py | 8 ++-- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/masternodes/errors.h b/src/masternodes/errors.h index fb706e5a46..58aeb04705 100644 --- a/src/masternodes/errors.h +++ b/src/masternodes/errors.h @@ -427,11 +427,11 @@ class DeFiErrors { } static Res TransferDomainETHSourceAddress() { - return Res::Err("Src address must not be an ETH address in case of \"DVM\" domain"); + return Res::Err("Src address must be an ETH address in case of \"EVM\" domain"); } static Res TransferDomainDFISourceAddress() { - return Res::Err("Src address must be an ETH address in case of \"EVM\" domain"); + return Res::Err("Src address must be a legacy or Bech32 address in case of \"DVM\" domain"); } static Res TransferDomainInvalidSourceDomain() { @@ -439,11 +439,11 @@ class DeFiErrors { } static Res TransferDomainETHDestinationAddress() { - return Res::Err("Dst address must not be an ETH address in case of \"DVM\" domain"); + return Res::Err("Dst address must be an ETH address in case of \"EVM\" domain"); } static Res TransferDomainDVMDestinationAddress() { - return Res::Err("Dst address must be an ETH address in case of \"EVM\" domain"); + return Res::Err("Dst address must be a legacy or Bech32 address in case of \"DVM\" domain"); } static Res TransferDomainInvalidDestinationDomain() { diff --git a/src/masternodes/mn_checks.cpp b/src/masternodes/mn_checks.cpp index e20c125202..0002f36d42 100644 --- a/src/masternodes/mn_checks.cpp +++ b/src/masternodes/mn_checks.cpp @@ -3975,6 +3975,28 @@ Res HasAuth(const CTransaction &tx, const CCoinsViewCache &coins, const CScript return DeFiErrors::InvalidAuth(); } +enum class DomainTransferType : uint8_t { + DVM, + EVM, +}; + +static bool DomainTransferAllowedAddress(const CScript &address, DomainTransferType type) { + CTxDestination dest; + if (ExtractDestination(address, dest)) { + if (type == DomainTransferType::DVM) { + if (dest.index() == PKHashType || dest.index() == WitV0KeyHashType) { + return true; + } + } else if (type == DomainTransferType::EVM) { + if (dest.index() == WitV16KeyEthHashType) { + return true; + } + } + } + + return false; +} + Res ValidateTransferDomain(const CTransaction &tx, uint32_t height, const CCoinsViewCache &coins, @@ -4009,9 +4031,15 @@ Res ValidateTransferDomain(const CTransaction &tx, // Check domain type if (src.domain == static_cast(VMDomain::DVM)) { // Reject if source address is ETH address - if (ExtractDestination(src.address, dest)) { - if (dest.index() == WitV16KeyEthHashType) { - return DeFiErrors::TransferDomainETHSourceAddress(); + if (height < static_cast(consensus.ChangiIntermediateHeight3)) { + if (ExtractDestination(src.address, dest)) { + if (dest.index() == WitV16KeyEthHashType) { + return DeFiErrors::TransferDomainDFISourceAddress(); + } + } + } else { + if (!DomainTransferAllowedAddress(src.address, DomainTransferType::DVM)) { + return DeFiErrors::TransferDomainDFISourceAddress(); } } // Check for authorization on source address @@ -4020,9 +4048,15 @@ Res ValidateTransferDomain(const CTransaction &tx, return res; } else if (src.domain == static_cast(VMDomain::EVM)) { // Reject if source address is DFI address - if (ExtractDestination(src.address, dest)) { - if (dest.index() != WitV16KeyEthHashType) { - return DeFiErrors::TransferDomainDFISourceAddress(); + if (height < static_cast(consensus.ChangiIntermediateHeight3)) { + if (ExtractDestination(src.address, dest)) { + if (dest.index() != WitV16KeyEthHashType) { + return DeFiErrors::TransferDomainETHSourceAddress(); + } + } + } else { + if (!DomainTransferAllowedAddress(src.address, DomainTransferType::EVM)) { + return DeFiErrors::TransferDomainETHSourceAddress(); } } // Check for authorization on source address @@ -4036,16 +4070,28 @@ Res ValidateTransferDomain(const CTransaction &tx, // Check domain type if (dst.domain == static_cast(VMDomain::DVM)) { // Reject if source address is ETH address - if (ExtractDestination(dst.address, dest)) { - if (dest.index() == WitV16KeyEthHashType) { - return DeFiErrors::TransferDomainETHDestinationAddress(); + if (height < static_cast(consensus.ChangiIntermediateHeight3)) { + if (ExtractDestination(dst.address, dest)) { + if (dest.index() == WitV16KeyEthHashType) { + return DeFiErrors::TransferDomainDVMDestinationAddress(); + } + } + } else { + if (!DomainTransferAllowedAddress(dst.address, DomainTransferType::DVM)) { + return DeFiErrors::TransferDomainDVMDestinationAddress(); } } } else if (dst.domain == static_cast(VMDomain::EVM)) { // Reject if source address is DFI address - if (ExtractDestination(dst.address, dest)) { - if (dest.index() != WitV16KeyEthHashType) { - return DeFiErrors::TransferDomainDVMDestinationAddress(); + if (height < static_cast(consensus.ChangiIntermediateHeight3)) { + if (ExtractDestination(dst.address, dest)) { + if (dest.index() != WitV16KeyEthHashType) { + return DeFiErrors::TransferDomainETHDestinationAddress(); + } + } + } else { + if (!DomainTransferAllowedAddress(dst.address, DomainTransferType::EVM)) { + return DeFiErrors::TransferDomainETHDestinationAddress(); } } } else diff --git a/test/functional/feature_evm.py b/test/functional/feature_evm.py index f963e07b49..891dae9f72 100755 --- a/test/functional/feature_evm.py +++ b/test/functional/feature_evm.py @@ -20,8 +20,8 @@ def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True self.extra_args = [ - ['-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-fortcanningepilogueheight=96', '-grandcentralheight=101', '-nextnetworkupgradeheight=105', '-changiintermediateheight=105', '-subsidytest=1', '-txindex=1'], - ['-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-fortcanningepilogueheight=96', '-grandcentralheight=101', '-nextnetworkupgradeheight=105', '-changiintermediateheight=105', '-subsidytest=1', '-txindex=1'] + ['-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-fortcanningepilogueheight=96', '-grandcentralheight=101', '-nextnetworkupgradeheight=105', '-changiintermediateheight=105', '-changiintermediate3height=105', '-subsidytest=1', '-txindex=1'], + ['-dummypos=0', '-txnotokens=0', '-amkheight=50', '-bayfrontheight=51', '-eunosheight=80', '-fortcanningheight=82', '-fortcanninghillheight=84', '-fortcanningroadheight=86', '-fortcanningcrunchheight=88', '-fortcanningspringheight=90', '-fortcanninggreatworldheight=94', '-fortcanningepilogueheight=96', '-grandcentralheight=101', '-nextnetworkupgradeheight=105', '-changiintermediateheight=105', '-changiintermediate3height=105', '-subsidytest=1', '-txindex=1'] ] def test_tx_without_chainid(self, node, keypair): @@ -120,7 +120,7 @@ def run_test(self): assert_raises_rpc_error(-5, "recipient (blablabla) does not refer to any valid address", self.nodes[0].transferdomain, [{"src": {"address":address, "amount":"100@DFI", "domain": 2}, "dst":{"address":"blablabla", "amount":"100@DFI", "domain": 3}}]) # Check for valid values DVM->EVM in transferdomain rpc - assert_raises_rpc_error(-32600, "Src address must not be an ETH address in case of \"DVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 2}, "dst":{"address":eth_address, "amount":"100@DFI", "domain": 3}}]) + assert_raises_rpc_error(-32600, "Src address must be a legacy or Bech32 address in case of \"DVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 2}, "dst":{"address":eth_address, "amount":"100@DFI", "domain": 3}}]) assert_raises_rpc_error(-32600, "Dst address must be an ETH address in case of \"EVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":address, "amount":"100@DFI", "domain": 2}, "dst":{"address":address, "amount":"100@DFI", "domain": 3}}]) assert_raises_rpc_error(-32600, "Cannot transfer inside same domain", self.nodes[0].transferdomain, [{"src": {"address":address, "amount":"100@DFI", "domain": 2}, "dst":{"address":eth_address, "amount":"100@DFI", "domain": 2}}]) assert_raises_rpc_error(-32600, "Source amount must be equal to destination amount", self.nodes[0].transferdomain, [{"src": {"address":address, "amount":"100@DFI", "domain": 2}, "dst":{"address":eth_address, "amount":"101@DFI", "domain": 3}}]) @@ -156,7 +156,7 @@ def run_test(self): # Check for valid values EVM->DVM in transferdomain rpc assert_raises_rpc_error(-32600, "Src address must be an ETH address in case of \"EVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":address, "amount":"100@DFI", "domain": 3}, "dst":{"address":address, "amount":"100@DFI", "domain": 2}}]) - assert_raises_rpc_error(-32600, "Dst address must not be an ETH address in case of \"DVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 3}, "dst":{"address":eth_address, "amount":"100@DFI", "domain": 2}}]) + assert_raises_rpc_error(-32600, "Dst address must be a legacy or Bech32 address in case of \"DVM\" domain", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 3}, "dst":{"address":eth_address, "amount":"100@DFI", "domain": 2}}]) assert_raises_rpc_error(-32600, "Cannot transfer inside same domain", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 3}, "dst":{"address":address, "amount":"100@DFI", "domain": 3}}]) assert_raises_rpc_error(-32600, "Source amount must be equal to destination amount", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@DFI", "domain": 3}, "dst":{"address":address, "amount":"101@DFI", "domain": 2}}]) assert_raises_rpc_error(-32600, "For transferdomain, only DFI token is currently supported", self.nodes[0].transferdomain, [{"src": {"address":eth_address, "amount":"100@BTC", "domain": 3}, "dst":{"address":address, "amount":"100@DFI", "domain": 2}}])