Skip to content

Commit

Permalink
Don't use pointers for CReserveKeys
Browse files Browse the repository at this point in the history
  • Loading branch information
@instagibbs
instagibbs committed Aug 8, 2017
1 parent 209df50 commit f400def
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 32 deletions.
24 changes: 8 additions & 16 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,14 +406,11 @@ static void SendMoney(const CScript& scriptPubKey, CAmount nValue, CAsset asset,
throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");

// Create and send the transaction
std::vector<CReserveKey*> vpChangeKey;
std::vector<CReserveKey> vChangeKey;
vChangeKey.reserve(2);
vChangeKey.emplace_back(pwalletMain);
vpChangeKey.push_back(&vChangeKey[0]);
if (policyAsset != asset) {
vChangeKey.emplace_back(pwalletMain);
vpChangeKey.push_back(&vChangeKey[1]);
}

CAmount nFeeRequired;
Expand All @@ -422,13 +419,13 @@ static void SendMoney(const CScript& scriptPubKey, CAmount nValue, CAsset asset,
int nChangePosRet = -1;
CRecipient recipient = {scriptPubKey, nValue, asset, confidentiality_key, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
if (!pwalletMain->CreateTransaction(vecSend, wtxNew, vpChangeKey, nFeeRequired, nChangePosRet, strError, NULL, true, NULL, true, NULL, NULL, NULL, fIgnoreBlindFail)) {
if (!pwalletMain->CreateTransaction(vecSend, wtxNew, vChangeKey, nFeeRequired, nChangePosRet, strError, NULL, true, NULL, true, NULL, NULL, NULL, fIgnoreBlindFail)) {
if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance)
strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired));
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
CValidationState state;
if (!pwalletMain->CommitTransaction(wtxNew, vpChangeKey, g_connman.get(), state)) {
if (!pwalletMain->CommitTransaction(wtxNew, vChangeKey, g_connman.get(), state)) {
strError = strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason());
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
Expand All @@ -449,17 +446,14 @@ static void SendGenerationTransaction(const CScript& assetScriptPubKey, const CP
}

// Create and send the transaction
std::vector<CReserveKey*> vpChangeKey;
std::vector<CReserveKey> vChangeKey;
// Need to be careful to not copy CReserveKeys or bad things happen
// We need 2 change outputs possibly for reissuance case:
// one for policyAsset, the other for reissuance token
vChangeKey.reserve(2);
vChangeKey.emplace_back(pwalletMain);
vpChangeKey.push_back(&vChangeKey[0]);
if (!reissuanceAsset.IsNull()) {
vChangeKey.emplace_back(pwalletMain);
vpChangeKey.push_back(&vChangeKey[1]);
}

CAmount nFeeRequired;
Expand All @@ -481,11 +475,11 @@ static void SendGenerationTransaction(const CScript& assetScriptPubKey, const CP
}
vecSend.push_back(recipient);
}
if (!pwalletMain->CreateTransaction(vecSend, wtxNew, vpChangeKey, nFeeRequired, nChangePosRet, strError, NULL, true, NULL, fBlindIssuances, &entropy, &reissuanceAsset, &reissuanceToken)) {
if (!pwalletMain->CreateTransaction(vecSend, wtxNew, vChangeKey, nFeeRequired, nChangePosRet, strError, NULL, true, NULL, fBlindIssuances, &entropy, &reissuanceAsset, &reissuanceToken)) {
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
CValidationState state;
if (!pwalletMain->CommitTransaction(wtxNew, vpChangeKey, g_connman.get(), state))
if (!pwalletMain->CommitTransaction(wtxNew, vChangeKey, g_connman.get(), state))
throw JSONRPCError(RPC_WALLET_ERROR, "Error: The transaction was rejected! This might happen if some of the coins in your wallet were already spent, such as if you used a copy of the wallet and coins were spent in the copy but not marked as spent here.");
}

Expand Down Expand Up @@ -1201,7 +1195,6 @@ UniValue sendmany(const JSONRPCRequest& request)

// Send
std::vector<CReserveKey> vChangeKey;
std::vector<CReserveKey*> vpChangeKey;
std::set<CAsset> setAssets;
setAssets.insert(policyAsset);
for (auto recipient : vecSend) {
Expand All @@ -1210,16 +1203,15 @@ UniValue sendmany(const JSONRPCRequest& request)
vChangeKey.reserve(setAssets.size());
for (unsigned int i = 0; i < setAssets.size(); i++) {
vChangeKey.emplace_back(pwalletMain);
vpChangeKey.push_back(&vChangeKey[i]);
}
CAmount nFeeRequired = 0;
int nChangePosRet = -1;
string strFailReason;
bool fCreated = pwalletMain->CreateTransaction(vecSend, wtx, vpChangeKey, nFeeRequired, nChangePosRet, strFailReason, NULL, true, NULL, false, NULL, NULL, NULL, fIgnoreBlindFail);
bool fCreated = pwalletMain->CreateTransaction(vecSend, wtx, vChangeKey, nFeeRequired, nChangePosRet, strFailReason, NULL, true, NULL, false, NULL, NULL, NULL, fIgnoreBlindFail);
if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
CValidationState state;
if (!pwalletMain->CommitTransaction(wtx, vpChangeKey, g_connman.get(), state)) {
if (!pwalletMain->CommitTransaction(wtx, vChangeKey, g_connman.get(), state)) {
strFailReason = strprintf("Transaction commit failed:: %s", state.GetRejectReason());
throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
}
Expand Down Expand Up @@ -3296,7 +3288,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
}

// commit/broadcast the tx
std::vector<CReserveKey*> vpChangeKey;
std::vector<CReserveKey> vChangeKey;
CWalletTx wtxBumped(pwalletMain, MakeTransactionRef(std::move(tx)));
wtxBumped.mapValue = wtx.mapValue;
wtxBumped.mapValue["replaces_txid"] = hash.ToString();
Expand All @@ -3305,7 +3297,7 @@ UniValue bumpfee(const JSONRPCRequest& request)
wtxBumped.fTimeReceivedIsTxTime = true;
wtxBumped.fFromMe = true;
CValidationState state;
if (!pwalletMain->CommitTransaction(wtxBumped, vpChangeKey, g_connman.get(), state)) {
if (!pwalletMain->CommitTransaction(wtxBumped, vChangeKey, g_connman.get(), state)) {
// NOTE: CommitTransaction never returns false, so this should never happen.
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason()));
}
Expand Down
25 changes: 11 additions & 14 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2607,7 +2607,6 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
std::vector<CReserveKey> vChangeKey;
// Avoid copying CReserveKeys which causes badness
vChangeKey.reserve((tx.vin.size()+tx.vout.size())*2);
std::vector<CReserveKey*> vpChangeKey;
std::set<CAsset> setAssets;

// Turn the txout set into a CRecipient vector
Expand All @@ -2627,7 +2626,6 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov

if (setAssets.count(txOut.nAsset.GetAsset()) == 0) {
vChangeKey.push_back(CReserveKey(this));
vpChangeKey.push_back(&vChangeKey[vChangeKey.size()-1]);
setAssets.insert(txOut.nAsset.GetAsset());
}
}
Expand All @@ -2636,13 +2634,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
// Any excess will never be reserved, so this should be safe.
for (unsigned int i = 0; i < tx.vin.size(); i++) {
vChangeKey.push_back(CReserveKey(this));
vpChangeKey.push_back(&vChangeKey[vChangeKey.size()-1]);
}

// Always add policyAsset, as fees via policyAsset may create change
if (setAssets.count(policyAsset) == 0) {
vChangeKey.push_back(CReserveKey(this));
vpChangeKey.push_back(&vChangeKey[vChangeKey.size()-1]);
}

CCoinControl coinControl;
Expand All @@ -2656,7 +2652,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov
coinControl.Select(txin.prevout);

CWalletTx wtx;
if (!CreateTransaction(vecSend, wtx, vpChangeKey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false))
if (!CreateTransaction(vecSend, wtx, vChangeKey, nFeeRet, nChangePosInOut, strFailReason, &coinControl, false))
return false;

// Wipe outputs and output witness and re-add one by one
Expand Down Expand Up @@ -2691,14 +2687,14 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, bool ov

// optionally keep the change output key
if (keepReserveKey) {
for (auto& changekey : vpChangeKey) {
changekey->KeepKey();
for (auto& changekey : vChangeKey) {
changekey.KeepKey();
}
}
return true;
}

bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wtxNew, std::vector<CReserveKey*>& vpChangeKey, CAmount& nFeeRet,
bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wtxNew, std::vector<CReserveKey>& vChangeKey, CAmount& nFeeRet,
int& nChangePosInOut, std::string& strFailReason, const CCoinControl* coinControl, bool sign, std::vector<CAmount> *outAmounts, bool fBlindIssuances, const uint256* issuanceEntropy, const CAsset* reissuanceAsset, const CAsset* reissuanceToken, bool fIgnoreBlindFail)
{
// TODO re-enable to support multiple assets in a logical fashion, since the number of possible
Expand Down Expand Up @@ -2910,7 +2906,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
// Reserve a new key pair from key pool
CPubKey vchPubKey;
bool ret;
ret = vpChangeKey[changeCounter]->GetReservedKey(vchPubKey);
ret = vChangeKey[changeCounter].GetReservedKey(vchPubKey);
if (!ret)
{
strFailReason = _("Keypool ran out, please call keypoolrefill first");
Expand Down Expand Up @@ -2950,7 +2946,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
{
nChangePosInOut = -1;
nFeeRet += it->second;
vpChangeKey[changeCounter]->ReturnKey();
vChangeKey[changeCounter].ReturnKey();
}
else
{
Expand Down Expand Up @@ -2979,8 +2975,9 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
nChangePosInOut = -1;
}
}
else
vpChangeKey[changeCounter]->ReturnKey();
else {
vChangeKey[changeCounter].ReturnKey();
}
changeCounter++;
}

Expand Down Expand Up @@ -3347,15 +3344,15 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
/**
* Call after CreateTransaction unless you want to abort
*/
bool CWallet::CommitTransaction(CWalletTx& wtxNew, std::vector<CReserveKey*>& reservekey, CConnman* connman, CValidationState& state)
bool CWallet::CommitTransaction(CWalletTx& wtxNew, std::vector<CReserveKey>& reservekey, CConnman* connman, CValidationState& state)
{
{
LOCK2(cs_main, cs_wallet);
LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString());
{
// Take key pair from key pool so it won't be used again
for (auto&& key : reservekey)
key->KeepKey();
key.KeepKey();

// Add tx to wallet, because if it has change it's also ours,
// otherwise just for transaction history.
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -874,9 +874,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
* selected by SelectCoins(); Also create the change output, when needed
* @note passing nChangePosInOut as -1 will result in setting a random position
*/
bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, std::vector<CReserveKey*>& vpChangeKey, CAmount& nFeeRet, int& nChangePosInOut,
bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, std::vector<CReserveKey>& vChangeKey, CAmount& nFeeRet, int& nChangePosInOut,
std::string& strFailReason, const CCoinControl *coinControl = NULL, bool sign = true, std::vector<CAmount> *outAmounts = NULL, bool fBlindIssuances = true, const uint256* issuanceEntropy = NULL, const CAsset* reissuanceAsset = NULL, const CAsset* reissuanceToken = NULL, bool fIgnoreBlindFail = true);
bool CommitTransaction(CWalletTx& wtxNew, std::vector<CReserveKey*>& reservekey, CConnman* connman, CValidationState& state);
bool CommitTransaction(CWalletTx& wtxNew, std::vector<CReserveKey>& reservekey, CConnman* connman, CValidationState& state);

void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries);
bool AddAccountingEntry(const CAccountingEntry&);
Expand Down

0 comments on commit f400def

Please sign in to comment.