Replace CReserveKey with ReserveDestinatoin

Instead of reserving keys, reserve destinations which are backed by keys
This commit is contained in:
Andrew Chow 2019-06-18 15:48:20 -04:00
parent 172213be5b
commit 33d13edd2b
6 changed files with 73 additions and 74 deletions

View file

@ -36,7 +36,7 @@ namespace {
class PendingWalletTxImpl : public PendingWalletTx
{
public:
explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet), m_key(&wallet) {}
explicit PendingWalletTxImpl(CWallet& wallet) : m_wallet(wallet), m_dest(&wallet) {}
const CTransaction& get() override { return *m_tx; }
@ -47,7 +47,7 @@ public:
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
CValidationState state;
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, state)) {
if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_dest, state)) {
reject_reason = state.GetRejectReason();
return false;
}
@ -56,7 +56,7 @@ public:
CTransactionRef m_tx;
CWallet& m_wallet;
CReserveKey m_key;
ReserveDestination m_dest;
};
//! Construct wallet tx struct.
@ -238,7 +238,7 @@ public:
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet);
auto pending = MakeUnique<PendingWalletTxImpl>(*m_wallet);
if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_key, fee, change_pos,
if (!m_wallet->CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_dest, fee, change_pos,
fail_reason, coin_control, sign)) {
return {};
}

View file

@ -272,17 +272,17 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
new_coin_control.m_min_depth = 1;
CTransactionRef tx_new = MakeTransactionRef();
CReserveKey reservekey(wallet);
ReserveDestination reservedest(wallet);
CAmount fee_ret;
int change_pos_in_out = -1; // No requested location for change
std::string fail_reason;
if (!wallet->CreateTransaction(*locked_chain, recipients, tx_new, reservekey, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
if (!wallet->CreateTransaction(*locked_chain, recipients, tx_new, reservedest, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
errors.push_back("Unable to create transaction: " + fail_reason);
return Result::WALLET_ERROR;
}
// If change key hasn't been ReturnKey'ed by this point, we take it out of keypool
reservekey.KeepKey();
reservedest.KeepDestination();
// Write back new fee if successful
new_fee = fee_ret;
@ -330,9 +330,9 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti
mapValue_t mapValue = oldWtx.mapValue;
mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
CReserveKey reservekey(wallet);
ReserveDestination reservedest(wallet);
CValidationState state;
if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservekey, state)) {
if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservedest, state)) {
// NOTE: CommitTransaction never returns false, so this should never happen.
errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state)));
return Result::WALLET_ERROR;

View file

@ -237,16 +237,12 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request)
}
}
CReserveKey reservekey(pwallet);
CPubKey vchPubKey;
if (!reservekey.GetReservedKey(vchPubKey, true))
ReserveDestination reservedest(pwallet);
CTxDestination dest;
if (!reservedest.GetReservedDestination(output_type, dest, true))
throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
reservekey.KeepKey();
pwallet->LearnRelatedScripts(vchPubKey, output_type);
CTxDestination dest = GetDestinationForKey(vchPubKey, output_type);
reservedest.KeepDestination();
return EncodeDestination(dest);
}
@ -313,7 +309,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
CScript scriptPubKey = GetScriptForDestination(address);
// Create and send the transaction
CReserveKey reservekey(pwallet);
ReserveDestination reservedest(pwallet);
CAmount nFeeRequired;
std::string strError;
std::vector<CRecipient> vecSend;
@ -321,13 +317,13 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
vecSend.push_back(recipient);
CTransactionRef tx;
if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) {
if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservedest, nFeeRequired, nChangePosRet, strError, coin_control)) {
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 (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservekey, state)) {
if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservedest, state)) {
strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state));
throw JSONRPCError(RPC_WALLET_ERROR, strError);
}
@ -921,16 +917,16 @@ static UniValue sendmany(const JSONRPCRequest& request)
std::shuffle(vecSend.begin(), vecSend.end(), FastRandomContext());
// Send
CReserveKey keyChange(pwallet);
ReserveDestination changedest(pwallet);
CAmount nFeeRequired = 0;
int nChangePosRet = -1;
std::string strFailReason;
CTransactionRef tx;
bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control);
bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, changedest, nFeeRequired, nChangePosRet, strFailReason, coin_control);
if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
CValidationState state;
if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, keyChange, state)) {
if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, changedest, state)) {
strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state));
throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
}

View file

@ -363,17 +363,17 @@ public:
CWalletTx& AddTx(CRecipient recipient)
{
CTransactionRef tx;
CReserveKey reservekey(wallet.get());
ReserveDestination reservedest(wallet.get());
CAmount fee;
int changePos = -1;
std::string error;
CCoinControl dummy;
{
auto locked_chain = m_chain->lock();
BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, reservekey, fee, changePos, error, dummy));
BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, reservedest, fee, changePos, error, dummy));
}
CValidationState state;
BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservekey, state));
BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservedest, state));
CMutableTransaction blocktx;
{
LOCK(wallet->cs_wallet);

View file

@ -2609,9 +2609,9 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
auto locked_chain = chain().lock();
LOCK(cs_wallet);
CReserveKey reservekey(this);
ReserveDestination reservedest(this);
CTransactionRef tx_new;
if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservedest, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
return false;
}
@ -2619,7 +2619,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
// We don't have the normal Create/Commit cycle, and don't want to risk
// reusing change, so just remove the key from the keypool here.
reservekey.KeepKey();
reservedest.KeepDestination();
}
// Copy output sizes from new transaction; they may have had the fee
@ -2730,7 +2730,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec
return m_default_address_type;
}
bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet,
bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, ReserveDestination& reservedest, CAmount& nFeeRet,
int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
{
CAmount nValue = 0;
@ -2771,7 +2771,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
// Create change script that will be used if we need change
// TODO: pass in scriptChange instead of reservekey so
// TODO: pass in scriptChange instead of reservedest so
// change transaction isn't always pay-to-bitcoin-address
CScript scriptChange;
@ -2791,19 +2791,16 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
strFailReason = _("Can't generate a change-address key. No keys in the internal keypool and can't generate any keys.");
return false;
}
CPubKey vchPubKey;
bool ret;
ret = reservekey.GetReservedKey(vchPubKey, true);
CTxDestination dest;
const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend);
bool ret = reservedest.GetReservedDestination(change_type, dest, true);
if (!ret)
{
strFailReason = _("Keypool ran out, please call keypoolrefill first");
strFailReason = "Keypool ran out, please call keypoolrefill first";
return false;
}
const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend);
LearnRelatedScripts(vchPubKey, change_type);
scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type));
scriptChange = GetScriptForDestination(dest);
}
CTxOut change_prototype_txout(0, scriptChange);
coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout);
@ -3024,7 +3021,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
}
}
if (nChangePosInOut == -1) reservekey.ReturnKey(); // Return any reserved key if we don't have change
if (nChangePosInOut == -1) reservedest.ReturnDestination(); // Return any reserved address if we don't have change
// Shuffle selected coins and fill in final vin
txNew.vin.clear();
@ -3097,7 +3094,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
/**
* Call after CreateTransaction unless you want to abort
*/
bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CReserveKey& reservekey, CValidationState& state)
bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, ReserveDestination& reservedest, CValidationState& state)
{
{
auto locked_chain = chain().lock();
@ -3112,7 +3109,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
WalletLogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); /* Continued */
{
// Take key pair from key pool so it won't be used again
reservekey.KeepKey();
reservedest.KeepDestination();
// Add tx to wallet, because if it has change it's also ours,
// otherwise just for transaction history.
@ -3713,7 +3710,7 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co
return result;
}
bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool internal)
bool ReserveDestination::GetReservedDestination(const OutputType type, CTxDestination& dest, bool internal)
{
if (!pwallet->CanGetAddresses(internal)) {
return false;
@ -3729,25 +3726,29 @@ bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool internal)
fInternal = keypool.fInternal;
}
assert(vchPubKey.IsValid());
pubkey = vchPubKey;
pwallet->LearnRelatedScripts(vchPubKey, type);
address = GetDestinationForKey(vchPubKey, type);
dest = address;
return true;
}
void CReserveKey::KeepKey()
void ReserveDestination::KeepDestination()
{
if (nIndex != -1)
pwallet->KeepKey(nIndex);
nIndex = -1;
vchPubKey = CPubKey();
address = CNoDestination();
}
void CReserveKey::ReturnKey()
void ReserveDestination::ReturnDestination()
{
if (nIndex != -1) {
pwallet->ReturnKey(nIndex, fInternal, vchPubKey);
}
nIndex = -1;
vchPubKey = CPubKey();
address = CNoDestination();
}
void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id)

View file

@ -85,11 +85,11 @@ static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91;
class CCoinControl;
class COutput;
class CReserveKey;
class CScript;
class CWalletTx;
struct FeeCalculation;
enum class FeeEstimateMode;
class ReserveDestination;
/** (client) version numbers for particular wallet features */
enum WalletFeature
@ -234,55 +234,57 @@ public:
}
};
/** A wrapper to reserve a key from a wallet keypool
/** A wrapper to reserve an address from a wallet
*
* CReserveKey is used to reserve a key from the keypool. It is passed around
* ReserveDestination is used to reserve an address. It is passed around
* during the CreateTransaction/CommitTransaction procedure.
*
* Instantiating a CReserveKey does not reserve a keypool key. To do so,
* GetReservedKey() needs to be called on the object. Once a key has been
* reserved, call KeepKey() on the CReserveKey object to make sure it is not
* returned to the keypool. Call ReturnKey() to return the key to the keypool
* so it can be re-used (for example, if the key was used in a new transaction
* Instantiating a ReserveDestination does not reserve an address. To do so,
* GetReservedDestination() needs to be called on the object. Once an address has been
* reserved, call KeepDestination() on the ReserveDestination object to make sure it is not
* returned. Call ReturnDestination() to return the address so it can be re-used (for
* example, if the address was used in a new transaction
* and that transaction was not completed and needed to be aborted).
*
* If a key is reserved and KeepKey() is not called, then the key will be
* returned to the keypool when the CReserveObject goes out of scope.
* If an address is reserved and KeepDestination() is not called, then the address will be
* returned when the ReserveDestination goes out of scope.
*/
class CReserveKey
class ReserveDestination
{
protected:
//! The wallet to reserve the keypool key from
//! The wallet to reserve from
CWallet* pwallet;
//! The index of the key in the keypool
//! The index of the address's key in the keypool
int64_t nIndex{-1};
//! The public key
//! The public key for the address
CPubKey vchPubKey;
//! The destination
CTxDestination address;
//! Whether this is from the internal (change output) keypool
bool fInternal{false};
public:
//! Construct a CReserveKey object. This does NOT reserve a key from the keypool yet
explicit CReserveKey(CWallet* pwalletIn)
//! Construct a ReserveDestination object. This does NOT reserve an address yet
explicit ReserveDestination(CWallet* pwalletIn)
{
pwallet = pwalletIn;
}
CReserveKey(const CReserveKey&) = delete;
CReserveKey& operator=(const CReserveKey&) = delete;
ReserveDestination(const ReserveDestination&) = delete;
ReserveDestination& operator=(const ReserveDestination&) = delete;
//! Destructor. If a key has been reserved and not KeepKey'ed, it will be returned to the keypool
~CReserveKey()
~ReserveDestination()
{
ReturnKey();
ReturnDestination();
}
//! Reserve a key from the keypool
bool GetReservedKey(CPubKey &pubkey, bool internal = false);
//! Return a key to the keypool
void ReturnKey();
//! Keep the key. Do not return it to the keypool when this object goes out of scope
void KeepKey();
//! Reserve an address
bool GetReservedDestination(const OutputType type, CTxDestination& pubkey, bool internal);
//! Return reserved address
void ReturnDestination();
//! Keep the address. Do not return it's key to the keypool when this object goes out of scope
void KeepDestination();
};
/** Address book data */
@ -1056,9 +1058,9 @@ public:
* selected by SelectCoins(); Also create the change output, when needed
* @note passing nChangePosInOut as -1 will result in setting a random position
*/
bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut,
bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, ReserveDestination& reservedest, CAmount& nFeeRet, int& nChangePosInOut,
std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CReserveKey& reservekey, CValidationState& state);
bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, ReserveDestination& reservedest, CValidationState& state);
bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const
{