change wallet pointers to references in feebumper

adamjonas · adamjonas · commit 66b29848c71c · 2019-10-10T17:46:59.000-04:00
diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
@@ -241,7 +241,7 @@ class WalletImpl : public Wallet
     }
     bool transactionCanBeBumped(const uint256& txid) override
     {
-        return feebumper::TransactionCanBeBumped(m_wallet.get(), txid);
+        return feebumper::TransactionCanBeBumped(*m_wallet.get(), txid);
     }
     bool createBumpTransaction(const uint256& txid,
         const CCoinControl& coin_control,
@@ -255,17 +255,17 @@ class WalletImpl : public Wallet
             return feebumper::CreateTotalBumpTransaction(m_wallet.get(), txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) ==
                 feebumper::Result::OK;
         } else {
-            return feebumper::CreateRateBumpTransaction(m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) ==
+            return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) ==
                 feebumper::Result::OK;
         }
     }
-    bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(m_wallet.get(), mtx); }
+    bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
     bool commitBumpTransaction(const uint256& txid,
         CMutableTransaction&& mtx,
         std::vector<std::string>& errors,
         uint256& bumped_txid) override
     {
-        return feebumper::CommitTransaction(m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==
+        return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==
                feebumper::Result::OK;
     }
     CTransactionRef getTx(const uint256& txid) override
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -17,15 +17,15 @@
 
 //! Check whether transaction has descendant in wallet or mempool, or has been
 //! mined, or conflicts with a mined transaction. Return a feebumper::Result.
-static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet)
+static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet& wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
-    if (wallet->HasWalletSpend(wtx.GetHash())) {
+    if (wallet.HasWalletSpend(wtx.GetHash())) {
         errors.push_back("Transaction has descendants in the wallet");
         return feebumper::Result::INVALID_PARAMETER;
     }
 
     {
-        if (wallet->chain().hasDescendantsInMempool(wtx.GetHash())) {
+        if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
             errors.push_back("Transaction has descendants in the mempool");
             return feebumper::Result::INVALID_PARAMETER;
         }
@@ -48,7 +48,7 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai
 
     // check that original tx consists entirely of our inputs
     // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
-    if (!wallet->IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
+    if (!wallet.IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
         errors.push_back("Transaction contains inputs that don't belong to this wallet");
         return feebumper::Result::WALLET_ERROR;
     }
@@ -58,13 +58,13 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai
 }
 
 //! Check if the user provided a valid feeRate
-static feebumper::Result CheckFeeRate(const CWallet* wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) {
+static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) {
     // check that fee rate is higher than mempool's minimum fee
     // (no point in bumping fee if we know that the new tx won't be accepted to the mempool)
     // This may occur if the user set FeeRate, TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps,
     // in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
     // moment earlier. In this case, we report an error to the user, who may adjust the fee.
-    CFeeRate minMempoolFeeRate = wallet->chain().mempoolMinFee();
+    CFeeRate minMempoolFeeRate = wallet.chain().mempoolMinFee();
 
     if (newFeerate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
         errors.push_back(strprintf(
@@ -76,7 +76,7 @@ static feebumper::Result CheckFeeRate(const CWallet* wallet, const CWalletTx& wt
 
     CAmount new_total_fee = newFeerate.GetFee(maxTxSize);
 
-    CFeeRate incrementalRelayFee = std::max(wallet->chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE));
+    CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE));
 
     // Given old total fee and transaction size, calculate the old feeRate
     CAmount old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut();
@@ -91,15 +91,15 @@ static feebumper::Result CheckFeeRate(const CWallet* wallet, const CWalletTx& wt
         return feebumper::Result::INVALID_PARAMETER;
     }
 
-    CAmount requiredFee = GetRequiredFee(*wallet, maxTxSize);
+    CAmount requiredFee = GetRequiredFee(wallet, maxTxSize);
     if (new_total_fee < requiredFee) {
         errors.push_back(strprintf("Insufficient total fee (cannot be less than required fee %s)",
             FormatMoney(requiredFee)));
         return feebumper::Result::INVALID_PARAMETER;
     }
 
     // Check that in all cases the new fee doesn't violate maxTxFee
-    const CAmount max_tx_fee = wallet->m_default_max_tx_fee;
+    const CAmount max_tx_fee = wallet.m_default_max_tx_fee;
     if (new_total_fee > max_tx_fee) {
         errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)",
                             FormatMoney(new_total_fee), FormatMoney(max_tx_fee)));
@@ -109,7 +109,7 @@ static feebumper::Result CheckFeeRate(const CWallet* wallet, const CWalletTx& wt
     return feebumper::Result::OK;
 }
 
-static CFeeRate EstimateFeeRate(CWallet* wallet, const CWalletTx& wtx, CCoinControl& coin_control, CAmount& old_fee)
+static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, CCoinControl& coin_control, CAmount& old_fee)
 {
     // Get the fee rate of the original transaction. This is calculated from
     // the tx fee/vsize, so it may have been rounded down. Add 1 satoshi to the
@@ -126,24 +126,24 @@ static CFeeRate EstimateFeeRate(CWallet* wallet, const CWalletTx& wtx, CCoinCont
     // aware of. This ensures we're over the required relay fee rate
     // (BIP 125 rule 4).  The replacement tx will be at least as large as the
     // original tx, so the total fee will be greater (BIP 125 rule 3)
-    CFeeRate node_incremental_relay_fee = wallet->chain().relayIncrementalFee();
+    CFeeRate node_incremental_relay_fee = wallet.chain().relayIncrementalFee();
     CFeeRate wallet_incremental_relay_fee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE);
     feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee);
 
     // Fee rate must also be at least the wallet's GetMinimumFeeRate
-    CFeeRate min_feerate(GetMinimumFeeRate(*wallet, coin_control, /* feeCalc */ nullptr));
+    CFeeRate min_feerate(GetMinimumFeeRate(wallet, coin_control, /* feeCalc */ nullptr));
 
     // Set the required fee rate for the replacement transaction in coin control.
     return std::max(feerate, min_feerate);
 }
 
 namespace feebumper {
 
-bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid)
+bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
 {
-    auto locked_chain = wallet->chain().lock();
-    LOCK(wallet->cs_wallet);
-    const CWalletTx* wtx = wallet->GetWalletTx(txid);
+    auto locked_chain = wallet.chain().lock();
+    LOCK(wallet.cs_wallet);
+    const CWalletTx* wtx = wallet.GetWalletTx(txid);
     if (wtx == nullptr) return false;
 
     std::vector<std::string> errors_dummy;
@@ -166,7 +166,7 @@ Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, co
     }
     const CWalletTx& wtx = it->second;
 
-    Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors);
+    Result result = PreconditionChecks(*locked_chain, *wallet, wtx, errors);
     if (result != Result::OK) {
         return result;
     }
@@ -276,17 +276,17 @@ Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, co
 }
 
 
-Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<std::string>& errors,
+Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<std::string>& errors,
                                  CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
 {
     // We are going to modify coin control later, copy to re-use
     CCoinControl new_coin_control(coin_control);
 
-    auto locked_chain = wallet->chain().lock();
-    LOCK(wallet->cs_wallet);
+    auto locked_chain = wallet.chain().lock();
+    LOCK(wallet.cs_wallet);
     errors.clear();
-    auto it = wallet->mapWallet.find(txid);
-    if (it == wallet->mapWallet.end()) {
+    auto it = wallet.mapWallet.find(txid);
+    if (it == wallet.mapWallet.end()) {
         errors.push_back("Invalid or non-wallet transaction id");
         return Result::INVALID_ADDRESS_OR_KEY;
     }
@@ -300,7 +300,7 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
     // Fill in recipients(and preserve a single change key if there is one)
     std::vector<CRecipient> recipients;
     for (const auto& output : wtx.tx->vout) {
-        if (!wallet->IsChange(output)) {
+        if (!wallet.IsChange(output)) {
             CRecipient recipient = {output.scriptPubKey, output.nValue, false};
             recipients.push_back(recipient);
         } else {
@@ -313,7 +313,7 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
     if (coin_control.m_feerate) {
         // The user provided a feeRate argument.
         // We calculate this here to avoid compiler warning on the cs_wallet lock
-        const int64_t maxTxSize = CalculateMaximumSignedTxSize(*wtx.tx, wallet);
+        const int64_t maxTxSize = CalculateMaximumSignedTxSize(*wtx.tx, &wallet);
         Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, errors);
         if (res != Result::OK) {
             return res;
@@ -342,7 +342,7 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
     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, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
+    if (!wallet.CreateTransaction(*locked_chain, recipients, tx_new, 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;
     }
@@ -353,7 +353,7 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
     // Write back transaction
     mtx = CMutableTransaction(*tx_new);
     // Mark new tx not replaceable, if requested.
-    if (!coin_control.m_signal_bip125_rbf.get_value_or(wallet->m_signal_rbf)) {
+    if (!coin_control.m_signal_bip125_rbf.get_value_or(wallet.m_signal_rbf)) {
         for (auto& input : mtx.vin) {
             if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe;
         }
@@ -362,21 +362,21 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo
     return Result::OK;
 }
 
-bool SignTransaction(CWallet* wallet, CMutableTransaction& mtx) {
-    auto locked_chain = wallet->chain().lock();
-    LOCK(wallet->cs_wallet);
-    return wallet->SignTransaction(mtx);
+bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
+    auto locked_chain = wallet.chain().lock();
+    LOCK(wallet.cs_wallet);
+    return wallet.SignTransaction(mtx);
 }
 
-Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid)
+Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid)
 {
-    auto locked_chain = wallet->chain().lock();
-    LOCK(wallet->cs_wallet);
+    auto locked_chain = wallet.chain().lock();
+    LOCK(wallet.cs_wallet);
     if (!errors.empty()) {
         return Result::MISC_ERROR;
     }
-    auto it = txid.IsNull() ? wallet->mapWallet.end() : wallet->mapWallet.find(txid);
-    if (it == wallet->mapWallet.end()) {
+    auto it = txid.IsNull() ? wallet.mapWallet.end() : wallet.mapWallet.find(txid);
+    if (it == wallet.mapWallet.end()) {
         errors.push_back("Invalid or non-wallet transaction id");
         return Result::MISC_ERROR;
     }
@@ -394,7 +394,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti
     mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
 
     CValidationState state;
-    if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, state)) {
+    if (!wallet.CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, 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;
@@ -408,7 +408,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti
     }
 
     // mark the original tx as bumped
-    if (!wallet->MarkReplaced(oldWtx.GetHash(), bumped_txid)) {
+    if (!wallet.MarkReplaced(oldWtx.GetHash(), bumped_txid)) {
         // TODO: see if JSON-RPC has a standard way of returning a response
         // along with an exception. It would be good to return information about
         // wtxBumped to the caller even if marking the original transaction
diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h
@@ -26,7 +26,7 @@ enum class Result
 };
 
 //! Return whether transaction can be bumped.
-bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid);
+bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
 
 //! Create bumpfee transaction based on total amount.
 Result CreateTotalBumpTransaction(const CWallet* wallet,
@@ -39,7 +39,7 @@ Result CreateTotalBumpTransaction(const CWallet* wallet,
                          CMutableTransaction& mtx);
 
 //! Create bumpfee transaction based on feerate estimates.
-Result CreateRateBumpTransaction(CWallet* wallet,
+Result CreateRateBumpTransaction(CWallet& wallet,
                          const uint256& txid,
                          const CCoinControl& coin_control,
                          std::vector<std::string>& errors,
@@ -50,13 +50,13 @@ Result CreateRateBumpTransaction(CWallet* wallet,
 //! Sign the new transaction,
 //! @return false if the tx couldn't be found or if it was
 //! impossible to create the signature(s)
-bool SignTransaction(CWallet* wallet, CMutableTransaction& mtx);
+bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx);
 
 //! Commit the bumpfee transaction.
 //! @return success in case of CWallet::CommitTransaction was successful,
 //! but sets errors if the tx could not be added to the mempool (will try later)
 //! or if the old transaction could not be marked as replaced.
-Result CommitTransaction(CWallet* wallet,
+Result CommitTransaction(CWallet& wallet,
                          const uint256& txid,
                          CMutableTransaction&& mtx,
                          std::vector<std::string>& errors,
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3417,7 +3417,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
         res = feebumper::CreateTotalBumpTransaction(pwallet, hash, coin_control, totalFee, errors, old_fee, new_fee, mtx);
     } else {
         // Targeting feerate bump.
-        res = feebumper::CreateRateBumpTransaction(pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);
+        res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);
     }
     if (res != feebumper::Result::OK) {
         switch(res) {
@@ -3440,12 +3440,12 @@ static UniValue bumpfee(const JSONRPCRequest& request)
     }
 
     // sign bumped transaction
-    if (!feebumper::SignTransaction(pwallet, mtx)) {
+    if (!feebumper::SignTransaction(*pwallet, mtx)) {
         throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");
     }
     // commit the bumped transaction
     uint256 txid;
-    if (feebumper::CommitTransaction(pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
+    if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {
         throw JSONRPCError(RPC_WALLET_ERROR, errors[0]);
     }
     UniValue result(UniValue::VOBJ);

Original file line number	Diff line number	Diff line change
`@@ -241,7 +241,7 @@ class WalletImpl : public Wallet`
`241`	`241`	`}`
`242`	`242`	`bool transactionCanBeBumped(const uint256& txid) override`
`243`	`243`	`{`
`244`		`- return feebumper::TransactionCanBeBumped(m_wallet.get(), txid);`
	`244`	`+ return feebumper::TransactionCanBeBumped(*m_wallet.get(), txid);`
`245`	`245`	`}`
`246`	`246`	`bool createBumpTransaction(const uint256& txid,`
`247`	`247`	`const CCoinControl& coin_control,`
`@@ -255,17 +255,17 @@ class WalletImpl : public Wallet`
`255`	`255`	`return feebumper::CreateTotalBumpTransaction(m_wallet.get(), txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) ==`
`256`	`256`	`feebumper::Result::OK;`
`257`	`257`	`} else {`
`258`		`- return feebumper::CreateRateBumpTransaction(m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) ==`
	`258`	`+ return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) ==`
`259`	`259`	`feebumper::Result::OK;`
`260`	`260`	`}`
`261`	`261`	`}`
`262`		`- bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(m_wallet.get(), mtx); }`
	`262`	`+ bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }`
`263`	`263`	`bool commitBumpTransaction(const uint256& txid,`
`264`	`264`	`CMutableTransaction&& mtx,`
`265`	`265`	`std::vector<std::string>& errors,`
`266`	`266`	`uint256& bumped_txid) override`
`267`	`267`	`{`
`268`		`- return feebumper::CommitTransaction(m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==`
	`268`	`+ return feebumper::CommitTransaction(*m_wallet.get(), txid, std::move(mtx), errors, bumped_txid) ==`
`269`	`269`	`feebumper::Result::OK;`
`270`	`270`	`}`
`271`	`271`	`CTransactionRef getTx(const uint256& txid) override`
Original file line number	Diff line number	Diff line change
`@@ -3417,7 +3417,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)`
`3417`	`3417`	`res = feebumper::CreateTotalBumpTransaction(pwallet, hash, coin_control, totalFee, errors, old_fee, new_fee, mtx);`
`3418`	`3418`	`} else {`
`3419`	`3419`	`// Targeting feerate bump.`
`3420`		`- res = feebumper::CreateRateBumpTransaction(pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);`
	`3420`	`+ res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);`
`3421`	`3421`	`}`
`3422`	`3422`	`if (res != feebumper::Result::OK) {`
`3423`	`3423`	`switch(res) {`
`@@ -3440,12 +3440,12 @@ static UniValue bumpfee(const JSONRPCRequest& request)`
`3440`	`3440`	`}`
`3441`	`3441`
`3442`	`3442`	`// sign bumped transaction`
`3443`		`- if (!feebumper::SignTransaction(pwallet, mtx)) {`
	`3443`	`+ if (!feebumper::SignTransaction(*pwallet, mtx)) {`
`3444`	`3444`	`throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction.");`
`3445`	`3445`	`}`
`3446`	`3446`	`// commit the bumped transaction`
`3447`	`3447`	`uint256 txid;`
`3448`		`- if (feebumper::CommitTransaction(pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {`
	`3448`	`+ if (feebumper::CommitTransaction(*pwallet, hash, std::move(mtx), errors, txid) != feebumper::Result::OK) {`
`3449`	`3449`	`throw JSONRPCError(RPC_WALLET_ERROR, errors[0]);`
`3450`	`3450`	`}`
`3451`	`3451`	`UniValue result(UniValue::VOBJ);`