Page MenuHomePhabricator
Files F13115767

D7807.diff
No OneTemporary
Actions

D7807.diff
View Options

diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp
--- a/src/dummywallet.cpp
+++ b/src/dummywallet.cpp
@@ -31,10 +31,10 @@
void DummyWalletInit::AddWalletOptions() const {
std::vector<std::string> opts = {
"-avoidpartialspends", "-disablewallet", "-fallbackfee=<amt>",
- "-keypool=<n>", "-maxtxfee=<amt>", "-mintxfee=<amt>", "-paytxfee=<amt>",
- "-rescan", "-salvagewallet", "-spendzeroconfchange", "-upgradewallet",
- "-wallet=<path>", "-walletbroadcast", "-walletdir=<dir>",
- "-walletnotify=<cmd>", "-zapwallettxes=<mode>",
+ "-keypool=<n>", "-maxapsfee=<n>", "-maxtxfee=<amt>", "-mintxfee=<amt>",
+ "-paytxfee=<amt>", "-rescan", "-salvagewallet", "-spendzeroconfchange",
+ "-upgradewallet", "-wallet=<path>", "-walletbroadcast",
+ "-walletdir=<dir>", "-walletnotify=<cmd>", "-zapwallettxes=<mode>",
// Wallet debug options
"-dblogsize=<n>", "-flushwallet", "-privdb", "-walletrejectlongchains"};
gArgs.AddHiddenArgs(opts);
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -61,6 +61,13 @@
strprintf("Set key pool size to <n> (default: %u)",
DEFAULT_KEYPOOL_SIZE),
ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
+ gArgs.AddArg(
+ "-maxapsfee=<n>",
+ strprintf(
+ "Spend up to this amount in additional (absolute) fees (in %s) if "
+ "it allows the use of partial spend avoidance (default: %s)",
+ CURRENCY_UNIT, FormatMoney(DEFAULT_MAX_AVOIDPARTIALSPEND_FEE)),
+ ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
gArgs.AddArg(
"-maxtxfee=<amt>",
strprintf("Maximum total fees (in %s) to use in a single wallet "
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -16,6 +16,7 @@
#include <ui_interface.h>
#include <util/strencodings.h>
#include <util/system.h>
+#include <util/translation.h>
#include <validationinterface.h>
#include <wallet/coinselection.h>
#include <wallet/crypter.h>
@@ -76,6 +77,19 @@
static const Amount DEFAULT_FALLBACK_FEE = Amount::zero();
//! -mintxfee default
static const Amount DEFAULT_TRANSACTION_MINFEE_PER_KB = 1000 * SATOSHI;
+/**
+ * maximum fee increase allowed to do partial spend avoidance, even for nodes
+ * with this feature disabled by default
+ *
+ * A value of -1 disables this feature completely.
+ * A value of 0 (current default) means to attempt to do partial spend
+ * avoidance, and use its results if the fees remain *unchanged* A value > 0
+ * means to do partial spend avoidance if the fee difference against a regular
+ * coin selection instance is in the range [0..value].
+ */
+static const Amount DEFAULT_MAX_AVOIDPARTIALSPEND_FEE = Amount::zero();
+//! discourage APS fee higher than this amount
+constexpr Amount HIGH_APS_FEE{COIN / 10000};
//! minimum recommended increment for BIP 125 replacement txs
static const Amount WALLET_INCREMENTAL_RELAY_FEE(5000 * SATOSHI);
//! Default for -spendzeroconfchange
@@ -770,6 +784,12 @@
*/
int m_last_block_processed_height GUARDED_BY(cs_wallet) = -1;
+ bool CreateTransactionInternal(interfaces::Chain::Lock &locked_chain,
+ const std::vector<CRecipient> &vecSend,
+ CTransactionRef &tx, Amount &nFeeRet,
+ int &nChangePosInOut, bilingual_str &error,
+ const CCoinControl &coin_control, bool sign);
+
public:
const CChainParams &chainParams;
/*
@@ -1123,6 +1143,8 @@
* -fallbackfee
*/
CFeeRate m_fallback_fee{DEFAULT_FALLBACK_FEE};
+ //! note: this is absolute fee, not fee rate
+ Amount m_max_aps_fee{DEFAULT_MAX_AVOIDPARTIALSPEND_FEE};
OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE};
OutputType m_default_change_type{DEFAULT_CHANGE_TYPE};
/**
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2880,15 +2880,17 @@
return m_default_address_type;
}
-bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
- const std::vector<CRecipient> &vecSend,
- CTransactionRef &tx, Amount &nFeeRet,
- int &nChangePosInOut, bilingual_str &error,
- const CCoinControl &coinControl, bool sign) {
+bool CWallet::CreateTransactionInternal(interfaces::Chain::Lock &locked_chainIn,
+ const std::vector<CRecipient> &vecSend,
+ CTransactionRef &tx, Amount &nFeeRet,
+ int &nChangePosInOut,
+ bilingual_str &error,
+ const CCoinControl &coin_control,
+ bool sign) {
Amount nValue = Amount::zero();
const OutputType change_type = TransactionChangeType(
- coinControl.m_change_type ? *coinControl.m_change_type
- : m_default_change_type,
+ coin_control.m_change_type ? *coin_control.m_change_type
+ : m_default_change_type,
vecSend);
ReserveDestination reservedest(this, change_type);
int nChangePosRequest = nChangePosInOut;
@@ -2921,7 +2923,7 @@
LOCK(cs_wallet);
std::vector<COutput> vAvailableCoins;
- AvailableCoins(*locked_chain, vAvailableCoins, true, &coinControl);
+ AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control);
// Parameters for coin selection, init with dummy
CoinSelectionParams coin_selection_params;
@@ -2931,8 +2933,8 @@
CScript scriptChange;
// coin control: send change to custom address
- if (!boost::get<CNoDestination>(&coinControl.destChange)) {
- scriptChange = GetScriptForDestination(coinControl.destChange);
+ if (!boost::get<CNoDestination>(&coin_control.destChange)) {
+ scriptChange = GetScriptForDestination(coin_control.destChange);
// no coin control: send change to newly generated address
} else {
@@ -2965,7 +2967,7 @@
GetSerializeSize(change_prototype_txout);
// Get the fee rate to use effective values in coin selection
- CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coinControl);
+ CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coin_control);
nFeeRet = Amount::zero();
bool pick_new_inputs = true;
@@ -3042,7 +3044,7 @@
this);
coin_selection_params.effective_fee = nFeeRateNeeded;
if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins,
- nValueIn, coinControl, coin_selection_params,
+ nValueIn, coin_control, coin_selection_params,
bnb_used)) {
// If BnB was used, it was the first pass. No longer the
// first pass and continue loop with knapsack.
@@ -3095,13 +3097,13 @@
CTransaction txNewConst(txNew);
int nBytes = CalculateMaximumSignedTxSize(
- txNewConst, this, coinControl.fAllowWatchOnly);
+ txNewConst, this, coin_control.fAllowWatchOnly);
if (nBytes < 0) {
error = _("Signing transaction failed");
return false;
}
- Amount nFeeNeeded = GetMinimumFee(*this, nBytes, coinControl);
+ Amount nFeeNeeded = GetMinimumFee(*this, nBytes, coin_control);
if (nFeeRet >= nFeeNeeded) {
// Reduce fee to only the needed amount if possible. This
@@ -3121,7 +3123,7 @@
unsigned int tx_size_with_change =
nBytes + coin_selection_params.change_output_size + 2;
Amount fee_needed_with_change =
- GetMinimumFee(*this, tx_size_with_change, coinControl);
+ GetMinimumFee(*this, tx_size_with_change, coin_control);
Amount minimum_value_for_change = GetDustThreshold(
change_prototype_txout, chain().relayDustFee());
if (nFeeRet >=
@@ -3253,6 +3255,46 @@
return true;
}
+bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chain,
+ const std::vector<CRecipient> &vecSend,
+ CTransactionRef &tx, Amount &nFeeRet,
+ int &nChangePosInOut, bilingual_str &error,
+ const CCoinControl &coin_control, bool sign) {
+ int nChangePosIn = nChangePosInOut;
+ CTransactionRef tx2 = tx;
+ bool res =
+ CreateTransactionInternal(locked_chain, vecSend, tx, nFeeRet,
+ nChangePosInOut, error, coin_control, sign);
+ // try with avoidpartialspends unless it's enabled already
+ if (res &&
+ nFeeRet >
+ Amount::zero() /* 0 means non-functional fee rate estimation */
+ && m_max_aps_fee > (-1 * SATOSHI) &&
+ !coin_control.m_avoid_partial_spends) {
+ CCoinControl tmp_cc = coin_control;
+ tmp_cc.m_avoid_partial_spends = true;
+ Amount nFeeRet2;
+ int nChangePosInOut2 = nChangePosIn;
+ // fired and forgotten; if an error occurs, we discard the results
+ bilingual_str error2;
+ if (CreateTransactionInternal(locked_chain, vecSend, tx2, nFeeRet2,
+ nChangePosInOut2, error2, tmp_cc, sign)) {
+ // if fee of this alternative one is within the range of the max
+ // fee, we use this one
+ const bool use_aps = nFeeRet2 <= nFeeRet + m_max_aps_fee;
+ WalletLogPrintf(
+ "Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet,
+ nFeeRet2, use_aps ? "grouped" : "non-grouped");
+ if (use_aps) {
+ tx = tx2;
+ nFeeRet = nFeeRet2;
+ nChangePosInOut = nChangePosInOut2;
+ }
+ }
+ }
+ return res;
+}
+
void CWallet::CommitTransaction(
CTransactionRef tx, mapValue_t mapValue,
std::vector<std::pair<std::string, std::string>> orderForm) {
@@ -4172,6 +4214,23 @@
walletInstance->m_min_fee = CFeeRate(n);
}
+ if (gArgs.IsArgSet("-maxapsfee")) {
+ Amount n = Amount::zero();
+ if (gArgs.GetArg("-maxapsfee", "") == "-1") {
+ n = -1 * SATOSHI;
+ } else if (!ParseMoney(gArgs.GetArg("-maxapsfee", ""), n)) {
+ error = AmountErrMsg("maxapsfee", gArgs.GetArg("-maxapsfee", ""));
+ return nullptr;
+ }
+ if (n > HIGH_APS_FEE) {
+ warnings.push_back(
+ AmountHighWarn("-maxapsfee") + Untranslated(" ") +
+ _("This is the maximum transaction fee you pay to prioritize "
+ "partial spend avoidance over regular coin selection."));
+ }
+ walletInstance->m_max_aps_fee = n;
+ }
+
if (gArgs.IsArgSet("-fallbackfee")) {
Amount nFeePerK = Amount::zero();
if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) {
diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py
--- a/test/functional/wallet_groups.py
+++ b/test/functional/wallet_groups.py
@@ -15,8 +15,9 @@
class WalletGroupTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
- self.num_nodes = 3
- self.extra_args = [[], [], ['-avoidpartialspends']]
+ self.num_nodes = 4
+ self.extra_args = [
+ [], [], ['-avoidpartialspends'], ["-maxapsfee=0.0001"]]
self.rpc_timeout = 120
def skip_test_if_missing_module(self):
@@ -62,6 +63,52 @@
assert_approx(v[0], 0.2)
assert_approx(v[1], 1.3, 0.0001)
+ # Test 'avoid partial if warranted, even if disabled'
+ self.sync_all()
+ self.nodes[0].generate(1)
+ # Nodes 1-2 now have confirmed UTXOs (letters denote destinations):
+ # Node #1: Node #2:
+ # - A 1.0 - D0 1.0
+ # - B0 1.0 - D1 0.5
+ # - B1 0.5 - E0 1.0
+ # - C0 1.0 - E1 0.5
+ # - C1 0.5 - F ~1.3
+ # - D ~0.3
+ assert_approx(self.nodes[1].getbalance(), 4.3, 0.0001)
+ assert_approx(self.nodes[2].getbalance(), 4.3, 0.0001)
+ # Sending 1.4 btc should pick one 1.0 + one more. For node #1,
+ # this could be (A / B0 / C0) + (B1 / C1 / D). We ensure that it is
+ # B0 + B1 or C0 + C1, because this avoids partial spends while not being
+ # detrimental to transaction cost
+ txid3 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1.4)
+ tx3 = self.nodes[1].getrawtransaction(txid3, True)
+ # tx3 should have 2 inputs and 2 outputs
+ assert_equal(2, len(tx3["vin"]))
+ assert_equal(2, len(tx3["vout"]))
+ # the accumulated value should be 1.5, so the outputs should be
+ # ~0.1 and 1.4 and should come from the same destination
+ values = sorted([vout["value"] for vout in tx3["vout"]])
+ assert_approx(values[0], 0.1, 0.0001)
+ assert_approx(values[1], 1.4)
+
+ input_txids = [vin["txid"] for vin in tx3["vin"]]
+ input_addrs = [self.nodes[1].gettransaction(
+ txid)['details'][0]['address'] for txid in input_txids]
+ assert_equal(input_addrs[0], input_addrs[1])
+ # Node 2 enforces avoidpartialspends so needs no checking here
+
+ # Test wallet option maxapsfee with Node 3
+ addr_aps = self.nodes[3].getnewaddress()
+ self.nodes[0].sendtoaddress(addr_aps, 1.0)
+ self.nodes[0].sendtoaddress(addr_aps, 1.0)
+ self.nodes[0].generate(1)
+ txid4 = self.nodes[3].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
+ tx4 = self.nodes[3].getrawtransaction(txid4, True)
+ # tx4 should have 2 inputs and 2 outputs although one output would
+ # have been enough and the transaction caused higher fees
+ assert_equal(2, len(tx4["vin"]))
+ assert_equal(2, len(tx4["vout"]))
+
# Empty out node2's wallet
self.nodes[2].sendtoaddress(address=self.nodes[0].getnewaddress(
), amount=self.nodes[2].getbalance(), subtractfeefromamount=True)

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 11:59 (3 h, 31 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5187753
Default Alt Text
D7807.diff (14 KB)

Event Timeline