Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC]Add assetlabel support to some commands #655

Merged
merged 1 commit into from
Jul 1, 2019
Merged

[RPC]Add assetlabel support to some commands #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions src/wallet/rpcwallet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
" \"UNSET\"\n"
" \"ECONOMICAL\"\n"
" \"CONSERVATIVE\""},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Hex asset id or asset label for balance."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
{"ignoreblindfail", RPCArg::Type::BOOL, /* default */ "true", "Return a transaction even when a blinding attempt fails due to number of blinded inputs/outputs."},
},
RPCResult{
Expand Down Expand Up @@ -647,14 +647,14 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request)
return NullUniValue;
}

if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
throw std::runtime_error(
RPCHelpMan{"getreceivedbyaddress",
"\nReturns the total amount received by the given address in transactions with at least minconf confirmations.\n",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address for transactions."},
{"minconf", RPCArg::Type::NUM, /* default */ "1", "Only include transactions confirmed at least this many times."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Hex asset id or asset label for balance."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
},
RPCResult{
"amount (numeric) The total amount in " + CURRENCY_UNIT + " received at this address.\n"
Expand Down Expand Up @@ -735,14 +735,14 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request)
return NullUniValue;
}

if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
throw std::runtime_error(
RPCHelpMan{"getreceivedbylabel",
"\nReturns the total amount received by addresses with <label> in transactions with at least [minconf] confirmations.\n",
{
{"label", RPCArg::Type::STR, RPCArg::Optional::NO, "The selected label, may be the default label using \"\"."},
{"minconf", RPCArg::Type::NUM, /* default */ "1", "Only include transactions confirmed at least this many times."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Hex asset id or asset label for balance."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
},
RPCResult{
"amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this label.\n"
Expand Down Expand Up @@ -828,7 +828,7 @@ static UniValue getbalance(const JSONRPCRequest& request)
{"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."},
{"minconf", RPCArg::Type::NUM, /* default */ "0", "Only include transactions confirmed at least this many times."},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Also include balance in watch-only addresses (see 'importaddress')"},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Hex asset id or asset label for balance."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
},
RPCResult{
"amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this wallet.\n"
Expand Down Expand Up @@ -1281,7 +1281,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co

bool has_filtered_address = false;
CTxDestination filtered_address = CNoDestination();
if (!by_label && params.size() > 3 && params[3].get_str() != "") {
if (!by_label && params[3].isStr() && params[3].get_str() != "") {
if (!IsValidDestinationString(params[3].get_str())) {
throw JSONRPCError(RPC_WALLET_ERROR, "address_filter parameter was invalid");
}
Expand Down Expand Up @@ -1391,7 +1391,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co
if(fIsWatchonly)
obj.pushKV("involvesWatchonly", true);
obj.pushKV("address", EncodeDestination(address));
obj.pushKV("amount", AmountMapToUniv(mapAmount, ""));
obj.pushKV("amount", AmountMapToUniv(mapAmount, strasset));
obj.pushKV("confirmations", (nConf == std::numeric_limits<int>::max() ? 0 : nConf));
obj.pushKV("label", label);
UniValue transactions(UniValue::VARR);
Expand Down Expand Up @@ -1444,7 +1444,7 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request)
{"include_empty", RPCArg::Type::BOOL, /* default */ "false", "Whether to include addresses that haven't received any payments."},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Whether to include watch-only addresses (see 'importaddress')."},
{"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present, only return information on this address."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Hex asset id or asset label for balance."},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
},
RPCResult{
"[\n"
Expand Down Expand Up @@ -1918,13 +1918,14 @@ static UniValue gettransaction(const JSONRPCRequest& request)
return NullUniValue;
}

if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
throw std::runtime_error(
RPCHelpMan{"gettransaction",
"\nGet detailed information about in-wallet transaction <txid>\n",
{
{"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"},
{"include_watchonly", RPCArg::Type::BOOL, /* default */ "false", "Whether to include watch-only addresses in balance calculation and details[]"},
{"assetlabel", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Hex asset id or asset label for balance."},
},
RPCResult{
"{\n"
Expand Down Expand Up @@ -1983,6 +1984,11 @@ static UniValue gettransaction(const JSONRPCRequest& request)
if(request.params[1].get_bool())
filter = filter | ISMINE_WATCH_ONLY;

std::string asset = "";
if (request.params[2].isStr() && !request.params[2].get_str().empty()) {
asset = request.params[2].get_str();
}

UniValue entry(UniValue::VOBJ);
auto it = pwallet->mapWallet.find(hash);
if (it == pwallet->mapWallet.end()) {
Expand All @@ -2004,7 +2010,7 @@ static UniValue gettransaction(const JSONRPCRequest& request)
nFee[::policyAsset] = wtx.IsFromMe(filter) ? total_out - nDebit[::policyAsset] : 0;
}

entry.pushKV("amount", AmountMapToUniv(nNet - nFee, ""));
entry.pushKV("amount", AmountMapToUniv(nNet - nFee, asset));
if (wtx.IsFromMe(filter))
entry.pushKV("fee", AmountMapToUniv(nFee, ""));

Expand Down Expand Up @@ -6545,12 +6551,12 @@ static const CRPCCommand commands[] =
{ "wallet", "encryptwallet", &encryptwallet, {"passphrase"} },
{ "wallet", "getaddressesbylabel", &getaddressesbylabel, {"label"} },
{ "wallet", "getaddressinfo", &getaddressinfo, {"address"} },
{ "wallet", "getbalance", &getbalance, {"dummy","minconf","include_watchonly"} },
{ "wallet", "getbalance", &getbalance, {"dummy","minconf","include_watchonly","assetlabel"} },
{ "wallet", "getnewaddress", &getnewaddress, {"label","address_type"} },
{ "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} },
{ "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} },
{ "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf"} },
{ "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} },
{ "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf","assetlabel"} },
{ "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf","assetlabel"} },
{ "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","assetlabel"} },
{ "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} },
{ "wallet", "getwalletinfo", &getwalletinfo, {} },
{ "wallet", "importaddress", &importaddress, {"address","label","rescan","p2sh"} },
Expand All @@ -6563,7 +6569,7 @@ static const CRPCCommand commands[] =
{ "wallet", "listaddressgroupings", &listaddressgroupings, {} },
{ "wallet", "listlabels", &listlabels, {"purpose"} },
{ "wallet", "listlockunspent", &listlockunspent, {} },
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} },
{ "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter","assetlabel"} },
{ "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} },
{ "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} },
{ "wallet", "listtransactions", &listtransactions, {"label|dummy","count","skip","include_watchonly"} },
Expand Down
6 changes: 6 additions & 0 deletions test/functional/feature_confidential_transactions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ def run_test(self):

# Check 2's listreceivedbyaddress
received_by_address = self.nodes[2].listreceivedbyaddress(0, False, False, "", "bitcoin")
validate_by_address = [(address2, value1 + value3), (address, value0 + value2)]
assert_equal(sorted([(ele['address'], ele['amount']) for ele in received_by_address], key=lambda t: t[0]),
sorted(validate_by_address, key = lambda t: t[0]))
received_by_address = self.nodes[2].listreceivedbyaddress(0, False, False, "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. You're inserting a line that overwrites an earlier variable and a line that sets a variable that gets overwritten two lines later. It seems like you need an extra block with an assert_equal in the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevenroose Thank you for your reviewing. I will address soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevenroose Added assert_equal (and rebased).
Tests fails but feature_confidential_transactions.py has not executed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevenroose satsfied?

validate_by_address = [(address2, {"bitcoin": value1 + value3}), (address, {"bitcoin": value0 + value2})]
assert_equal(sorted([(ele['address'], ele['amount']) for ele in received_by_address], key=lambda t: t[0]),
sorted(validate_by_address, key = lambda t: t[0]))
Expand Down Expand Up @@ -215,6 +219,7 @@ def run_test(self):
assert(found_unblinded)

assert_equal(self.nodes[1].gettransaction(raw_tx_id, True)['amount']["bitcoin"], value3)
assert_equal(self.nodes[1].gettransaction(raw_tx_id, True, "bitcoin")['amount'], value3)
list_unspent = self.nodes[1].listunspent(1, 9999999, [], True, {"asset": "bitcoin"})
assert_equal(list_unspent[0]['amount']+list_unspent[1]['amount'], value1+value3)
received_by_address = self.nodes[1].listreceivedbyaddress(1, False, True)
Expand Down Expand Up @@ -357,6 +362,7 @@ def run_test(self):
# Assets balance checking, note that accounts are completely ignored because
# balance queries with accounts are horrifically broken upstream
assert_equal(self.nodes[0].getbalance("*", 0, False, "bitcoin"), self.nodes[0].getbalance("*", 0, False, "bitcoin"))
assert_equal(self.nodes[0].getbalance("*", 0, False)['bitcoin'], self.nodes[0].getbalance("*", 0, False, "bitcoin"))
assert_equal(self.nodes[0].getwalletinfo()['balance']['bitcoin'], self.nodes[0].getbalance("*", 0, False, "bitcoin"))

# Send some bitcoin and other assets over as well to fund wallet
Expand Down
3 changes: 3 additions & 0 deletions test/functional/wallet_labels.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ def run_test(self):
for label in labels:
assert_equal(
node.getreceivedbyaddress(label.addresses[0])['bitcoin'], amount_to_send)
assert_equal(
node.getreceivedbyaddress(label.addresses[0], 1, "bitcoin"), amount_to_send)
assert_equal(node.getreceivedbylabel(label.name)['bitcoin'], amount_to_send)
assert_equal(node.getreceivedbylabel(label.name, 1, "bitcoin"), amount_to_send)

for i, label in enumerate(labels):
to_label = labels[(i + 1) % len(labels)]
Expand Down