diff --git a/doc/release-notes.md b/doc/release-notes.md
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -5,24 +5,3 @@
This release includes the following features and fixes:
-
-Command-line options
---------------------
-
-- The `-debug=db` logging category has been renamed to `-debug=walletdb`, to distinguish it from `coindb`.
- `-debug=db` has been deprecated and will be removed in a next release.
-
-Low-level RPC Changes
----------------------
-
-- The RPC gettransaction, listtransactions and listsinceblock responses now also
-includes the height of the block that contains the wallet transaction, if any.
-
-Deprecated or removed RPCs
---------------------------
-
-- The `getaddressinfo` RPC `labels` field now returns an array of label name
- strings. Previously, it returned an array of JSON objects containing `name` and
- `purpose` key/value pairs, which is now deprecated and will be removed in a future
- release. To re-enable the previous behavior, launch bitcoind with
- `-deprecatedrpc=labelspurpose`.
diff --git a/doc/release-notes/release-notes-.md b/doc/release-notes/release-notes-.md
--- a/doc/release-notes/release-notes-.md
+++ b/doc/release-notes/release-notes-.md
@@ -6,7 +6,23 @@
This release includes the following features and fixes:
-Wallet
-------
+Command-line options
+--------------------
-- The way that output trust was computed has been fixed, which impacts confirmed/unconfirmed balance status and coin selection.
+- The `-debug=db` logging category has been renamed to `-debug=walletdb`, to distinguish it from `coindb`.
+ `-debug=db` has been deprecated and will be removed in a next release.
+
+Low-level RPC Changes
+---------------------
+
+- The RPC gettransaction, listtransactions and listsinceblock responses now also
+includes the height of the block that contains the wallet transaction, if any.
+
+Deprecated or removed RPCs
+--------------------------
+
+- The `getaddressinfo` RPC `labels` field now returns an array of label name
+ strings. Previously, it returned an array of JSON objects containing `name` and
+ `purpose` key/value pairs, which is now deprecated and will be removed in a future
+ release. To re-enable the previous behavior, launch bitcoind with
+ `-deprecatedrpc=labelspurpose`.
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -60,8 +60,7 @@
}
static void add_coin(CWallet &wallet, const Amount nValue, int nAge = 6 * 24,
- bool fIsFromMe = false, int nInput = 0,
- bool spendable = false) {
+ bool fIsFromMe = false, int nInput = 0) {
balance += nValue;
static int nextLockTime = 0;
CMutableTransaction tx;
@@ -69,12 +68,6 @@
tx.nLockTime = nextLockTime++;
tx.vout.resize(nInput + 1);
tx.vout[nInput].nValue = nValue;
- if (spendable) {
- CTxDestination dest;
- std::string error;
- assert(wallet.GetNewDestination(OutputType::LEGACY, "", dest, error));
- tx.vout[nInput].scriptPubKey = GetScriptForDestination(dest);
- }
if (fIsFromMe) {
// IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if
// vin.empty(), so stop vin being empty, and cache a non-zero Debit to
@@ -294,42 +287,19 @@
1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet,
coin_selection_params_bnb, bnb_used));
- // Test fees subtracted from output:
- empty_wallet();
- add_coin(m_wallet, 1 * CENT);
- vCoins.at(0).nInputBytes = 40;
- BOOST_CHECK(!m_wallet.SelectCoinsMinConf(
- 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet,
- coin_selection_params_bnb, bnb_used));
- coin_selection_params_bnb.m_subtract_fee_outputs = true;
- BOOST_CHECK(m_wallet.SelectCoinsMinConf(
- 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet,
- coin_selection_params_bnb, bnb_used));
- BOOST_CHECK_EQUAL(nValueRet, 1 * CENT);
-
- // Make sure that can use BnB when there are preset inputs
+ // Make sure that we aren't using BnB when there are preset inputs
empty_wallet();
- {
- auto wallet =
- std::make_unique(Params(), m_chain.get(), WalletLocation(),
- WalletDatabase::CreateMock());
- bool firstRun;
- wallet->LoadWallet(firstRun);
- LOCK(wallet->cs_wallet);
- add_coin(*wallet, 5 * CENT, 6 * 24, false, 0, true);
- add_coin(*wallet, 3 * CENT, 6 * 24, false, 0, true);
- add_coin(*wallet, 2 * CENT, 6 * 24, false, 0, true);
- CCoinControl coin_control;
- coin_control.fAllowOtherInputs = true;
- coin_control.Select(
- COutPoint(vCoins.at(0).tx->GetId(), vCoins.at(0).i));
- coin_selection_params_bnb.effective_fee = CFeeRate(Amount::zero());
- BOOST_CHECK(wallet->SelectCoins(vCoins, 10 * CENT, setCoinsRet,
- nValueRet, coin_control,
- coin_selection_params_bnb, bnb_used));
- BOOST_CHECK(bnb_used);
- BOOST_CHECK(coin_selection_params_bnb.use_bnb);
- }
+ add_coin(m_wallet, 5 * CENT);
+ add_coin(m_wallet, 3 * CENT);
+ add_coin(m_wallet, 2 * CENT);
+ CCoinControl coin_control;
+ coin_control.fAllowOtherInputs = true;
+ coin_control.Select(COutPoint(vCoins.at(0).tx->GetId(), vCoins.at(0).i));
+ BOOST_CHECK(m_wallet.SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet,
+ coin_control, coin_selection_params_bnb,
+ bnb_used));
+ BOOST_CHECK(!bnb_used);
+ BOOST_CHECK(!coin_selection_params_bnb.use_bnb);
}
BOOST_AUTO_TEST_CASE(knapsack_solver_test) {
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -619,8 +619,6 @@
size_t change_spend_size = 0;
CFeeRate effective_fee = CFeeRate(Amount::zero());
size_t tx_noinputs_size = 0;
- //! Indicate that we are subtracting the fee from outputs
- bool m_subtract_fee_outputs = false;
CoinSelectionParams(bool use_bnb_, size_t change_output_size_,
size_t change_spend_size_, CFeeRate effective_fee_,
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2534,11 +2534,7 @@
coin.m_input_bytes < 0
? Amount::zero()
: long_term_feerate.GetFee(coin.m_input_bytes);
- if (coin_selection_params.m_subtract_fee_outputs) {
- group.effective_value += coin.txout.nValue;
- } else {
- group.effective_value += effective_value;
- }
+ group.effective_value += effective_value;
++it;
} else {
it = group.Discard(coin);
@@ -2574,14 +2570,13 @@
CoinSelectionParams &coin_selection_params,
bool &bnb_used) const {
std::vector vCoins(vAvailableCoins);
- Amount value_to_select = nTargetValue;
-
- // Default to bnb was not used. If we use it, we set it later
- bnb_used = false;
// coin control -> return all selected outputs (we want all selected to go
// into the transaction for sure)
if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) {
+ // We didn't use BnB here, so set it to false.
+ bnb_used = false;
+
for (const COutput &out : vCoins) {
if (!out.fSpendable) {
continue;
@@ -2602,34 +2597,27 @@
coin_control.ListSelected(vPresetInputs);
for (const COutPoint &outpoint : vPresetInputs) {
+ // For now, don't use BnB if preset inputs are selected. TODO: Enable
+ // this later
+ bnb_used = false;
+ coin_selection_params.use_bnb = false;
+
std::map::const_iterator it =
mapWallet.find(outpoint.GetTxId());
- if (it != mapWallet.end()) {
- const CWalletTx &wtx = it->second;
- // Clearly invalid input, fail
- if (wtx.tx->vout.size() <= outpoint.GetN()) {
- return false;
- }
- // Just to calculate the marginal byte size
- CInputCoin coin(wtx.tx, outpoint.GetN(),
- wtx.GetSpendSize(outpoint.GetN(), false));
- nValueFromPresetInputs += coin.txout.nValue;
- if (coin.m_input_bytes <= 0) {
- // Not solvable, can't estimate size for fee
- return false;
- }
- coin.effective_value =
- coin.txout.nValue -
- coin_selection_params.effective_fee.GetFee(coin.m_input_bytes);
- if (coin_selection_params.use_bnb) {
- value_to_select -= coin.effective_value;
- } else {
- value_to_select -= coin.txout.nValue;
- }
- setPresetCoins.insert(coin);
- } else {
- return false; // TODO: Allow non-wallet inputs
+ if (it == mapWallet.end()) {
+ // TODO: Allow non-wallet inputs
+ return false;
+ }
+
+ const CWalletTx &wtx = it->second;
+ // Clearly invalid input, fail.
+ if (wtx.tx->vout.size() <= outpoint.GetN()) {
+ return false;
}
+
+ // Just to calculate the marginal byte size
+ nValueFromPresetInputs += wtx.tx->vout[outpoint.GetN()].nValue;
+ setPresetCoins.insert(CInputCoin(wtx.tx, outpoint.GetN()));
}
// Remove preset inputs from vCoins
@@ -2662,39 +2650,39 @@
"-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
bool res =
- value_to_select <= Amount::zero() ||
- SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0),
- groups, setCoinsRet, nValueRet,
- coin_selection_params, bnb_used) ||
- SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0),
- groups, setCoinsRet, nValueRet,
- coin_selection_params, bnb_used) ||
+ nTargetValue <= nValueFromPresetInputs ||
+ SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs,
+ CoinEligibilityFilter(1, 6, 0), groups, setCoinsRet,
+ nValueRet, coin_selection_params, bnb_used) ||
+ SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs,
+ CoinEligibilityFilter(1, 1, 0), groups, setCoinsRet,
+ nValueRet, coin_selection_params, bnb_used) ||
(m_spend_zero_conf_change &&
- SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2),
- groups, setCoinsRet, nValueRet,
- coin_selection_params, bnb_used)) ||
+ SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs,
+ CoinEligibilityFilter(0, 1, 2), groups, setCoinsRet,
+ nValueRet, coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change &&
SelectCoinsMinConf(
- value_to_select,
- CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors / 3),
- std::min((size_t)4, max_descendants / 3)),
+ nTargetValue - nValueFromPresetInputs,
+ CoinEligibilityFilter(0, 1, std::min(4, max_ancestors / 3),
+ std::min(4, max_descendants / 3)),
groups, setCoinsRet, nValueRet, coin_selection_params,
bnb_used)) ||
(m_spend_zero_conf_change &&
- SelectCoinsMinConf(value_to_select,
+ SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs,
CoinEligibilityFilter(0, 1, max_ancestors / 2,
max_descendants / 2),
groups, setCoinsRet, nValueRet,
coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change &&
- SelectCoinsMinConf(value_to_select,
+ SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs,
CoinEligibilityFilter(0, 1, max_ancestors - 1,
max_descendants - 1),
groups, setCoinsRet, nValueRet,
coin_selection_params, bnb_used)) ||
(m_spend_zero_conf_change && !fRejectLongChains &&
SelectCoinsMinConf(
- value_to_select,
+ nTargetValue - nValueFromPresetInputs,
CoinEligibilityFilter(0, 1, std::numeric_limits::max()),
groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used));
@@ -2975,11 +2963,8 @@
// BnB selector is the only selector used when this is true.
// That should only happen on the first pass through the loop.
- coin_selection_params.use_bnb = true;
- // If we are doing subtract fee from recipient, don't use effective
- // values
- coin_selection_params.m_subtract_fee_outputs =
- nSubtractFeeFromAmount != 0;
+ // If we are doing subtract fee from recipient, then don't use BnB
+ coin_selection_params.use_bnb = nSubtractFeeFromAmount == 0;
// Start with no fee and loop until there is enough fee
while (true) {
nChangePosInOut = nChangePosRequest;
@@ -2992,12 +2977,9 @@
nValueToSelect += nFeeRet;
}
- // vouts to the payees
- if (!coin_selection_params.m_subtract_fee_outputs) {
- // Static size overhead + outputs vsize. 4 nVersion, 4
- // nLocktime, 1 input count, 1 output count
- coin_selection_params.tx_noinputs_size = 10;
- }
+ // Static size overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1
+ // input count, 1 output count
+ coin_selection_params.tx_noinputs_size = 10;
// vouts to the payees
for (const auto &recipient : vecSend) {
CTxOut txout(recipient.nAmount, recipient.scriptPubKey);
@@ -3017,10 +2999,8 @@
// Include the fee cost for outputs. Note this is only used for
// BnB right now
- if (!coin_selection_params.m_subtract_fee_outputs) {
- coin_selection_params.tx_noinputs_size +=
- ::GetSerializeSize(txout, PROTOCOL_VERSION);
- }
+ coin_selection_params.tx_noinputs_size +=
+ ::GetSerializeSize(txout, PROTOCOL_VERSION);
if (IsDust(txout, chain().relayDustFee())) {
if (recipient.fSubtractFeeFromAmount &&
@@ -3043,7 +3023,7 @@
}
// Choose coins to use
- bool bnb_used = false;
+ bool bnb_used;
if (pick_new_inputs) {
nValueIn = Amount::zero();
setCoins.clear();
diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
--- a/test/functional/rpc_fundrawtransaction.py
+++ b/test/functional/rpc_fundrawtransaction.py
@@ -90,7 +90,6 @@
self.test_option_feerate()
self.test_address_reuse()
self.test_option_subtract_fee_from_outputs()
- self.test_subtract_fee_with_presets()
def test_change_position(self):
# ensure that setting changePosition in fundraw with an exact match is
@@ -849,21 +848,6 @@
# the total subtracted from the outputs is equal to the fee
assert_equal(share[0] + share[2] + share[3], result[0]['fee'])
- def test_subtract_fee_with_presets(self):
- self.log.info(
- "Test fundrawtxn subtract fee from outputs with preset inputs that are sufficient")
-
- addr = self.nodes[0].getnewaddress()
- txid = self.nodes[0].sendtoaddress(addr, 10)
- vout = find_vout_for_address(self.nodes[0], txid, addr)
-
- rawtx = self.nodes[0].createrawtransaction([{'txid': txid, 'vout': vout}], [
- {self.nodes[0].getnewaddress(): 5}])
- fundedtx = self.nodes[0].fundrawtransaction(
- rawtx, {'subtractFeeFromOutputs': [0]})
- signedtx = self.nodes[0].signrawtransactionwithwallet(fundedtx['hex'])
- self.nodes[0].sendrawtransaction(signedtx['hex'])
-
if __name__ == '__main__':
RawTransactionsTest().main()