Skip to content

Commit

Permalink
In TransferDomain handle ExtractDestination failure (#2117)
Browse files Browse the repository at this point in the history 
* Handle ExtractDestination failure

* Fork guard ExtractDestination failure

* Limit DVM transfers to PKHash and Bech32

* Correct error messages and test
  • Loading branch information
@Bushstar
Bushstar authored Jun 26, 2023
1 parent 927ce69 commit 30399f6
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 20 deletions.
8 changes: 4 additions & 4 deletions src/masternodes/errors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,23 +427,23 @@ 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() {
return Res::Err("Invalid domain set for \"src\" argument");
}

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() {
Expand Down
70 changes: 58 additions & 12 deletions src/masternodes/mn_checks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -4009,9 +4031,15 @@ Res ValidateTransferDomain(const CTransaction &tx,
// Check domain type
if (src.domain == static_cast<uint8_t>(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<uint32_t>(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
Expand All @@ -4020,9 +4048,15 @@ Res ValidateTransferDomain(const CTransaction &tx,
return res;
} else if (src.domain == static_cast<uint8_t>(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<uint32_t>(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
Expand All @@ -4036,16 +4070,28 @@ Res ValidateTransferDomain(const CTransaction &tx,
// Check domain type
if (dst.domain == static_cast<uint8_t>(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<uint32_t>(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<uint8_t>(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<uint32_t>(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
Expand Down
8 changes: 4 additions & 4 deletions test/functional/feature_evm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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}}])
Expand Down Expand Up @@ -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}}])
Expand Down

0 comments on commit 30399f6

Please sign in to comment.