From ffa41a28ade7d4df20e0d243cd65c23564e07b0b Mon Sep 17 00:00:00 2001 From: Karol Rychlicki Date: Sun, 21 May 2017 12:19:17 +0200 Subject: [PATCH 1/4] dont waste keys from keypool on failure in CreateDenominated --- src/privatesend-client.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index 6b47ce9c9eae..359268507c6c 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -13,6 +13,7 @@ #include "txmempool.h" #include "util.h" #include "utilmoneystr.h" +#include CPrivateSendClient privateSendClient; @@ -1241,7 +1242,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo // ****** Add denoms ************ / // make our denom addresses - CReserveKey reservekeyDenom(pwalletMain); + std::vector> reservekeyDenomVec; // try few times - skipping smallest denoms first if there are too much already, if failed - use them int nOutputsTotal = 0; @@ -1273,10 +1274,11 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo CScript scriptDenom; CPubKey vchPubKey; //use a unique change address - assert(reservekeyDenom.GetReservedKey(vchPubKey)); // should never fail, as we just unlocked + std::shared_ptr reservekeyDenom = std::make_shared(pwalletMain); + reservekeyDenomVec.push_back(reservekeyDenom); + + assert(reservekeyDenom->GetReservedKey(vchPubKey)); // should never fail, as we just unlocked scriptDenom = GetScriptForDestination(vchPubKey.GetID()); - // TODO: do not keep reservekeyDenom here - reservekeyDenom.KeepKey(); vecSend.push_back((CRecipient){ scriptDenom, nDenomValue, false }); @@ -1316,13 +1318,17 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo nFeeRet, nChangePosRet, strFail, &coinControl, true, ONLY_NONDENOMINATED_NOT1000IFMN); if(!fSuccess) { LogPrintf("CPrivateSendClient::CreateDenominated -- Error: %s\n", strFail); - // TODO: return reservekeyDenom here + for(auto key : reservekeyDenomVec) + key->ReturnKey(); reservekeyCollateral.ReturnKey(); + LogPrintf("CPrivateSendClient::CreateDenominated -- %d keys returned\n", reservekeyDenomVec.size() + 1); return false; } - // TODO: keep reservekeyDenom here + for(auto key : reservekeyDenomVec) + key->KeepKey(); reservekeyCollateral.KeepKey(); + LogPrintf("CPrivateSendClient::CreateDenominated -- %d keys keeped\n", reservekeyDenomVec.size() + 1); if(!pwalletMain->CommitTransaction(wtx, reservekeyChange)) { LogPrintf("CPrivateSendClient::CreateDenominated -- CommitTransaction failed!\n"); From 4b2ebd4420633597700905f97167a1037df4f296 Mon Sep 17 00:00:00 2001 From: Karol Rychlicki Date: Sun, 21 May 2017 12:24:48 +0200 Subject: [PATCH 2/4] bug fix - log actual number of total outputs, comment error --- src/privatesend-client.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index 359268507c6c..28dff2475cd2 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1269,7 +1269,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo int nOutputs = 0; - // add each output up to 10 times until it can't be added again + // add each output up to 11 times until it can't be added again while(nValueLeft - nDenomValue >= 0 && nOutputs <= 10) { CScript scriptDenom; CPubKey vchPubKey; @@ -1285,7 +1285,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo //increment outputs and subtract denomination amount nOutputs++; nValueLeft -= nDenomValue; - LogPrintf("CreateDenominated1: nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal, nOutputs, (float)nValueLeft/COIN); + LogPrintf("CreateDenominated1: nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal + nOutputs, nOutputs, (float)nValueLeft/COIN); } nOutputsTotal += nOutputs; From a74cd5081a3e9f8418c7c559458a5fb1dc17cbc1 Mon Sep 17 00:00:00 2001 From: Karol Rychlicki Date: Mon, 22 May 2017 22:16:39 +0200 Subject: [PATCH 3/4] log number of total outputs as separate value --- src/privatesend-client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index 28dff2475cd2..d6c904cfbf73 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1285,7 +1285,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo //increment outputs and subtract denomination amount nOutputs++; nValueLeft -= nDenomValue; - LogPrintf("CreateDenominated1: nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal + nOutputs, nOutputs, (float)nValueLeft/COIN); + LogPrintf("CreateDenominated1: totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float)nValueLeft/COIN); } nOutputsTotal += nOutputs; From f09c0e88501a7e0ac40b6fa79fc3fb63b1cd41cc Mon Sep 17 00:00:00 2001 From: Karol Rychlicki Date: Fri, 26 May 2017 20:41:04 +0200 Subject: [PATCH 4/4] add lock so no one can spend outputs used for denominations --- src/privatesend-client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/privatesend-client.cpp b/src/privatesend-client.cpp index d6c904cfbf73..46ae26056abf 100644 --- a/src/privatesend-client.cpp +++ b/src/privatesend-client.cpp @@ -1199,6 +1199,8 @@ bool CPrivateSendClient::MakeCollateralAmounts(const CompactTallyItem& tallyItem // Create denominations by looping through inputs grouped by addresses bool CPrivateSendClient::CreateDenominated() { + LOCK2(cs_main, pwalletMain->cs_wallet); + std::vector vecTally; if(!pwalletMain->SelectCoinsGrouppedByAddresses(vecTally)) { LogPrint("privatesend", "CPrivateSendClient::CreateDenominated -- SelectCoinsGrouppedByAddresses can't find any inputs!\n");