Skip to content

Commit

Permalink
Auto merge of #4696 - str4d:3640-z_sendmany-any-taddr, r=str4d
Browse files Browse the repository at this point in the history
wallet: Add ANY_TADDR special string to z_sendmany

When using this special string as the from address, non-coinbase UTXOs
from any transparent addresses within the wallet will be used to fund the
transaction. Change outputs will be sent to a new transparent address,
as with any other spend of transparent funds.

Closes zcash/zcash#3640.
  • Loading branch information
zkbot committed Sep 22, 2020
2 parents 8dcab5e + 3311a85 commit c9672a4
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 31 deletions.
1 change: 1 addition & 0 deletions qa/pull-tester/rpc-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ testScripts=(
'wallet_changeaddresses.py'
'wallet_changeindicator.py'
'wallet_import_export.py'
'wallet_sendmany_any_taddr.py'
'wallet_shieldingcoinbase.py'
'wallet_shieldcoinbase_sprout.py'
'wallet_shieldcoinbase_sapling.py'
Expand Down
79 changes: 79 additions & 0 deletions qa/rpc-tests/wallet_sendmany_any_taddr.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#!/usr/bin/env python3
# Copyright (c) 2020 The Zcash developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://www.opensource.org/licenses/mit-license.php .

from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
wait_and_assert_operationid_status,
)

# Test ANY_TADDR special string in z_sendmany
class WalletSendManyAnyTaddr(BitcoinTestFramework):
def run_test(self):
# Sanity-check the test harness
assert_equal(self.nodes[0].getblockcount(), 200)

# Create the addresses we will be using.
recipient = self.nodes[1].z_getnewaddress()
node3zaddr = self.nodes[3].z_getnewaddress()
node3taddr1 = self.nodes[3].getnewaddress()
node3taddr2 = self.nodes[3].getnewaddress()

# We should not be able to spend multiple coinbase UTXOs at once.
wait_and_assert_operationid_status(
self.nodes[3],
self.nodes[3].z_sendmany('ANY_TADDR', [{'address': recipient, 'amount': 100}]),
'failed',
'Could not find any non-coinbase UTXOs to spend. Coinbase UTXOs can only be sent to a single zaddr recipient from a single taddr.',
)

# Prepare some non-coinbase UTXOs
wait_and_assert_operationid_status(
self.nodes[3],
self.nodes[3].z_shieldcoinbase("*", node3zaddr, 0)['opid'],
)
self.sync_all()
self.nodes[0].generate(1)
self.sync_all()
assert_equal(self.nodes[3].z_getbalance(node3zaddr), 250)

wait_and_assert_operationid_status(
self.nodes[3],
self.nodes[3].z_sendmany(
node3zaddr,
[
{'address': node3taddr1, 'amount': 60},
{'address': node3taddr2, 'amount': 75},
],
),
)
self.sync_all()
self.nodes[0].generate(1)
self.sync_all()

# Check the various balances.
assert_equal(self.nodes[1].z_getbalance(recipient), 0)
assert_equal(self.nodes[3].z_getbalance(node3taddr1), 60)
assert_equal(self.nodes[3].z_getbalance(node3taddr2), 75)

# We should be able to spend multiple UTXOs at once
wait_and_assert_operationid_status(
self.nodes[3],
self.nodes[3].z_sendmany('ANY_TADDR', [{'address': recipient, 'amount': 100}]),
)

self.sync_all()
self.nodes[0].generate(1)
self.sync_all()

# The recipient has their funds!
assert_equal(self.nodes[1].z_getbalance(recipient), 100)

# Change is sent to a new t-address.
assert_equal(self.nodes[3].z_getbalance(node3taddr1), 0)
assert_equal(self.nodes[3].z_getbalance(node3taddr2), 0)

if __name__ == '__main__':
WalletSendManyAnyTaddr().main()
21 changes: 11 additions & 10 deletions src/wallet/asyncrpcoperation_sendmany.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(

KeyIO keyIO(Params());

useanyutxo_ = fromAddress == "ANY_TADDR";
fromtaddr_ = keyIO.DecodeDestination(fromAddress);
isfromtaddr_ = IsValidDestination(fromtaddr_);
isfromtaddr_ = useanyutxo_ || IsValidDestination(fromtaddr_);
isfromzaddr_ = false;

if (!isfromtaddr_) {
Expand Down Expand Up @@ -213,7 +214,8 @@ bool AsyncRPCOperation_sendmany::main_impl() {
// When spending coinbase utxos, you can only specify a single zaddr as the change must go somewhere
// and if there are multiple zaddrs, we don't know where to send it.
if (isfromtaddr_) {
if (isSingleZaddrOutput) {
// Only select coinbase if we are spending from a single t-address to a single z-address.
if (!useanyutxo_ && isSingleZaddrOutput) {
bool b = find_utxos(true);
if (!b) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds, no UTXOs found for taddr from address.");
Expand All @@ -222,7 +224,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
bool b = find_utxos(false);
if (!b) {
if (isMultipleZaddrOutput) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any non-coinbase UTXOs to spend. Coinbase UTXOs can only be sent to a single zaddr recipient.");
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any non-coinbase UTXOs to spend. Coinbase UTXOs can only be sent to a single zaddr recipient from a single taddr.");
} else {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Could not find any non-coinbase UTXOs to spend.");
}
Expand Down Expand Up @@ -319,12 +321,8 @@ bool AsyncRPCOperation_sendmany::main_impl() {

// update the transaction with these inputs
if (isUsingBuilder_) {
CScript scriptPubKey = GetScriptForDestination(fromtaddr_);
for (auto t : t_inputs_) {
uint256 txid = t.txid;
int vout = t.vout;
CAmount amount = t.amount;
builder_.AddTransparentInput(COutPoint(txid, vout), scriptPubKey, amount);
builder_.AddTransparentInput(COutPoint(t.txid, t.vout), t.scriptPubKey, t.amount);
}
} else {
CMutableTransaction rawTx(tx_);
Expand Down Expand Up @@ -895,7 +893,9 @@ bool AsyncRPCOperation_sendmany::main_impl() {

bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
std::set<CTxDestination> destinations;
destinations.insert(fromtaddr_);
if (!useanyutxo_) {
destinations.insert(fromtaddr_);
}
vector<COutput> vecOutputs;

LOCK2(cs_main, pwalletMain->cs_wallet);
Expand Down Expand Up @@ -928,8 +928,9 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
continue;
}

CScript scriptPubKey = out.tx->vout[out.i].scriptPubKey;
CAmount nValue = out.tx->vout[out.i].nValue;
SendManyInputUTXO utxo(out.tx->GetHash(), out.i, nValue, isCoinbase);
SendManyInputUTXO utxo(out.tx->GetHash(), out.i, scriptPubKey, nValue, isCoinbase);
t_inputs_.push_back(utxo);
}

Expand Down
6 changes: 4 additions & 2 deletions src/wallet/asyncrpcoperation_sendmany.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ class SendManyInputUTXO {
public:
uint256 txid;
int vout;
CScript scriptPubKey;
CAmount amount;
bool coinbase;

SendManyInputUTXO(uint256 txid_, int vout_, CAmount amount_, bool coinbase_) :
txid(txid_), vout(vout_), amount(amount_), coinbase(coinbase_) {}
SendManyInputUTXO(uint256 txid_, int vout_, CScript scriptPubKey_, CAmount amount_, bool coinbase_) :
txid(txid_), vout(vout_), scriptPubKey(scriptPubKey_), amount(amount_), coinbase(coinbase_) {}
};

class SendManyInputJSOP {
Expand Down Expand Up @@ -111,6 +112,7 @@ class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
CAmount fee_;
int mindepth_;
std::string fromaddress_;
bool useanyutxo_;
bool isfromtaddr_;
bool isfromzaddr_;
CTxDestination fromtaddr_;
Expand Down
48 changes: 29 additions & 19 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4022,12 +4022,17 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
throw runtime_error(
"z_sendmany \"fromaddress\" [{\"address\":... ,\"amount\":...},...] ( minconf ) ( fee )\n"
"\nSend multiple times. Amounts are decimal numbers with at most 8 digits of precision."
"\nChange generated from a taddr flows to a new taddr address, while change generated from a zaddr returns to itself."
"\nWhen sending coinbase UTXOs to a zaddr, change is not allowed. The entire value of the UTXO(s) must be consumed."
"\nChange generated from one or more transparent addresses flows to a new transparent"
"\naddress, while change generated from a shielded address returns to itself."
"\nWhen sending coinbase UTXOs to a shielded address, change is not allowed."
"\nThe entire value of the UTXO(s) must be consumed."
+ strprintf("\nBefore Sapling activates, the maximum number of zaddr outputs is %d due to transaction size limits.\n", Z_SENDMANY_MAX_ZADDR_OUTPUTS_BEFORE_SAPLING)
+ HelpRequiringPassphrase() + "\n"
"\nArguments:\n"
"1. \"fromaddress\" (string, required) The taddr or zaddr to send the funds from.\n"
"1. \"fromaddress\" (string, required) The transparent or shielded address to send the funds from.\n"
" The following special strings are also accepted:\n"
" - \"ANY_TADDR\": Select non-coinbase UTXOs from any transparent addresses belonging to the wallet.\n"
" Use z_shieldcoinbase to shield coinbase UTXOs from multiple transparent addresses.\n"
"2. \"amounts\" (array, required) An array of json objects representing the amounts to send.\n"
" [{\n"
" \"address\":address (string, required) The address is a taddr or zaddr\n"
Expand All @@ -4040,8 +4045,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
"\nResult:\n"
"\"operationid\" (string) An operationid to pass to z_getoperationstatus to get the result of the operation.\n"
"\nExamples:\n"
+ HelpExampleCli("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" '[{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\" ,\"amount\": 5.0}]'")
+ HelpExampleRpc("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", [{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\" ,\"amount\": 5.0}]")
+ HelpExampleCli("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" '[{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\", \"amount\": 5.0}]'")
+ HelpExampleCli("z_sendmany", "\"ANY_TADDR\" '[{\"address\": \"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", \"amount\": 2.0}]'")
+ HelpExampleRpc("z_sendmany", "\"t1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", [{\"address\": \"ztfaW34Gj9FrnGUEf833ywDVL62NWXBM81u6EQnM6VR45eYnXhwztecW1SjxA7JrmAXKJhxhj3vDNEpVCQoSvVoSpmbhtjf\", \"amount\": 5.0}]")
);

LOCK2(cs_main, pwalletMain->cs_wallet);
Expand All @@ -4053,22 +4059,26 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
bool fromTaddr = false;
bool fromSapling = false;
KeyIO keyIO(Params());
CTxDestination taddr = keyIO.DecodeDestination(fromaddress);
fromTaddr = IsValidDestination(taddr);
if (!fromTaddr) {
auto res = keyIO.DecodePaymentAddress(fromaddress);
if (!IsValidPaymentAddress(res)) {
// invalid
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr.");
}
if (fromaddress == "ANY_TADDR") {
fromTaddr = true;
} else {
CTxDestination taddr = keyIO.DecodeDestination(fromaddress);
fromTaddr = IsValidDestination(taddr);
if (!fromTaddr) {
auto res = keyIO.DecodePaymentAddress(fromaddress);
if (!IsValidPaymentAddress(res)) {
// invalid
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr.");
}

// Check that we have the spending key
if (!boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), res)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key not found.");
}
// Check that we have the spending key
if (!boost::apply_visitor(HaveSpendingKeyForPaymentAddress(pwalletMain), res)) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key not found.");
}

// Remember whether this is a Sprout or Sapling address
fromSapling = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
// Remember whether this is a Sprout or Sapling address
fromSapling = boost::get<libzcash::SaplingPaymentAddress>(&res) != nullptr;
}
}
// This logic will need to be updated if we add a new shielded pool
bool fromSprout = !(fromTaddr || fromSapling);
Expand Down

0 comments on commit c9672a4

Please sign in to comment.