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.

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

Add transaction size and zaddr output limit checks to z_sendmany #1808

Merged
merged 1 commit into from
Nov 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 46 additions & 0 deletions qa/rpc-tests/wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,52 @@ def run_test (self):
myvjoinsplits = mytxdetails["vjoinsplit"]
assert_equal(0, len(myvjoinsplits))

# z_sendmany is expected to fail if tx size breaks limit
myzaddr = self.nodes[0].z_getnewaddress()

recipients = []
num_t_recipients = 3000
amount_per_recipient = Decimal('0.00000001')
errorString = ''
for i in xrange(0,num_t_recipients):
newtaddr = self.nodes[2].getnewaddress()
recipients.append({"address":newtaddr, "amount":amount_per_recipient})
try:
self.nodes[0].z_sendmany(myzaddr, recipients)
except JSONRPCException,e:
errorString = e.error['message']
assert("Too many outputs, size of raw transaction" in errorString)

recipients = []
num_t_recipients = 2000
num_z_recipients = 50
amount_per_recipient = Decimal('0.00000001')
errorString = ''
for i in xrange(0,num_t_recipients):
newtaddr = self.nodes[2].getnewaddress()
recipients.append({"address":newtaddr, "amount":amount_per_recipient})
for i in xrange(0,num_z_recipients):
newzaddr = self.nodes[2].z_getnewaddress()
recipients.append({"address":newzaddr, "amount":amount_per_recipient})
try:
self.nodes[0].z_sendmany(myzaddr, recipients)
except JSONRPCException,e:
errorString = e.error['message']
assert("size of raw transaction would be larger than limit" in errorString)

recipients = []
num_z_recipients = 100
amount_per_recipient = Decimal('0.00000001')
errorString = ''
for i in xrange(0,num_z_recipients):
newzaddr = self.nodes[2].z_getnewaddress()
recipients.append({"address":newzaddr, "amount":amount_per_recipient})
try:
self.nodes[0].z_sendmany(myzaddr, recipients)
except JSONRPCException,e:
errorString = e.error['message']
assert("Invalid parameter, too many zaddr outputs" in errorString)

# add zaddr to node 2
myzaddr = self.nodes[2].z_getnewaddress()

Expand Down
36 changes: 36 additions & 0 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3166,6 +3166,17 @@ Value z_getoperationstatus_IMPL(const Array& params, bool fRemoveFinishedOperati
return ret;
}


// Here we define the maximum number of zaddr outputs that can be included in a transaction.
// If input notes are small, we might actually require more than one joinsplit per zaddr output.
// For now though, we assume we use one joinsplit per zaddr output (and the second output note is change).
// We reduce the result by 1 to ensure there is room for non-joinsplit CTransaction data.
#define Z_SENDMANY_MAX_ZADDR_OUTPUTS ((MAX_TX_SIZE / JSDescription().GetSerializeSize(SER_NETWORK, PROTOCOL_VERSION)) - 1)

// transaction.h comment: spending taddr output requires CTxIn >= 148 bytes and typical taddr txout is 34 bytes
#define CTXIN_SPEND_DUST_SIZE 148
#define CTXOUT_REGULAR_SIZE 34

Value z_sendmany(const Array& params, bool fHelp)
{
if (!EnsureWalletIsAvailable(fHelp))
Expand All @@ -3177,6 +3188,7 @@ Value z_sendmany(const Array& params, bool fHelp)
"\nSend multiple times. Amounts are double-precision floating point numbers."
"\nChange from a taddr flows to a new taddr address, while change from zaddr returns to itself."
"\nWhen sending coinbase UTXOs to a zaddr, change is not alllowed. The entire value of the UTXO(s) must be consumed."
+ strprintf("\nCurrently, the maximum number of zaddr outputs is %d due to transaction size limits.\n", Z_SENDMANY_MAX_ZADDR_OUTPUTS)
+ HelpRequiringPassphrase() + "\n"
"\nArguments:\n"
"1. \"fromaddress\" (string, required) The taddr or zaddr to send the funds from.\n"
Expand Down Expand Up @@ -3284,6 +3296,30 @@ Value z_sendmany(const Array& params, bool fHelp)
}
}

// Check the number of zaddr outputs does not exceed the limit.
if (zaddrRecipients.size() > Z_SENDMANY_MAX_ZADDR_OUTPUTS) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if a change address takes this over the limit? Are there other ways the underlying API might add data to the transaction that wouldn't be accounted for in these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added txout change, which is likely, when sending from taddr.

throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, too many zaddr outputs");
}

// As a sanity check, estimate and verify that the size of the transaction will be valid.
// Depending on the input notes, the actual tx size may turn out to be larger and perhaps invalid.
size_t txsize = 0;
CMutableTransaction mtx;
mtx.nVersion = 2;
for (int i = 0; i < zaddrRecipients.size(); i++) {
mtx.vjoinsplit.push_back(JSDescription());
}
CTransaction tx(mtx);
txsize += tx.GetSerializeSize(SER_NETWORK, tx.nVersion);
if (fromTaddr) {
txsize += CTXIN_SPEND_DUST_SIZE;
txsize += CTXOUT_REGULAR_SIZE; // There will probably be taddr change
}
txsize += CTXOUT_REGULAR_SIZE * taddrRecipients.size();
if (txsize > MAX_TX_SIZE) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Too many outputs, size of raw transaction would be larger than limit of %d bytes", MAX_TX_SIZE ));
}

// Minimum confirmations
int nMinDepth = 1;
if (params.size() > 2) {
Expand Down