Skip to content

Commit

Permalink
RPC: Add single key check for source and destination addresses in tra…
Browse files Browse the repository at this point in the history
…nsferdomain RPC (#2709)

* Add single key flag check in transferdomain RPC, defaults to true

* Add invalid single key check test

* Fix fmt cpp

* Add legacy address support in addressmap

* Better refactor, add addressmap tests for legacy addresses

* Fix ethlibs test fixture script

* Fix typo

---------

Co-authored-by: Prasanna Loganathar <pvl@prasannavl.com>
  • Loading branch information
sieniven and prasannavl authored Nov 21, 2023
1 parent 75d67f1 commit 409a891
Showing 26 changed files with 504 additions and 35 deletions.
2 changes: 1 addition & 1 deletion ci/ethlibs_test/main.sh
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ setup_fixtures() {
"v0/params/feature/transferdomain": "true"}}'
$DEFI_CLI_BIN -regtest generatetoaddress 2 "$OWNERAUTHADDR"

$DEFI_CLI_BIN -regtest transferdomain '[{"src":{"address":"'"$OWNERAUTHADDR"'", "amount":"200@DFI", "domain":2}, "dst":{"address":"'"$ALICE"'", "amount":"200@DFI", "domain":3}}]'
$DEFI_CLI_BIN -regtest transferdomain '[{"src":{"address":"'"$OWNERAUTHADDR"'", "amount":"200@DFI", "domain":2}, "dst":{"address":"'"$ALICE"'", "amount":"200@DFI", "domain":3}, "singlekeycheck": false}]'
$DEFI_CLI_BIN -regtest generatetoaddress 1 "$OWNERAUTHADDR"

$DEFI_CLI_BIN -regtest eth_sendTransaction '{"from":"'"$ALICE"'", "data":"'"$CONTRACT_COUNTER"'", "value":"0x00", "gas":"0x7a120", "gasPrice": "0x22ecb25c00"}'
44 changes: 38 additions & 6 deletions src/dfi/rpc_accounts.cpp
Original file line number Diff line number Diff line change
@@ -2172,10 +2172,19 @@ UniValue transferdomain(const JSONRPCRequest &request) {
// {"data", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Optional data"},
},
},
{"nonce",
RPCArg::Type::NUM,
RPCArg::Optional::OMITTED,
"Optional parameter to specify the transaction nonce"},
{
"nonce",
RPCArg::Type::NUM,
RPCArg::Optional::OMITTED,
"Optional parameter to specify the transaction nonce",

This comment has been minimized.

Copy link
@prasannavl

prasannavl Nov 21, 2023

Author Member

We might have to support a nonce of -1 for auto, since we have a items after this.

},
{
"singlekeycheck",
RPCArg::Type::BOOL,
RPCArg::Optional::OMITTED,
"Optional flag to ensure single key check between the corresponding address types "
"(default = true)",
},
},
},
},
@@ -2210,12 +2219,21 @@ UniValue transferdomain(const JSONRPCRequest &request) {
std::vector<std::pair<std::string, uint64_t>> nonce_cache;

for (unsigned int i = 0; i < srcDstArray.size(); i++) {
const UniValue &elem = srcDstArray[i];
RPCTypeCheck(elem, {UniValue::VOBJ, UniValue::VOBJ, UniValue::VNUM}, false);
const UniValue &elem = srcDstArray[i].get_obj();
RPCTypeCheckObj(elem,
{
{"src", UniValueType(UniValue::VOBJ) },
{"dst", UniValueType(UniValue::VOBJ) },
{"nonce", UniValueType(UniValue::VNUM) },
{"singlekeycheck", UniValueType(UniValue::VBOOL)},
},
true,
true);

const UniValue &srcObj = elem["src"].get_obj();
const UniValue &dstObj = elem["dst"].get_obj();
const UniValue &nonceObj = elem["nonce"];
const UniValue &singlekeycheckObj = elem["singlekeycheck"];

CTransferDomainItem src, dst;

@@ -2290,6 +2308,20 @@ UniValue transferdomain(const JSONRPCRequest &request) {
// if (!dstObj["data"].isNull())
// dst.data.assign(dstObj["data"].getValStr().begin(), dstObj["data"].getValStr().end());

// Single key check
bool singlekeycheck = true;
if (!singlekeycheckObj.isNull()) {
singlekeycheck = singlekeycheckObj.getBool();
}
if (singlekeycheck) {
auto dstKey = AddrToPubKey(pwallet, ScriptToString(dst.address));
auto [uncompSrcKey, compSrcKey] = GetBothPubkeyCompressions(srcKey);
auto [uncompDstKey, compDstKey] = GetBothPubkeyCompressions(dstKey);
if (uncompSrcKey != uncompDstKey || compSrcKey != compDstKey) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Dst address does not match source key");
}
}

// Create signed EVM TX
CKey key;
if (!pwallet->GetKey(srcKey.GetID(), key)) {
7 changes: 3 additions & 4 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
@@ -4330,10 +4330,9 @@ UniValue addressmap(const JSONRPCRequest &request) {
throwInvalidAddressFmt();
}
CPubKey key = AddrToPubKey(pwallet, input);
if (!key.IsCompressed()) {
key.Compress();
}
format.pushKV("bech32", EncodeDestination(WitnessV0KeyHash(key)));
auto [uncomp, comp] = GetBothPubkeyCompressions(key);
format.pushKV("bech32", EncodeDestination(WitnessV0KeyHash(comp)));
format.pushKV("legacy", EncodeDestination(PKHash(uncomp)));
break;
}
default:
61 changes: 39 additions & 22 deletions test/functional/feature_address_map.py
Original file line number Diff line number Diff line change
@@ -127,14 +127,17 @@ def addressmap_address_basics_manual_import(self):
]
addr_maps = [
[
"n32jT7A5sv6t9Hm2NAYK6juuwsCdsWe1SF",
"bcrt1qmhpq9hxgdglwja6uruc92yne8ekxljgykrfta5",
"0xfD0766e7aBe123A25c73c95f6dc3eDe26D0b7263",
],
[
"n4VvfBJBTQpDf8ooyjgehiq9mTjeKhcwEc",
"bcrt1qtqggfdte5jp8duffzmt54aqtqwlv3l8xsjdrhf",
"0x4d07A76Db2a281a348d5A5a1833F4322D77799d5",
],
[
"n1HUChzN3RR89jhtehNBNboLUnt4QNAJ8E",
"bcrt1qdw7fqrq9n2d530uh05vdm2yvpag2ydm0z67yc5",
"0x816a4DDbC26B80602767B13Fb17B2e1785125BE7",
],
@@ -151,52 +154,66 @@ def addressmap_address_basics_manual_import(self):
#
# self.nodes[0].importprivkey(wif)
self.nodes[0].importprivkey(rawkey)
for [dfi_addr, eth_addr] in addr_maps:
for [legacy_address, bech32_address, erc55_address] in addr_maps:
# dvm -> evm
res = self.nodes[0].addressmap(
dfi_addr, AddressConversionType.DVMToEVMAddress
legacy_address, AddressConversionType.DVMToEVMAddress
)
assert_equal(res["input"], dfi_addr)
assert_equal(res["input"], legacy_address)
assert_equal(res["type"], 1)
assert_equal(res["format"]["erc55"], eth_addr)
assert_equal(res["format"]["erc55"], erc55_address)

res = self.nodes[0].addressmap(
bech32_address, AddressConversionType.DVMToEVMAddress
)
assert_equal(res["input"], bech32_address)
assert_equal(res["type"], 1)
assert_equal(res["format"]["erc55"], erc55_address)

# evm -> dvm
res = self.nodes[0].addressmap(
eth_addr, AddressConversionType.EVMToDVMAddress
erc55_address, AddressConversionType.EVMToDVMAddress
)
assert_equal(res["input"], eth_addr)
assert_equal(res["input"], erc55_address)
assert_equal(res["type"], 2)
assert_equal(res["format"]["bech32"], dfi_addr)
assert_equal(res["format"]["legacy"], legacy_address)
assert_equal(res["format"]["bech32"], bech32_address)

# auto (dvm -> evm)
res = self.nodes[0].addressmap(dfi_addr, AddressConversionType.Auto)
assert_equal(res["input"], dfi_addr)
res = self.nodes[0].addressmap(legacy_address, AddressConversionType.Auto)
assert_equal(res["input"], legacy_address)
assert_equal(res["type"], 1)
assert_equal(res["format"]["erc55"], erc55_address)

res = self.nodes[0].addressmap(bech32_address, AddressConversionType.Auto)
assert_equal(res["input"], bech32_address)
assert_equal(res["type"], 1)
assert_equal(res["format"]["erc55"], eth_addr)
assert_equal(res["format"]["erc55"], erc55_address)

# auto (evm -> dvm)
res = self.nodes[0].addressmap(eth_addr, AddressConversionType.Auto)
assert_equal(res["input"], eth_addr)
res = self.nodes[0].addressmap(erc55_address, AddressConversionType.Auto)
assert_equal(res["input"], erc55_address)
assert_equal(res["type"], 2)
assert_equal(res["format"]["bech32"], dfi_addr)
assert_equal(res["format"]["legacy"], legacy_address)
assert_equal(res["format"]["bech32"], bech32_address)

def addressmap_valid_address_not_present_should_fail(self):
self.rollback_to(self.start_block_height)
# Give an address that is not own by the node. THis should fail since we don't have the public key of the address.
eth_address = self.nodes[1].getnewaddress("", "erc55")
erc55_addressess = self.nodes[1].getnewaddress("", "erc55")
assert_raises_rpc_error(
-5,
"no full public key for address " + eth_address,
"no full public key for address " + erc55_addressess,
self.nodes[0].addressmap,
eth_address,
erc55_addressess,
AddressConversionType.EVMToDVMAddress,
)

def addressmap_valid_address_invalid_type_should_fail(self):
self.rollback_to(self.start_block_height)
address = self.nodes[0].getnewaddress("", "legacy")
p2sh_address = self.nodes[0].getnewaddress("", "p2sh-segwit")
eth_address = self.nodes[0].getnewaddress("", "erc55")
erc55_addressess = self.nodes[0].getnewaddress("", "erc55")
assert_invalid_parameter = lambda *args: assert_raises_rpc_error(
-8, "Invalid type parameter", self.nodes[0].addressmap, *args
)
@@ -205,27 +222,27 @@ def addressmap_valid_address_invalid_type_should_fail(self):
)
assert_invalid_parameter(address, 9)
assert_invalid_parameter(address, -1)
assert_invalid_format(eth_address, AddressConversionType.DVMToEVMAddress)
assert_invalid_format(erc55_addressess, AddressConversionType.DVMToEVMAddress)
assert_invalid_format(address, AddressConversionType.EVMToDVMAddress)
assert_invalid_format(p2sh_address, AddressConversionType.DVMToEVMAddress)
assert_invalid_format(p2sh_address, AddressConversionType.DVMToEVMAddress)

def addressmap_invalid_address_should_fail(self):
self.rollback_to(self.start_block_height)
# Check that addressmap is failing on wrong input
eth_address = "0x0000000000000000000000000000000000000000"
erc55_addressess = "0x0000000000000000000000000000000000000000"
assert_raises_rpc_error(
-5,
eth_address + " does not refer to a key",
erc55_addressess + " does not refer to a key",
self.nodes[0].addressmap,
eth_address,
erc55_addressess,
AddressConversionType.EVMToDVMAddress,
)
assert_raises_rpc_error(
-8,
"Invalid address format",
self.nodes[0].addressmap,
eth_address,
erc55_addressess,
AddressConversionType.DVMToEVMAddress,
)
assert_raises_rpc_error(
13 changes: 13 additions & 0 deletions test/functional/feature_evm.py
Original file line number Diff line number Diff line change
@@ -63,6 +63,7 @@ def test_tx_without_chainid(self):
"amount": "50@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
@@ -361,6 +362,7 @@ def verify_evm_not_enabled():
"amount": "100@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
],
)
@@ -383,6 +385,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"amount": "100@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
],
)
@@ -418,6 +421,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"amount": "100@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
],
)
@@ -447,6 +451,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"amount": "100@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
],
)
@@ -581,6 +586,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"amount": "100@DFI",
"domain": 2,
},
"singlekeycheck": False,
}
],
)
@@ -610,6 +616,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"domain": 3,
},
"dst": {"address": self.address, "amount": "100@DFI", "domain": 2},
"singlekeycheck": False,
}
],
)
@@ -639,6 +646,7 @@ def verify_transferdomain_not_enabled_post_evm_on():
"domain": 3,
},
"dst": {"address": self.address, "amount": "100@DFI", "domain": 2},
"singlekeycheck": False,
}
],
)
@@ -738,6 +746,7 @@ def setup_accounts(self):
"amount": "200@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
@@ -1362,6 +1371,7 @@ def encrypt_wallet(self):
"amount": "0.1@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
@@ -1377,6 +1387,7 @@ def encrypt_wallet(self):
"amount": "0.1@DFI",
"domain": 2,
},
"singlekeycheck": False,
}
]
)
@@ -1396,6 +1407,7 @@ def delete_account_from_trie(self):
"amount": "2@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
@@ -1416,6 +1428,7 @@ def delete_account_from_trie(self):
"amount": "20@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
1 change: 1 addition & 0 deletions test/functional/feature_evm_contract_env_vars.py
Original file line number Diff line number Diff line change
@@ -73,6 +73,7 @@ def should_create_contract(self):
"amount": "50@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
1 change: 1 addition & 0 deletions test/functional/feature_evm_contracts.py
Original file line number Diff line number Diff line change
@@ -75,6 +75,7 @@ def setup(self):
"amount": "50@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
1 change: 1 addition & 0 deletions test/functional/feature_evm_dfi_intrinsics.py
Original file line number Diff line number Diff line change
@@ -147,6 +147,7 @@ def run_test(self):
"amount": "50@DFI",
"domain": 3,
},
"singlekeycheck": False,
}
]
)
Loading

0 comments on commit 409a891

Please sign in to comment.