Skip to content

Commit a5fd746

Browse files
committed
Merge #9681: Refactor Bumpfee, move core functionality to CWallet
5f59d3e Improve CFeeBumper interface, add comments, make use of std::move (Jonas Schnelli) 0df22ed Cancel feebump is vErrors is not empty (Jonas Schnelli) 44cabe6 Use static calls for GetRequiredFee and GetMinimumFee, remove make_pair from emplace_back (Jonas Schnelli) bb78c15 Restore CalculateMaximumSignedTxSize function signature (Jonas Schnelli) 51ea44f Use "return false" instead assert() in CWallet::SignTransaction (Jonas Schnelli) bcc72cc Directly abort execution in FeeBumper::commit if wallet or tx is not available (Jonas Schnelli) 2718db0 Restore invalid fee check (must be > 0) (Jonas Schnelli) 0337a39 Refactor Bumpfee core functionality (Jonas Schnelli) d1a95e8 Bumpfee move request parameter interaction to the top (Jonas Schnelli) Tree-SHA512: 0e6d1f3322ed671fa2291e59ac9556ce4646bc78267edc6eedc46b0014b7b08aa83c30315358b911d82898847d4845634a18b67e253a7b699dcc852eb2652c07
2 parents 928695b + 5f59d3e commit a5fd746

File tree

6 files changed

+407
-224
lines changed

6 files changed

+407
-224
lines changed

src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ BITCOIN_CORE_H = \
157157
wallet/coincontrol.h \
158158
wallet/crypter.h \
159159
wallet/db.h \
160+
wallet/feebumper.h \
160161
wallet/rpcwallet.h \
161162
wallet/wallet.h \
162163
wallet/walletdb.h \
@@ -231,6 +232,7 @@ libbitcoin_wallet_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
231232
libbitcoin_wallet_a_SOURCES = \
232233
wallet/crypter.cpp \
233234
wallet/db.cpp \
235+
wallet/feebumper.cpp \
234236
wallet/rpcdump.cpp \
235237
wallet/rpcwallet.cpp \
236238
wallet/wallet.cpp \

src/wallet/feebumper.cpp

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,283 @@
1+
// Copyright (c) 2017 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include "consensus/validation.h"
6+
#include "wallet/feebumper.h"
7+
#include "wallet/wallet.h"
8+
#include "policy/policy.h"
9+
#include "policy/rbf.h"
10+
#include "validation.h" //for mempool access
11+
#include "txmempool.h"
12+
#include "utilmoneystr.h"
13+
#include "util.h"
14+
#include "net.h"
15+
16+
// Calculate the size of the transaction assuming all signatures are max size
17+
// Use DummySignatureCreator, which inserts 72 byte signatures everywhere.
18+
// TODO: re-use this in CWallet::CreateTransaction (right now
19+
// CreateTransaction uses the constructed dummy-signed tx to do a priority
20+
// calculation, but we should be able to refactor after priority is removed).
21+
// NOTE: this requires that all inputs must be in mapWallet (eg the tx should
22+
// be IsAllFromMe).
23+
int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *pWallet)
24+
{
25+
CMutableTransaction txNew(tx);
26+
std::vector<std::pair<const CWalletTx *, unsigned int>> vCoins;
27+
// Look up the inputs. We should have already checked that this transaction
28+
// IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our
29+
// wallet, with a valid index into the vout array.
30+
for (auto& input : tx.vin) {
31+
const auto mi = pWallet->mapWallet.find(input.prevout.hash);
32+
assert(mi != pWallet->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size());
33+
vCoins.emplace_back(&(mi->second), input.prevout.n);
34+
}
35+
if (!pWallet->DummySignTx(txNew, vCoins)) {
36+
// This should never happen, because IsAllFromMe(ISMINE_SPENDABLE)
37+
// implies that we can sign for every input.
38+
return -1;
39+
}
40+
return GetVirtualTransactionSize(txNew);
41+
}
42+
43+
CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool specifiedConfirmTarget, CAmount totalFee, bool newTxReplaceable)
44+
:
45+
txid(std::move(txidIn)),
46+
nOldFee(0),
47+
nNewFee(0)
48+
{
49+
vErrors.clear();
50+
bumpedTxid.SetNull();
51+
AssertLockHeld(pWallet->cs_wallet);
52+
if (!pWallet->mapWallet.count(txid)) {
53+
vErrors.push_back("Invalid or non-wallet transaction id");
54+
currentResult = BumpFeeResult::INVALID_ADDRESS_OR_KEY;
55+
return;
56+
}
57+
auto it = pWallet->mapWallet.find(txid);
58+
const CWalletTx& wtx = it->second;
59+
60+
if (pWallet->HasWalletSpend(txid)) {
61+
vErrors.push_back("Transaction has descendants in the wallet");
62+
currentResult = BumpFeeResult::INVALID_PARAMETER;
63+
return;
64+
}
65+
66+
{
67+
LOCK(mempool.cs);
68+
auto it_mp = mempool.mapTx.find(txid);
69+
if (it_mp != mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1) {
70+
vErrors.push_back("Transaction has descendants in the mempool");
71+
currentResult = BumpFeeResult::INVALID_PARAMETER;
72+
return;
73+
}
74+
}
75+
76+
if (wtx.GetDepthInMainChain() != 0) {
77+
vErrors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
78+
currentResult = BumpFeeResult::WALLET_ERROR;
79+
return;
80+
}
81+
82+
if (!SignalsOptInRBF(wtx)) {
83+
vErrors.push_back("Transaction is not BIP 125 replaceable");
84+
currentResult = BumpFeeResult::WALLET_ERROR;
85+
return;
86+
}
87+
88+
if (wtx.mapValue.count("replaced_by_txid")) {
89+
vErrors.push_back(strprintf("Cannot bump transaction %s which was already bumped by transaction %s", txid.ToString(), wtx.mapValue.at("replaced_by_txid")));
90+
currentResult = BumpFeeResult::WALLET_ERROR;
91+
return;
92+
}
93+
94+
// check that original tx consists entirely of our inputs
95+
// 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)
96+
if (!pWallet->IsAllFromMe(wtx, ISMINE_SPENDABLE)) {
97+
vErrors.push_back("Transaction contains inputs that don't belong to this wallet");
98+
currentResult = BumpFeeResult::WALLET_ERROR;
99+
return;
100+
}
101+
102+
// figure out which output was change
103+
// if there was no change output or multiple change outputs, fail
104+
int nOutput = -1;
105+
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
106+
if (pWallet->IsChange(wtx.tx->vout[i])) {
107+
if (nOutput != -1) {
108+
vErrors.push_back("Transaction has multiple change outputs");
109+
currentResult = BumpFeeResult::WALLET_ERROR;
110+
return;
111+
}
112+
nOutput = i;
113+
}
114+
}
115+
if (nOutput == -1) {
116+
vErrors.push_back("Transaction does not have a change output");
117+
currentResult = BumpFeeResult::WALLET_ERROR;
118+
return;
119+
}
120+
121+
// Calculate the expected size of the new transaction.
122+
int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
123+
const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx, pWallet);
124+
if (maxNewTxSize < 0) {
125+
vErrors.push_back("Transaction contains inputs that cannot be signed");
126+
currentResult = BumpFeeResult::INVALID_ADDRESS_OR_KEY;
127+
return;
128+
}
129+
130+
// calculate the old fee and fee-rate
131+
nOldFee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut();
132+
CFeeRate nOldFeeRate(nOldFee, txSize);
133+
CFeeRate nNewFeeRate;
134+
// The wallet uses a conservative WALLET_INCREMENTAL_RELAY_FEE value to
135+
// future proof against changes to network wide policy for incremental relay
136+
// fee that our node may not be aware of.
137+
CFeeRate walletIncrementalRelayFee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE);
138+
if (::incrementalRelayFee > walletIncrementalRelayFee) {
139+
walletIncrementalRelayFee = ::incrementalRelayFee;
140+
}
141+
142+
if (totalFee > 0) {
143+
CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + ::incrementalRelayFee.GetFee(maxNewTxSize);
144+
if (totalFee < minTotalFee) {
145+
vErrors.push_back(strprintf("Insufficient totalFee, must be at least %s (oldFee %s + incrementalFee %s)",
146+
FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxNewTxSize)), FormatMoney(::incrementalRelayFee.GetFee(maxNewTxSize))));
147+
currentResult = BumpFeeResult::INVALID_PARAMETER;
148+
return;
149+
}
150+
CAmount requiredFee = CWallet::GetRequiredFee(maxNewTxSize);
151+
if (totalFee < requiredFee) {
152+
vErrors.push_back(strprintf("Insufficient totalFee (cannot be less than required fee %s)",
153+
FormatMoney(requiredFee)));
154+
currentResult = BumpFeeResult::INVALID_PARAMETER;
155+
return;
156+
}
157+
nNewFee = totalFee;
158+
nNewFeeRate = CFeeRate(totalFee, maxNewTxSize);
159+
} else {
160+
// if user specified a confirm target then don't consider any global payTxFee
161+
if (specifiedConfirmTarget) {
162+
nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, CAmount(0));
163+
}
164+
// otherwise use the regular wallet logic to select payTxFee or default confirm target
165+
else {
166+
nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool);
167+
}
168+
169+
nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize);
170+
171+
// New fee rate must be at least old rate + minimum incremental relay rate
172+
// walletIncrementalRelayFee.GetFeePerK() should be exact, because it's initialized
173+
// in that unit (fee per kb).
174+
// However, nOldFeeRate is a calculated value from the tx fee/size, so
175+
// add 1 satoshi to the result, because it may have been rounded down.
176+
if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK()) {
177+
nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK());
178+
nNewFee = nNewFeeRate.GetFee(maxNewTxSize);
179+
}
180+
}
181+
182+
// Check that in all cases the new fee doesn't violate maxTxFee
183+
if (nNewFee > maxTxFee) {
184+
vErrors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
185+
FormatMoney(nNewFee), FormatMoney(maxTxFee)));
186+
currentResult = BumpFeeResult::WALLET_ERROR;
187+
return;
188+
}
189+
190+
// check that fee rate is higher than mempool's minimum fee
191+
// (no point in bumping fee if we know that the new tx won't be accepted to the mempool)
192+
// This may occur if the user set TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps,
193+
// in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
194+
// moment earlier. In this case, we report an error to the user, who may use totalFee to make an adjustment.
195+
CFeeRate minMempoolFeeRate = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
196+
if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
197+
vErrors.push_back(strprintf("New fee rate (%s) is less than the minimum fee rate (%s) to get into the mempool. totalFee value should to be at least %s or settxfee value should be at least %s to add transaction.", FormatMoney(nNewFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFeePerK()), FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize)), FormatMoney(minMempoolFeeRate.GetFeePerK())));
198+
currentResult = BumpFeeResult::WALLET_ERROR;
199+
return;
200+
}
201+
202+
// Now modify the output to increase the fee.
203+
// If the output is not large enough to pay the fee, fail.
204+
CAmount nDelta = nNewFee - nOldFee;
205+
assert(nDelta > 0);
206+
mtx = *wtx.tx;
207+
CTxOut* poutput = &(mtx.vout[nOutput]);
208+
if (poutput->nValue < nDelta) {
209+
vErrors.push_back("Change output is too small to bump the fee");
210+
currentResult = BumpFeeResult::WALLET_ERROR;
211+
return;
212+
}
213+
214+
// If the output would become dust, discard it (converting the dust to fee)
215+
poutput->nValue -= nDelta;
216+
if (poutput->nValue <= poutput->GetDustThreshold(::dustRelayFee)) {
217+
LogPrint(BCLog::RPC, "Bumping fee and discarding dust output\n");
218+
nNewFee += poutput->nValue;
219+
mtx.vout.erase(mtx.vout.begin() + nOutput);
220+
}
221+
222+
// Mark new tx not replaceable, if requested.
223+
if (!newTxReplaceable) {
224+
for (auto& input : mtx.vin) {
225+
if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe;
226+
}
227+
}
228+
229+
currentResult = BumpFeeResult::OK;
230+
}
231+
232+
bool CFeeBumper::signTransaction(CWallet *pWallet)
233+
{
234+
return pWallet->SignTransaction(mtx);
235+
}
236+
237+
bool CFeeBumper::commit(CWallet *pWallet)
238+
{
239+
AssertLockHeld(pWallet->cs_wallet);
240+
if (!vErrors.empty() || currentResult != BumpFeeResult::OK) {
241+
return false;
242+
}
243+
if (txid.IsNull() || !pWallet->mapWallet.count(txid)) {
244+
vErrors.push_back("Invalid or non-wallet transaction id");
245+
currentResult = BumpFeeResult::MISC_ERROR;
246+
return false;
247+
}
248+
CWalletTx& oldWtx = pWallet->mapWallet[txid];
249+
250+
CWalletTx wtxBumped(pWallet, MakeTransactionRef(std::move(mtx)));
251+
// commit/broadcast the tx
252+
CReserveKey reservekey(pWallet);
253+
wtxBumped.mapValue = oldWtx.mapValue;
254+
wtxBumped.mapValue["replaces_txid"] = oldWtx.GetHash().ToString();
255+
wtxBumped.vOrderForm = oldWtx.vOrderForm;
256+
wtxBumped.strFromAccount = oldWtx.strFromAccount;
257+
wtxBumped.fTimeReceivedIsTxTime = true;
258+
wtxBumped.fFromMe = true;
259+
CValidationState state;
260+
if (!pWallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) {
261+
// NOTE: CommitTransaction never returns false, so this should never happen.
262+
vErrors.push_back(strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason()));
263+
return false;
264+
}
265+
266+
bumpedTxid = wtxBumped.GetHash();
267+
if (state.IsInvalid()) {
268+
// This can happen if the mempool rejected the transaction. Report
269+
// what happened in the "errors" response.
270+
vErrors.push_back(strprintf("Error: The transaction was rejected: %s", FormatStateMessage(state)));
271+
}
272+
273+
// mark the original tx as bumped
274+
if (!pWallet->MarkReplaced(oldWtx.GetHash(), wtxBumped.GetHash())) {
275+
// TODO: see if JSON-RPC has a standard way of returning a response
276+
// along with an exception. It would be good to return information about
277+
// wtxBumped to the caller even if marking the original transaction
278+
// replaced does not succeed for some reason.
279+
vErrors.push_back("Error: Created new bumpfee transaction but could not mark the original transaction as replaced.");
280+
}
281+
return true;
282+
}
283+

src/wallet/feebumper.h

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright (c) 2017 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_WALLET_FEEBUMPER_H
6+
#define BITCOIN_WALLET_FEEBUMPER_H
7+
8+
#include <primitives/transaction.h>
9+
10+
class CWallet;
11+
class uint256;
12+
13+
enum class BumpFeeResult
14+
{
15+
OK,
16+
INVALID_ADDRESS_OR_KEY,
17+
INVALID_REQUEST,
18+
INVALID_PARAMETER,
19+
WALLET_ERROR,
20+
MISC_ERROR,
21+
};
22+
23+
class CFeeBumper
24+
{
25+
public:
26+
CFeeBumper(const CWallet *pWalletIn, const uint256 txidIn, int newConfirmTarget, bool specifiedConfirmTarget, CAmount totalFee, bool newTxReplaceable);
27+
BumpFeeResult getResult() const { return currentResult; }
28+
const std::vector<std::string>& getErrors() const { return vErrors; }
29+
CAmount getOldFee() const { return nOldFee; }
30+
CAmount getNewFee() const { return nNewFee; }
31+
uint256 getBumpedTxId() const { return bumpedTxid; }
32+
33+
/* signs the new transaction,
34+
* returns false if the tx couldn't be found or if it was
35+
* improssible to create the signature(s)
36+
*/
37+
bool signTransaction(CWallet *pWallet);
38+
39+
/* commits the fee bump,
40+
* returns true, in case of CWallet::CommitTransaction was successful
41+
* but, eventually sets vErrors if the tx could not be added to the mempool (will try later)
42+
* or if the old transaction could not be marked as replaced
43+
*/
44+
bool commit(CWallet *pWalletNonConst);
45+
46+
private:
47+
const uint256 txid;
48+
uint256 bumpedTxid;
49+
CMutableTransaction mtx;
50+
std::vector<std::string> vErrors;
51+
BumpFeeResult currentResult;
52+
CAmount nOldFee;
53+
CAmount nNewFee;
54+
};
55+
56+
#endif // BITCOIN_WALLET_FEEBUMPER_H

0 commit comments

Comments
 (0)