Skip to content

Commit

Permalink
Closes zcash#1854. z_sendmany selects more utxos to avoid dust change…
Browse files Browse the repository at this point in the history
… output.
  • Loading branch information
bitcartel committed Nov 16, 2016
1 parent 6661d9d commit 0afd368
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 1 deletion.
9 changes: 9 additions & 0 deletions qa/rpc-tests/wallet_protectcoinbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ def run_test (self):
assert_equal(Decimal(resp["private"]), Decimal('9.9998'))
assert_equal(Decimal(resp["total"]), Decimal('39.9998'))

# z_sendmany will return an error if there is transparent change output considered dust.
# UTXO selection in z_sendmany sorts in ascending order, so smallest utxos are consumed first.
# At this point in time, unspent notes all have a value of 10.0 and standard z_sendmany fee is 0.0001.
recipients = []
amount = Decimal('10.0') - Decimal('0.00010000') - Decimal('0.00000001') # this leaves change at 1 zatoshi less than dust threshold
recipients.append({"address":self.nodes[0].getnewaddress(), "amount":amount })
myopid = self.nodes[0].z_sendmany(mytaddr, recipients)
self.wait_and_assert_operationid_status(myopid, "failed", "Insufficient transparent funds, have 10.00, need 0.00000545 more to avoid creating invalid change output 0.00000001 (dust threshold is 0.00000546)")

# Send will fail because send amount is too big, even when including coinbase utxos
errorString = ""
try:
Expand Down
27 changes: 26 additions & 1 deletion src/wallet/asyncrpcoperation_sendmany.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,14 @@ bool AsyncRPCOperation_sendmany::main_impl() {
CAmount selectedUTXOAmount = 0;
bool selectedUTXOCoinbase = false;
if (isfromtaddr_) {
// Get dust threshold
CKey secret;
secret.MakeNewKey(true);
CScript scriptPubKey = GetScriptForDestination(secret.GetPubKey().GetID());
CTxOut out(CAmount(1), scriptPubKey);
CAmount dustThreshold = out.GetDustThreshold(minRelayTxFee);
CAmount dustChange = -1;

std::vector<SendManyInputUTXO> selectedTInputs;
for (SendManyInputUTXO & t : t_inputs_) {
bool b = std::get<3>(t);
Expand All @@ -224,9 +232,21 @@ bool AsyncRPCOperation_sendmany::main_impl() {
selectedUTXOAmount += std::get<2>(t);
selectedTInputs.push_back(t);
if (selectedUTXOAmount >= targetAmount) {
break;
// Select another utxo if there is change less than the dust threshold.
dustChange = selectedUTXOAmount - targetAmount;
if (dustChange == 0 || dustChange >= dustThreshold) {
break;
}
}
}

// If there is transparent change, is it valid or is it dust?
if (dustChange < dustThreshold && dustChange != 0) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS,
strprintf("Insufficient transparent funds, have %s, need %s more to avoid creating invalid change output %s (dust threshold is %s)",
FormatMoney(t_inputs_total), FormatMoney(dustThreshold - dustChange), FormatMoney(dustChange), FormatMoney(dustThreshold)));
}

t_inputs_ = selectedTInputs;
t_inputs_total = selectedUTXOAmount;

Expand Down Expand Up @@ -772,6 +792,11 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
t_inputs_.push_back(utxo);
}

// sort in ascending order, so smaller utxos appear first
std::sort(t_inputs_.begin(), t_inputs_.end(), [](SendManyInputUTXO i, SendManyInputUTXO j) -> bool {
return ( std::get<2>(i) < std::get<2>(j));
});

return t_inputs_.size() > 0;
}

Expand Down

0 comments on commit 0afd368

Please sign in to comment.