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

Jump to bottom

Fix intermediate vpub_new leakage in multi joinsplit tx #2440

Merged
merged 1 commit into from
Jun 16, 2017
Merged

Fix intermediate vpub_new leakage in multi joinsplit tx #2440

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
154 changes: 47 additions & 107 deletions src/wallet/asyncrpcoperation_sendmany.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,14 @@ bool AsyncRPCOperation_sendmany::main_impl() {
tx_ = CTransaction(mtx);

// Copy zinputs and zoutputs to more flexible containers
std::deque<SendManyInputJSOP> zInputsDeque;
std::deque<SendManyInputJSOP> zInputsDeque; // zInputsDeque stores minimum numbers of notes for target amount
CAmount tmp = 0;
for (auto o : z_inputs_) {
zInputsDeque.push_back(o);
tmp += std::get<2>(o);
if (tmp >= targetAmount) {
break;
}
}
std::deque<SendManyRecipient> zOutputsDeque;
for (auto o : z_outputs_) {
Expand Down Expand Up @@ -442,103 +447,25 @@ bool AsyncRPCOperation_sendmany::main_impl() {
* zaddr -> taddrs
* -> zaddrs
*
* Processing order:
* Part 1: taddrs and miners fee
* Part 2: zaddrs
*/

/**
* SCENARIO #3
* Part 1: Add to the transparent value pool.
* Send to zaddrs by chaining JoinSplits together and immediately consuming any change
* Send to taddrs by creating dummy z outputs and accumulating value in a change note
* which is used to set vpub_new in the last chained joinsplit.
*/
UniValue obj(UniValue::VOBJ);
CAmount jsChange = 0; // this is updated after each joinsplit
int changeOutputIndex = -1; // this is updated after each joinsplit if jsChange > 0
bool minersFeeProcessed = false;

bool vpubNewProcessed = false; // updated when vpub_new for miner fee and taddr outputs is set in last joinsplit
CAmount vpubNewTarget = minersFee;
if (t_outputs_total > 0) {
add_taddr_outputs_to_tx();
CAmount taddrTargetAmount = t_outputs_total + minersFee;
minersFeeProcessed = true;
while (zInputsDeque.size() > 0 && taddrTargetAmount > 0) {
AsyncJoinSplitInfo info;
info.vpub_old = 0;
info.vpub_new = 0;
std::vector<JSOutPoint> outPoints;
int n = 0;
while (n++ < ZC_NUM_JS_INPUTS && taddrTargetAmount > 0) {
SendManyInputJSOP o = zInputsDeque.front();
JSOutPoint outPoint = std::get<0>(o);
Note note = std::get<1>(o);
CAmount noteFunds = std::get<2>(o);
zInputsDeque.pop_front();

info.notes.push_back(note);
outPoints.push_back(outPoint);

int wtxHeight = -1;
int wtxDepth = -1;
{
LOCK2(cs_main, pwalletMain->cs_wallet);
const CWalletTx& wtx = pwalletMain->mapWallet[outPoint.hash];
wtxHeight = mapBlockIndex[wtx.hashBlock]->nHeight;
wtxDepth = wtx.GetDepthInMainChain();
vpubNewTarget += t_outputs_total;
}
LogPrint("zrpcunsafe", "%s: spending note (txid=%s, vjoinsplit=%d, ciphertext=%d, amount=%s, height=%d, confirmations=%d)\n",
getId(),
outPoint.hash.ToString().substr(0, 10),
outPoint.js,
int(outPoint.n), // uint8_t
FormatMoney(noteFunds),
wtxHeight,
wtxDepth
);


// Put value back into the value pool
if (noteFunds >= taddrTargetAmount) {
jsChange = noteFunds - taddrTargetAmount;
info.vpub_new += taddrTargetAmount;
} else {
info.vpub_new += noteFunds;
}

taddrTargetAmount -= noteFunds;
if (taddrTargetAmount <= 0) {
break;
}
}

if (jsChange > 0) {
info.vjsout.push_back(JSOutput());
info.vjsout.push_back(JSOutput(frompaymentaddress_, jsChange));

LogPrint("zrpcunsafe", "%s: generating note for change (amount=%s)\n",
getId(),
FormatMoney(jsChange)
);
}

obj = perform_joinsplit(info, outPoints);

if (jsChange > 0) {
changeOutputIndex = find_output(obj, 1);
}
}
}


/**
* SCENARIO #3
* Part 2: Send to zaddrs by chaining JoinSplits together and immediately consuming any change
*/
if (z_outputs_total>0) {

// Keep track of treestate within this transaction
boost::unordered_map<uint256, ZCIncrementalMerkleTree, CCoinsKeyHasher> intermediates;
std::vector<uint256> previousCommitments;

while (zOutputsDeque.size() > 0) {
while (!vpubNewProcessed) {
AsyncJoinSplitInfo info;
info.vpub_old = 0;
info.vpub_new = 0;
Expand Down Expand Up @@ -699,26 +626,37 @@ bool AsyncRPCOperation_sendmany::main_impl() {
std::copy(vInputNotes.begin(), vInputNotes.end(), std::back_inserter(info.notes));
}


//
// Find recipient to transfer funds to
//
std::string address, hexMemo;
CAmount value = 0;
if (zOutputsDeque.size() > 0) {
SendManyRecipient smr = zOutputsDeque.front();
std::string address = std::get<0>(smr);
CAmount value = std::get<1>(smr);
std::string hexMemo = std::get<2>(smr);
address = std::get<0>(smr);
value = std::get<1>(smr);
hexMemo = std::get<2>(smr);
zOutputsDeque.pop_front();
}

// Will we have any change? Has the miners fee been processed yet?
// Reset change
jsChange = 0;
CAmount outAmount = value;
if (!minersFeeProcessed) {
if (jsInputValue < minersFee) {
throw JSONRPCError(RPC_WALLET_ERROR, "Not enough funds to pay miners fee");
}
outAmount += minersFee;
}

// Set vpub_new in the last joinsplit (when there are no more notes to spend or zaddr outputs to satisfy)
if (zOutputsDeque.size() == 0 && zInputsDeque.size() == 0) {
assert(!vpubNewProcessed);
if (jsInputValue < vpubNewTarget) {
throw JSONRPCError(RPC_WALLET_ERROR,
strprintf("Insufficient funds for vpub_new %s (miners fee %s, taddr outputs %s)",
FormatMoney(vpubNewTarget), FormatMoney(minersFee), FormatMoney(t_outputs_total)));
}
outAmount += vpubNewTarget;
info.vpub_new += vpubNewTarget; // funds flowing back to public pool
vpubNewProcessed = true;
jsChange = jsInputValue - outAmount;
assert(jsChange >= 0);
}
else {
// This is not the last joinsplit, so compute change and any amount still due to the recipient
if (jsInputValue > outAmount) {
jsChange = jsInputValue - outAmount;
} else if (outAmount > jsInputValue) {
Expand All @@ -729,23 +667,21 @@ bool AsyncRPCOperation_sendmany::main_impl() {

// reduce the amount being sent right now to the value of all inputs
value = jsInputValue;
if (!minersFeeProcessed) {
value -= minersFee;
}
}

if (!minersFeeProcessed) {
minersFeeProcessed = true;
info.vpub_new += minersFee; // funds flowing back to public pool
}

// create output for recipient
if (address.empty()) {
assert(value==0);
info.vjsout.push_back(JSOutput()); // dummy output while we accumulate funds into a change note for vpub_new
} else {
PaymentAddress pa = CZCPaymentAddress(address).Get();
JSOutput jso = JSOutput(pa, value);
if (hexMemo.size() > 0) {
jso.memo = get_memo_from_hex_string(hexMemo);
}
info.vjsout.push_back(jso);
}

// create output for any change
if (jsChange>0) {
Expand All @@ -763,7 +699,11 @@ bool AsyncRPCOperation_sendmany::main_impl() {
changeOutputIndex = find_output(obj, 1);
}
}
}

// Sanity check in case changes to code block above exits loop by invoking 'break'
assert(zInputsDeque.size() == 0);
assert(zOutputsDeque.size() == 0);
assert(vpubNewProcessed);

sign_send_raw_transaction(obj);
return true;
Expand Down