Skip to content

Remove leftover bumpfee code (RBF) #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion qa/pull-tester/rpc-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@
'signmessages.py',
'nulldummy.py',
'import-rescan.py',
'bumpfee.py',
'rpcnamedargs.py',
'listsinceblock.py',
'p2p-leaktests.py',
Expand Down
326 changes: 0 additions & 326 deletions qa/rpc-tests/bumpfee.py

This file was deleted.

1 change: 0 additions & 1 deletion src/rpc/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "setnetworkactive", 0, "state" },
{ "getmempoolancestors", 1, "verbose" },
{ "getmempooldescendants", 1, "verbose" },
{ "bumpfee", 1, "options" },
// Echo with conversion (For testing only)
{ "echojson", 0, "arg0" },
{ "echojson", 1, "arg1" },
Expand Down
10 changes: 5 additions & 5 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -660,11 +660,11 @@ UniValue getbalance(const JSONRPCRequest& request)
" the balance associated with all wallet keys regardless of account.\n"
" When this option is specified, it calculates the balance in a different\n"
" way than when it is not specified, and which can count spends twice when\n"
" there are conflicting pending transactions (such as those created by\n"
" the bumpfee command), temporarily resulting in low or even negative\n"
" balances. In general, account balance calculation is not considered\n"
" reliable and has resulted in confusing outcomes, so it is recommended to\n"
" avoid passing this argument.\n"
" there are conflicting pending transactions temporarily resulting in low\n"
" or even negative balances.\n"
" In general, account balance calculation is not considered reliable and\n"
" has resulted in confusing outcomes, so it is recommended to avoid passing\n"
" this argument.\n"
"2. minconf (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n"
"3. include_watchonly (bool, optional, default=false) Also include balance in watch-only addresses (see 'importaddress')\n"
"\nResult:\n"
Expand Down
25 changes: 7 additions & 18 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2047,24 +2047,10 @@ void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const
if (nDepth == 0 && !pcoin->InMempool())
continue;

// We should not consider coins from transactions that are replacing
// other transactions.
//
// Example: There is a transaction A which is replaced by bumpfee
// transaction B. In this case, we want to prevent creation of
// a transaction B' which spends an output of B.
//
// Reason: If transaction A were initially confirmed, transactions B
// and B' would no longer be valid, so the user would have to create
// a new transaction C to replace B'. However, in the case of a
// one-block reorg, transactions B' and C might BOTH be accepted,
// when the user only wanted one of them. Specifically, there could
// be a 1-block reorg away from the chain where transactions A and C
// were accepted to another chain where B, B', and C were all
// accepted.
if (nDepth == 0 && fOnlyConfirmed && pcoin->mapValue.count("replaces_txid")) {
continue;
}
// Bitcoin-ABC: Removed check that prevents consideration of coins
// from transactions that are replacing other transactions.
// This check based on pcoin->mapValue.count("replaces_txid")
// which was not being set anywhere.

// Similarly, we should not consider coins from transactions that
// have been replaced. In the example above, we would want to prevent
Expand All @@ -2074,6 +2060,9 @@ void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const
// intending to replace A', but potentially resulting in a scenario
// where A, A', and D could all be accepted (instead of just B and
// D, or just A and A' like the user would want).

// Bitcoin-ABC: retained this check as 'replaced_by_txid' is
// still set in the wallet code.
if (nDepth == 0 && fOnlyConfirmed && pcoin->mapValue.count("replaced_by_txid")) {
continue;
}
Expand Down