PrivateSend: dont waste keys from keypool on failure in CreateDenominated (#1473)

* dont waste keys from keypool on failure in CreateDenominated

* bug fix - log actual number of total outputs, comment error

* log number of total outputs as separate value

* add lock so no one can spend outputs used for denominations
This commit is contained in:
krychlicki 2017-05-28 15:50:07 +02:00 committed by UdjinM6
parent 18c83f58e3
commit 68e858f8dd

View File

@ -13,6 +13,7 @@
#include "txmempool.h" #include "txmempool.h"
#include "util.h" #include "util.h"
#include "utilmoneystr.h" #include "utilmoneystr.h"
#include <memory>
CPrivateSendClient privateSendClient; CPrivateSendClient privateSendClient;
@ -1198,6 +1199,8 @@ bool CPrivateSendClient::MakeCollateralAmounts(const CompactTallyItem& tallyItem
// Create denominations by looping through inputs grouped by addresses // Create denominations by looping through inputs grouped by addresses
bool CPrivateSendClient::CreateDenominated() bool CPrivateSendClient::CreateDenominated()
{ {
LOCK2(cs_main, pwalletMain->cs_wallet);
std::vector<CompactTallyItem> vecTally; std::vector<CompactTallyItem> vecTally;
if(!pwalletMain->SelectCoinsGrouppedByAddresses(vecTally)) { if(!pwalletMain->SelectCoinsGrouppedByAddresses(vecTally)) {
LogPrint("privatesend", "CPrivateSendClient::CreateDenominated -- SelectCoinsGrouppedByAddresses can't find any inputs!\n"); LogPrint("privatesend", "CPrivateSendClient::CreateDenominated -- SelectCoinsGrouppedByAddresses can't find any inputs!\n");
@ -1241,7 +1244,7 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo
// ****** Add denoms ************ / // ****** Add denoms ************ /
// make our denom addresses // make our denom addresses
CReserveKey reservekeyDenom(pwalletMain); std::vector<std::shared_ptr<CReserveKey>> reservekeyDenomVec;
// try few times - skipping smallest denoms first if there are too much already, if failed - use them // try few times - skipping smallest denoms first if there are too much already, if failed - use them
int nOutputsTotal = 0; int nOutputsTotal = 0;
@ -1268,22 +1271,23 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo
int nOutputs = 0; 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) { while(nValueLeft - nDenomValue >= 0 && nOutputs <= 10) {
CScript scriptDenom; CScript scriptDenom;
CPubKey vchPubKey; CPubKey vchPubKey;
//use a unique change address //use a unique change address
assert(reservekeyDenom.GetReservedKey(vchPubKey)); // should never fail, as we just unlocked std::shared_ptr<CReserveKey> reservekeyDenom = std::make_shared<CReserveKey>(pwalletMain);
reservekeyDenomVec.push_back(reservekeyDenom);
assert(reservekeyDenom->GetReservedKey(vchPubKey)); // should never fail, as we just unlocked
scriptDenom = GetScriptForDestination(vchPubKey.GetID()); scriptDenom = GetScriptForDestination(vchPubKey.GetID());
// TODO: do not keep reservekeyDenom here
reservekeyDenom.KeepKey();
vecSend.push_back((CRecipient){ scriptDenom, nDenomValue, false }); vecSend.push_back((CRecipient){ scriptDenom, nDenomValue, false });
//increment outputs and subtract denomination amount //increment outputs and subtract denomination amount
nOutputs++; nOutputs++;
nValueLeft -= nDenomValue; nValueLeft -= nDenomValue;
LogPrintf("CreateDenominated1: nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal, nOutputs, (float)nValueLeft/COIN); LogPrintf("CreateDenominated1: totalOutputs: %d, nOutputsTotal: %d, nOutputs: %d, nValueLeft: %f\n", nOutputsTotal + nOutputs, nOutputsTotal, nOutputs, (float)nValueLeft/COIN);
} }
nOutputsTotal += nOutputs; nOutputsTotal += nOutputs;
@ -1316,13 +1320,17 @@ bool CPrivateSendClient::CreateDenominated(const CompactTallyItem& tallyItem, bo
nFeeRet, nChangePosRet, strFail, &coinControl, true, ONLY_NONDENOMINATED_NOT1000IFMN); nFeeRet, nChangePosRet, strFail, &coinControl, true, ONLY_NONDENOMINATED_NOT1000IFMN);
if(!fSuccess) { if(!fSuccess) {
LogPrintf("CPrivateSendClient::CreateDenominated -- Error: %s\n", strFail); LogPrintf("CPrivateSendClient::CreateDenominated -- Error: %s\n", strFail);
// TODO: return reservekeyDenom here for(auto key : reservekeyDenomVec)
key->ReturnKey();
reservekeyCollateral.ReturnKey(); reservekeyCollateral.ReturnKey();
LogPrintf("CPrivateSendClient::CreateDenominated -- %d keys returned\n", reservekeyDenomVec.size() + 1);
return false; return false;
} }
// TODO: keep reservekeyDenom here for(auto key : reservekeyDenomVec)
key->KeepKey();
reservekeyCollateral.KeepKey(); reservekeyCollateral.KeepKey();
LogPrintf("CPrivateSendClient::CreateDenominated -- %d keys keeped\n", reservekeyDenomVec.size() + 1);
if(!pwalletMain->CommitTransaction(wtx, reservekeyChange)) { if(!pwalletMain->CommitTransaction(wtx, reservekeyChange)) {
LogPrintf("CPrivateSendClient::CreateDenominated -- CommitTransaction failed!\n"); LogPrintf("CPrivateSendClient::CreateDenominated -- CommitTransaction failed!\n");