From a9df3e42b434c5f0a853192d0b68d4c8b688ed4a Mon Sep 17 00:00:00 2001 From: yanmaani Date: Sat, 17 Oct 2020 12:00:00 +0000 Subject: [PATCH] rpc, test: Make name value optional in name_firstupdate and name_update --- src/wallet/rpcnames.cpp | 50 ++++++++++++++------- test/functional/name_novalue.py | 60 +++++++++++++++++++++++++ test/functional/test_framework/names.py | 8 ++-- test/functional/test_runner.py | 1 + 4 files changed, 100 insertions(+), 19 deletions(-) create mode 100755 test/functional/name_novalue.py diff --git a/src/wallet/rpcnames.cpp b/src/wallet/rpcnames.cpp index a542c9129b..a1b097a43a 100644 --- a/src/wallet/rpcnames.cpp +++ b/src/wallet/rpcnames.cpp @@ -447,7 +447,7 @@ name_firstupdate (const JSONRPCRequest& request) /* We can not use RPCHelpMan::Check here, as we have that "hidden" sixth argument for "allow active" names (in tests). */ - if (request.fHelp || request.params.size () < 4 || request.params.size () > 6) + if (request.fHelp || request.params.size () < 3 || request.params.size () > 6) throw std::runtime_error ( RPCHelpMan ("name_firstupdate", "\nFinishes the registration of a name. Depends on name_new being already issued." @@ -456,7 +456,7 @@ name_firstupdate (const JSONRPCRequest& request) {"name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name to register"}, {"rand", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The rand value of name_new"}, {"tx", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The name_new txid"}, - {"value", RPCArg::Type::STR, RPCArg::Optional::NO, "Value for the name"}, + {"value", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Value for the name"}, optHelp.buildRpcArg (), }, RPCResult {RPCResult::Type::STR_HEX, "", "the transaction ID"}, @@ -473,7 +473,7 @@ name_firstupdate (const JSONRPCRequest& request) RPCTypeCheck (request.params, {UniValue::VSTR, UniValue::VSTR, UniValue::VSTR, UniValue::VSTR, - UniValue::VOBJ}); + UniValue::VOBJ}, true); UniValue options(UniValue::VOBJ); if (request.params.size () >= 5) @@ -489,7 +489,11 @@ name_firstupdate (const JSONRPCRequest& request) const uint256 prevTxid = ParseHashV (request.params[2], "txid"); - const valtype value = DecodeValueFromRPCOrThrow (request.params[3], options); + const bool isDefaultVal = (request.params.size () < 4 || request.params[3].isNull ()); + const valtype value = isDefaultVal ? + valtype (): + DecodeValueFromRPCOrThrow (request.params[3], options); + if (value.size () > MAX_VALUE_LENGTH_UI) throw JSONRPCError (RPC_INVALID_PARAMETER, "the value is too long"); @@ -567,7 +571,7 @@ name_update () + HELP_REQUIRING_PASSPHRASE, { {"name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name to update"}, - {"value", RPCArg::Type::STR, RPCArg::Optional::NO, "Value for the name"}, + {"value", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Value for the name"}, optHelp.buildRpcArg (), }, RPCResult {RPCResult::Type::STR_HEX, "", "the transaction ID"}, @@ -583,7 +587,7 @@ name_update () CWallet* const pwallet = wallet.get (); RPCTypeCheck (request.params, - {UniValue::VSTR, UniValue::VSTR, UniValue::VOBJ}); + {UniValue::VSTR, UniValue::VSTR, UniValue::VOBJ}, true); UniValue options(UniValue::VOBJ); if (request.params.size () >= 3) @@ -593,14 +597,21 @@ name_update () if (name.size () > MAX_NAME_LENGTH) throw JSONRPCError (RPC_INVALID_PARAMETER, "the name is too long"); - const valtype value = DecodeValueFromRPCOrThrow (request.params[1], options); - if (value.size () > MAX_VALUE_LENGTH_UI) - throw JSONRPCError (RPC_INVALID_PARAMETER, "the value is too long"); + const bool isDefaultVal = request.params.size() < 2 || request.params[1].isNull(); - /* For finding the name output to spend, we first check if there are - pending operations on the name in the mempool. If there are, then we - build upon the last one to get a valid chain. If there are none, then we - look up the last outpoint from the name database instead. */ + valtype value; + if (!isDefaultVal) { + value = DecodeValueFromRPCOrThrow (request.params[1], options); + if (value.size () > MAX_VALUE_LENGTH_UI) + throw JSONRPCError (RPC_INVALID_PARAMETER, "the value is too long"); + } + + /* For finding the name output to spend and its value, we first check if + there are pending operations on the name in the mempool. If there + are, then we build upon the last one to get a valid chain. If there + are none, then we look up the last outpoint from the name database + instead. */ + // TODO: Use name_show for this instead. const unsigned chainLimit = gArgs.GetArg ("-limitnamechains", DEFAULT_NAME_CHAIN_LIMIT); @@ -616,7 +627,14 @@ name_update () " on this name"); if (pendingOps > 0) - outp = mempool.lastNameOutput (name); + { + outp = mempool.lastNameOutput (name); + if (isDefaultVal) + { + const auto& tx = mempool.mapTx.find(outp.hash)->GetTx(); + value = CNameScript(tx.vout[outp.n].scriptPubKey).getOpValue(); + } + } } if (outp.IsNull ()) @@ -628,10 +646,10 @@ name_update () if (!coinsTip.GetName (name, oldData) || oldData.isExpired ()) throw JSONRPCError (RPC_TRANSACTION_ERROR, "this name can not be updated"); - + if (isDefaultVal) + value = oldData.getValue(); outp = oldData.getUpdateOutpoint (); } - assert (!outp.IsNull ()); const CTxIn txIn(outp); diff --git a/test/functional/name_novalue.py b/test/functional/name_novalue.py new file mode 100755 index 0000000000..8833878636 --- /dev/null +++ b/test/functional/name_novalue.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +# Licensed under CC0 (Public domain) + +# Test that name_firstupdate and name_update handle missing +# name values gracefully. This test uses direct RPC calls. +# Testing the wrappers is out of scope. + +from test_framework.names import NameTestFramework +from test_framework.util import * + +class NameNovalueTest(NameTestFramework): + + def set_test_params(self): + self.setup_clean_chain = True + self.setup_name_test ([["-namehistory", "-limitnamechains=3"]]) + + def run_test(self): + node = self.nodes[0] + node.generate(200) + + new_name = node.name_new("d/name") + node.generate(12) + self.log.info("Began registration of name.") + + # use node.name_firstupdate - we're not testing the framework + node.name_firstupdate ("d/name", new_name[1], new_name[0]) + node.generate(1) + self.log.info("Name registered; no value provided.") + + self.checkName(0, "d/name", "", 30, False) + self.log.info("Value equals empty string.") + + node.name_update("d/name", "1") + node.generate(1) + self.log.info('Value changed to "1"; change is in chain.') + + self.checkName(0, "d/name", "1", 30, False) + self.log.info('Value equals "1".') + + node.name_update("d/name") + node.generate(1) + self.log.info("Updated; no value specified.") + + self.checkName(0, "d/name", "1", 30, False) + self.log.info('Name was updated. Value still equals "1".') + + node.name_update("d/name", "2") + node.name_update("d/name") + node.name_update("d/name", "3") + node.generate(1) + self.log.info('Stack of 3 registrations performed.') + + history = node.name_history("d/name") + reference = ["", "1", "1", "2", "2", "3"] + assert(list(map(lambda x: x['value'], history)) == reference) + self.log.info('Updates correctly consider updates in the mempool.') + + +if __name__ == '__main__': + NameNovalueTest ().main () diff --git a/test/functional/test_framework/names.py b/test/functional/test_framework/names.py index a617d8e24f..252aa80e3f 100644 --- a/test/functional/test_framework/names.py +++ b/test/functional/test_framework/names.py @@ -21,7 +21,7 @@ def setup_name_test (self, args = [[]] * 4): # test_framework.py. This is needed to get us out of IBD. self.mocktime = 1388534400 + (201 * 10 * 60) - def firstupdateName (self, ind, name, newData, value, + def firstupdateName (self, ind, name, newData, value = None, opt = None, allowActive = False): """ Utility routine to perform a name_firstupdate command. The rand @@ -36,9 +36,11 @@ def firstupdateName (self, ind, name, newData, value, return node.name_firstupdate (name, newData[1], newData[0], value, opt, True) - if opt is None: + if opt is not None: + return node.name_firstupdate (name, newData[1], newData[0], value, opt) + if value is not None: return node.name_firstupdate (name, newData[1], newData[0], value) - return node.name_firstupdate (name, newData[1], newData[0], value, opt) + return node.name_firstupdate (name, newData[1], newData[0]) def checkName (self, ind, name, value, expiresIn, expired): """ diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c913c4978c..e36172051b 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -280,6 +280,7 @@ 'name_multisig.py', 'name_multisig.py --bip16-active', 'name_multiupdate.py', + 'name_novalue.py', 'name_pending.py', 'name_psbt.py', 'name_rawtx.py',