aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/wallet/coincontrol.cpp127
-rw-r--r--src/wallet/coincontrol.h101
-rw-r--r--src/wallet/feebumper.cpp10
-rw-r--r--src/wallet/interfaces.cpp4
-rw-r--r--src/wallet/rpc/spend.cpp50
-rw-r--r--src/wallet/spend.cpp164
-rw-r--r--src/wallet/spend.h8
-rw-r--r--src/wallet/test/coinselector_tests.cpp2
-rw-r--r--src/wallet/test/fuzz/coincontrol.cpp7
-rw-r--r--src/wallet/test/fuzz/notifications.cpp3
-rw-r--r--src/wallet/test/spend_tests.cpp3
-rw-r--r--src/wallet/test/wallet_tests.cpp3
12 files changed, 315 insertions, 167 deletions
diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp
index 2087119db9..873c5ab383 100644
--- a/src/wallet/coincontrol.cpp
+++ b/src/wallet/coincontrol.cpp
@@ -14,69 +14,142 @@ CCoinControl::CCoinControl()
bool CCoinControl::HasSelected() const
{
- return !m_selected_inputs.empty();
+ return !m_selected.empty();
}
-bool CCoinControl::IsSelected(const COutPoint& output) const
+bool CCoinControl::IsSelected(const COutPoint& outpoint) const
{
- return m_selected_inputs.count(output) > 0;
+ return m_selected.count(outpoint) > 0;
}
-bool CCoinControl::IsExternalSelected(const COutPoint& output) const
+bool CCoinControl::IsExternalSelected(const COutPoint& outpoint) const
{
- return m_external_txouts.count(output) > 0;
+ const auto it = m_selected.find(outpoint);
+ return it != m_selected.end() && it->second.HasTxOut();
}
std::optional<CTxOut> CCoinControl::GetExternalOutput(const COutPoint& outpoint) const
{
- const auto ext_it = m_external_txouts.find(outpoint);
- if (ext_it == m_external_txouts.end()) {
+ const auto it = m_selected.find(outpoint);
+ if (it == m_selected.end() || !it->second.HasTxOut()) {
return std::nullopt;
}
+ return it->second.GetTxOut();
+}
- return std::make_optional(ext_it->second);
+PreselectedInput& CCoinControl::Select(const COutPoint& outpoint)
+{
+ auto& input = m_selected[outpoint];
+ input.SetPosition(m_selection_pos);
+ ++m_selection_pos;
+ return input;
+}
+void CCoinControl::UnSelect(const COutPoint& outpoint)
+{
+ m_selected.erase(outpoint);
}
-void CCoinControl::Select(const COutPoint& output)
+void CCoinControl::UnSelectAll()
{
- m_selected_inputs.insert(output);
+ m_selected.clear();
}
-void CCoinControl::SelectExternal(const COutPoint& outpoint, const CTxOut& txout)
+std::vector<COutPoint> CCoinControl::ListSelected() const
{
- m_selected_inputs.insert(outpoint);
- m_external_txouts.emplace(outpoint, txout);
+ std::vector<COutPoint> outpoints;
+ std::transform(m_selected.begin(), m_selected.end(), std::back_inserter(outpoints),
+ [](const std::map<COutPoint, PreselectedInput>::value_type& pair) {
+ return pair.first;
+ });
+ return outpoints;
}
-void CCoinControl::UnSelect(const COutPoint& output)
+void CCoinControl::SetInputWeight(const COutPoint& outpoint, int64_t weight)
{
- m_selected_inputs.erase(output);
+ m_selected[outpoint].SetInputWeight(weight);
}
-void CCoinControl::UnSelectAll()
+std::optional<int64_t> CCoinControl::GetInputWeight(const COutPoint& outpoint) const
{
- m_selected_inputs.clear();
+ const auto it = m_selected.find(outpoint);
+ return it != m_selected.end() ? it->second.GetInputWeight() : std::nullopt;
}
-std::vector<COutPoint> CCoinControl::ListSelected() const
+std::optional<uint32_t> CCoinControl::GetSequence(const COutPoint& outpoint) const
{
- return {m_selected_inputs.begin(), m_selected_inputs.end()};
+ const auto it = m_selected.find(outpoint);
+ return it != m_selected.end() ? it->second.GetSequence() : std::nullopt;
}
-void CCoinControl::SetInputWeight(const COutPoint& outpoint, int64_t weight)
+std::pair<std::optional<CScript>, std::optional<CScriptWitness>> CCoinControl::GetScripts(const COutPoint& outpoint) const
+{
+ const auto it = m_selected.find(outpoint);
+ return it != m_selected.end() ? m_selected.at(outpoint).GetScripts() : std::make_pair(std::nullopt, std::nullopt);
+}
+
+void PreselectedInput::SetTxOut(const CTxOut& txout)
+{
+ m_txout = txout;
+}
+
+CTxOut PreselectedInput::GetTxOut() const
+{
+ assert(m_txout.has_value());
+ return m_txout.value();
+}
+
+bool PreselectedInput::HasTxOut() const
+{
+ return m_txout.has_value();
+}
+
+void PreselectedInput::SetInputWeight(int64_t weight)
+{
+ m_weight = weight;
+}
+
+std::optional<int64_t> PreselectedInput::GetInputWeight() const
+{
+ return m_weight;
+}
+
+void PreselectedInput::SetSequence(uint32_t sequence)
+{
+ m_sequence = sequence;
+}
+
+std::optional<uint32_t> PreselectedInput::GetSequence() const
+{
+ return m_sequence;
+}
+
+void PreselectedInput::SetScriptSig(const CScript& script)
+{
+ m_script_sig = script;
+}
+
+void PreselectedInput::SetScriptWitness(const CScriptWitness& script_wit)
+{
+ m_script_witness = script_wit;
+}
+
+bool PreselectedInput::HasScripts() const
+{
+ return m_script_sig.has_value() || m_script_witness.has_value();
+}
+
+std::pair<std::optional<CScript>, std::optional<CScriptWitness>> PreselectedInput::GetScripts() const
{
- m_input_weights[outpoint] = weight;
+ return {m_script_sig, m_script_witness};
}
-bool CCoinControl::HasInputWeight(const COutPoint& outpoint) const
+void PreselectedInput::SetPosition(unsigned int pos)
{
- return m_input_weights.count(outpoint) > 0;
+ m_pos = pos;
}
-int64_t CCoinControl::GetInputWeight(const COutPoint& outpoint) const
+std::optional<unsigned int> PreselectedInput::GetPosition() const
{
- auto it = m_input_weights.find(outpoint);
- assert(it != m_input_weights.end());
- return it->second;
+ return m_pos;
}
} // namespace wallet
diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h
index 71593e236f..b2f813383d 100644
--- a/src/wallet/coincontrol.h
+++ b/src/wallet/coincontrol.h
@@ -24,6 +24,58 @@ const int DEFAULT_MAX_DEPTH = 9999999;
//! Default for -avoidpartialspends
static constexpr bool DEFAULT_AVOIDPARTIALSPENDS = false;
+class PreselectedInput
+{
+private:
+ //! The previous output being spent by this input
+ std::optional<CTxOut> m_txout;
+ //! The input weight for spending this input
+ std::optional<int64_t> m_weight;
+ //! The sequence number for this input
+ std::optional<uint32_t> m_sequence;
+ //! The scriptSig for this input
+ std::optional<CScript> m_script_sig;
+ //! The scriptWitness for this input
+ std::optional<CScriptWitness> m_script_witness;
+ //! The position in the inputs vector for this input
+ std::optional<unsigned int> m_pos;
+
+public:
+ /**
+ * Set the previous output for this input.
+ * Only necessary if the input is expected to be an external input.
+ */
+ void SetTxOut(const CTxOut& txout);
+ /** Retrieve the previous output for this input. */
+ CTxOut GetTxOut() const;
+ /** Return whether the previous output is set for this input. */
+ bool HasTxOut() const;
+
+ /** Set the weight for this input. */
+ void SetInputWeight(int64_t weight);
+ /** Retrieve the input weight for this input. */
+ std::optional<int64_t> GetInputWeight() const;
+
+ /** Set the sequence for this input. */
+ void SetSequence(uint32_t sequence);
+ /** Retrieve the sequence for this input. */
+ std::optional<uint32_t> GetSequence() const;
+
+ /** Set the scriptSig for this input. */
+ void SetScriptSig(const CScript& script);
+ /** Set the scriptWitness for this input. */
+ void SetScriptWitness(const CScriptWitness& script_wit);
+ /** Return whether either the scriptSig or scriptWitness are set for this input. */
+ bool HasScripts() const;
+ /** Retrieve both the scriptSig and the scriptWitness. */
+ std::pair<std::optional<CScript>, std::optional<CScriptWitness>> GetScripts() const;
+
+ /** Store the position of this input. */
+ void SetPosition(unsigned int pos);
+ /** Retrieve the position of this input. */
+ std::optional<unsigned int> GetPosition() const;
+};
+
/** Coin Control Features. */
class CCoinControl
{
@@ -59,6 +111,10 @@ public:
int m_max_depth = DEFAULT_MAX_DEPTH;
//! SigningProvider that has pubkeys and scripts to do spend size estimation for external inputs
FlatSigningProvider m_external_provider;
+ //! Locktime
+ std::optional<uint32_t> m_locktime;
+ //! Version
+ std::optional<uint32_t> m_version;
CCoinControl();
@@ -69,11 +125,11 @@ public:
/**
* Returns true if the given output is pre-selected.
*/
- bool IsSelected(const COutPoint& output) const;
+ bool IsSelected(const COutPoint& outpoint) const;
/**
* Returns true if the given output is selected as an external input.
*/
- bool IsExternalSelected(const COutPoint& output) const;
+ bool IsExternalSelected(const COutPoint& outpoint) const;
/**
* Returns the external output for the given outpoint if it exists.
*/
@@ -82,16 +138,11 @@ public:
* Lock-in the given output for spending.
* The output will be included in the transaction even if it's not the most optimal choice.
*/
- void Select(const COutPoint& output);
- /**
- * Lock-in the given output as an external input for spending because it is not in the wallet.
- * The output will be included in the transaction even if it's not the most optimal choice.
- */
- void SelectExternal(const COutPoint& outpoint, const CTxOut& txout);
+ PreselectedInput& Select(const COutPoint& outpoint);
/**
* Unselects the given output.
*/
- void UnSelect(const COutPoint& output);
+ void UnSelect(const COutPoint& outpoint);
/**
* Unselects all outputs.
*/
@@ -105,22 +156,32 @@ public:
*/
void SetInputWeight(const COutPoint& outpoint, int64_t weight);
/**
- * Returns true if the input weight is set.
- */
- bool HasInputWeight(const COutPoint& outpoint) const;
- /**
* Returns the input weight.
*/
- int64_t GetInputWeight(const COutPoint& outpoint) const;
+ std::optional<int64_t> GetInputWeight(const COutPoint& outpoint) const;
+ /** Retrieve the sequence for an input */
+ std::optional<uint32_t> GetSequence(const COutPoint& outpoint) const;
+ /** Retrieves the scriptSig and scriptWitness for an input. */
+ std::pair<std::optional<CScript>, std::optional<CScriptWitness>> GetScripts(const COutPoint& outpoint) const;
+
+ bool HasSelectedOrder() const
+ {
+ return m_selection_pos > 0;
+ }
+
+ std::optional<unsigned int> GetSelectionPos(const COutPoint& outpoint) const
+ {
+ const auto it = m_selected.find(outpoint);
+ if (it == m_selected.end()) {
+ return std::nullopt;
+ }
+ return it->second.GetPosition();
+ }
private:
//! Selected inputs (inputs that will be used, regardless of whether they're optimal or not)
- std::set<COutPoint> m_selected_inputs;
- //! Map of external inputs to include in the transaction
- //! These are not in the wallet, so we need to track them separately
- std::map<COutPoint, CTxOut> m_external_txouts;
- //! Map of COutPoints to the maximum weight for that input
- std::map<COutPoint, int64_t> m_input_weights;
+ std::map<COutPoint, PreselectedInput> m_selected;
+ unsigned int m_selection_pos{0};
};
} // namespace wallet
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index d9a08310a8..c6ed0fe11c 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -203,10 +203,9 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
errors.push_back(Untranslated(strprintf("%s:%u is already spent", txin.prevout.hash.GetHex(), txin.prevout.n)));
return Result::MISC_ERROR;
}
- if (wallet.IsMine(txin.prevout)) {
- new_coin_control.Select(txin.prevout);
- } else {
- new_coin_control.SelectExternal(txin.prevout, coin.out);
+ PreselectedInput& preset_txin = new_coin_control.Select(txin.prevout);
+ if (!wallet.IsMine(txin.prevout)) {
+ preset_txin.SetTxOut(coin.out);
}
input_value += coin.out.nValue;
spent_outputs.push_back(coin.out);
@@ -317,8 +316,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
// We cannot source new unconfirmed inputs(bip125 rule 2)
new_coin_control.m_min_depth = 1;
- constexpr int RANDOM_CHANGE_POSITION = -1;
- auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
+ auto res = CreateTransaction(wallet, recipients, std::nullopt, new_coin_control, false);
if (!res) {
errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
return Result::WALLET_ERROR;
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 4ca5e29229..d15273dfc9 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -281,12 +281,12 @@ public:
CAmount& fee) override
{
LOCK(m_wallet->cs_wallet);
- auto res = CreateTransaction(*m_wallet, recipients, change_pos,
+ auto res = CreateTransaction(*m_wallet, recipients, change_pos == -1 ? std::nullopt : std::make_optional(change_pos),
coin_control, sign);
if (!res) return util::Error{util::ErrorString(res)};
const auto& txr = *res;
fee = txr.fee;
- change_pos = txr.change_pos;
+ change_pos = txr.change_pos ? *txr.change_pos : -1;
return txr.tx;
}
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index e7884af8b0..b121c8e1a7 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -155,8 +155,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
std::shuffle(recipients.begin(), recipients.end(), FastRandomContext());
// Send
- constexpr int RANDOM_CHANGE_POSITION = -1;
- auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true);
+ auto res = CreateTransaction(wallet, recipients, std::nullopt, coin_control, true);
if (!res) {
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original);
}
@@ -489,13 +488,13 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true)
return args;
}
-void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const UniValue& options, CCoinControl& coinControl, bool override_min_fee)
+CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransaction& tx, const UniValue& options, CCoinControl& coinControl, bool override_min_fee)
{
// Make sure the results are valid at least up to the most recent block
// the user could have gotten from another RPC command prior to now
wallet.BlockUntilSyncedToCurrentChain();
- change_position = -1;
+ std::optional<unsigned int> change_position;
bool lockUnspents = false;
UniValue subtractFeeFromOutputs;
std::set<int> setSubtractFeeFromOutputs;
@@ -553,7 +552,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
}
if (options.exists("changePosition") || options.exists("change_position")) {
- change_position = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt<int>();
+ int pos = (options.exists("change_position") ? options["change_position"] : options["changePosition"]).getInt<int>();
+ if (pos < 0 || (unsigned int)pos > tx.vout.size()) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds");
+ }
+ change_position = (unsigned int)pos;
}
if (options.exists("change_type")) {
@@ -703,9 +706,6 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
if (tx.vout.size() == 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output");
- if (change_position != -1 && (change_position < 0 || (unsigned int)change_position > tx.vout.size()))
- throw JSONRPCError(RPC_INVALID_PARAMETER, "changePosition out of bounds");
-
for (unsigned int idx = 0; idx < subtractFeeFromOutputs.size(); idx++) {
int pos = subtractFeeFromOutputs[idx].getInt<int>();
if (setSubtractFeeFromOutputs.count(pos))
@@ -717,11 +717,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
setSubtractFeeFromOutputs.insert(pos);
}
- bilingual_str error;
-
- if (!FundTransaction(wallet, tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) {
- throw JSONRPCError(RPC_WALLET_ERROR, error.original);
+ auto txr = FundTransaction(wallet, tx, change_position, lockUnspents, setSubtractFeeFromOutputs, coinControl);
+ if (!txr) {
+ throw JSONRPCError(RPC_WALLET_ERROR, ErrorString(txr).original);
}
+ return *txr;
}
static void SetOptionsInputWeights(const UniValue& inputs, UniValue& options)
@@ -844,17 +844,15 @@ RPCHelpMan fundrawtransaction()
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
}
- CAmount fee;
- int change_position;
CCoinControl coin_control;
// Automatically select (additional) coins. Can be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = true;
- FundTransaction(*pwallet, tx, fee, change_position, request.params[1], coin_control, /*override_min_fee=*/true);
+ auto txr = FundTransaction(*pwallet, tx, request.params[1], coin_control, /*override_min_fee=*/true);
UniValue result(UniValue::VOBJ);
- result.pushKV("hex", EncodeHexTx(CTransaction(tx)));
- result.pushKV("fee", ValueFromAmount(fee));
- result.pushKV("changepos", change_position);
+ result.pushKV("hex", EncodeHexTx(*txr.tx));
+ result.pushKV("fee", ValueFromAmount(txr.fee));
+ result.pushKV("changepos", txr.change_pos ? (int)*txr.change_pos : -1);
return result;
},
@@ -1276,8 +1274,6 @@ RPCHelpMan send()
PreventOutdatedOptions(options);
- CAmount fee;
- int change_position;
bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf};
CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf);
CCoinControl coin_control;
@@ -1285,9 +1281,9 @@ RPCHelpMan send()
// be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
SetOptionsInputWeights(options["inputs"], options);
- FundTransaction(*pwallet, rawTx, fee, change_position, options, coin_control, /*override_min_fee=*/false);
+ auto txr = FundTransaction(*pwallet, rawTx, options, coin_control, /*override_min_fee=*/false);
- return FinishTransaction(pwallet, options, rawTx);
+ return FinishTransaction(pwallet, options, CMutableTransaction(*txr.tx));
}
};
}
@@ -1712,8 +1708,6 @@ RPCHelpMan walletcreatefundedpsbt()
UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]};
- CAmount fee;
- int change_position;
const UniValue &replaceable_arg = options["replaceable"];
const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()};
CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf);
@@ -1722,10 +1716,10 @@ RPCHelpMan walletcreatefundedpsbt()
// be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
SetOptionsInputWeights(request.params[0], options);
- FundTransaction(wallet, rawTx, fee, change_position, options, coin_control, /*override_min_fee=*/true);
+ auto txr = FundTransaction(wallet, rawTx, options, coin_control, /*override_min_fee=*/true);
// Make a blank psbt
- PartiallySignedTransaction psbtx(rawTx);
+ PartiallySignedTransaction psbtx(CMutableTransaction(*txr.tx));
// Fill transaction with out data but don't sign
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
@@ -1741,8 +1735,8 @@ RPCHelpMan walletcreatefundedpsbt()
UniValue result(UniValue::VOBJ);
result.pushKV("psbt", EncodeBase64(ssTx.str()));
- result.pushKV("fee", ValueFromAmount(fee));
- result.pushKV("changepos", change_position);
+ result.pushKV("fee", ValueFromAmount(txr.fee));
+ result.pushKV("changepos", txr.change_pos ? (int)*txr.change_pos : -1);
return result;
},
};
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 35583642a5..5b28d38c37 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -117,8 +117,9 @@ static std::optional<int64_t> GetSignedTxinWeight(const CWallet* wallet, const C
const bool can_grind_r)
{
// If weight was provided, use that.
- if (coin_control && coin_control->HasInputWeight(txin.prevout)) {
- return coin_control->GetInputWeight(txin.prevout);
+ std::optional<int64_t> weight;
+ if (coin_control && (weight = coin_control->GetInputWeight(txin.prevout))) {
+ return weight.value();
}
// Otherwise, use the maximum satisfaction size provided by the descriptor.
@@ -261,7 +262,10 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
const bool can_grind_r = wallet.CanGrindR();
std::map<COutPoint, CAmount> map_of_bump_fees = wallet.chain().calculateIndividualBumpFees(coin_control.ListSelected(), coin_selection_params.m_effective_feerate);
for (const COutPoint& outpoint : coin_control.ListSelected()) {
- int input_bytes = -1;
+ int64_t input_bytes = coin_control.GetInputWeight(outpoint).value_or(-1);
+ if (input_bytes != -1) {
+ input_bytes = GetVirtualTransactionSize(input_bytes, 0, 0);
+ }
CTxOut txout;
if (auto ptr_wtx = wallet.GetWalletTx(outpoint.hash)) {
// Clearly invalid input, fail
@@ -269,7 +273,9 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
return util::Error{strprintf(_("Invalid pre-selected input %s"), outpoint.ToString())};
}
txout = ptr_wtx->tx->vout.at(outpoint.n);
- input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control);
+ if (input_bytes == -1) {
+ input_bytes = CalculateMaximumSignedInputSize(txout, &wallet, &coin_control);
+ }
} else {
// The input is external. We did not find the tx in mapWallet.
const auto out{coin_control.GetExternalOutput(outpoint)};
@@ -284,11 +290,6 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, can_grind_r, &coin_control);
}
- // If available, override calculated size with coin control specified size
- if (coin_control.HasInputWeight(outpoint)) {
- input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0);
- }
-
if (input_bytes == -1) {
return util::Error{strprintf(_("Not solvable pre-selected input %s"), outpoint.ToString())}; // Not solvable, can't estimate size for fee
}
@@ -964,18 +965,19 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng
static util::Result<CreatedTransactionResult> CreateTransactionInternal(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
- int change_pos,
+ std::optional<unsigned int> change_pos,
const CCoinControl& coin_control,
bool sign) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
AssertLockHeld(wallet.cs_wallet);
- // out variables, to be packed into returned result structure
- int nChangePosInOut = change_pos;
-
FastRandomContext rng_fast;
CMutableTransaction txNew; // The resulting transaction that we make
+ if (coin_control.m_version) {
+ txNew.nVersion = coin_control.m_version.value();
+ }
+
CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy
coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs;
@@ -1127,20 +1129,39 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee);
if (change_amount > 0) {
CTxOut newTxOut(change_amount, scriptChange);
- if (nChangePosInOut == -1) {
+ if (!change_pos) {
// Insert change txn at random position:
- nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1);
- } else if ((unsigned int)nChangePosInOut > txNew.vout.size()) {
+ change_pos = rng_fast.randrange(txNew.vout.size() + 1);
+ } else if ((unsigned int)*change_pos > txNew.vout.size()) {
return util::Error{_("Transaction change output index out of range")};
}
- txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut);
+ txNew.vout.insert(txNew.vout.begin() + *change_pos, newTxOut);
} else {
- nChangePosInOut = -1;
+ change_pos = std::nullopt;
}
// Shuffle selected coins and fill in final vin
std::vector<std::shared_ptr<COutput>> selected_coins = result.GetShuffledInputVector();
+ if (coin_control.HasSelected() && coin_control.HasSelectedOrder()) {
+ // When there are preselected inputs, we need to move them to be the first UTXOs
+ // and have them be in the order selected. We can use stable_sort for this, where we
+ // compare with the positions stored in coin_control. The COutputs that have positions
+ // will be placed before those that don't, and those positions will be in order.
+ std::stable_sort(selected_coins.begin(), selected_coins.end(),
+ [&coin_control](const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) {
+ auto a_pos = coin_control.GetSelectionPos(a->outpoint);
+ auto b_pos = coin_control.GetSelectionPos(b->outpoint);
+ if (a_pos.has_value() && b_pos.has_value()) {
+ return a_pos.value() < b_pos.value();
+ } else if (a_pos.has_value() && !b_pos.has_value()) {
+ return true;
+ } else {
+ return false;
+ }
+ });
+ }
+
// The sequence number is set to non-maxint so that DiscourageFeeSniping
// works.
//
@@ -1149,11 +1170,32 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// to avoid conflicting with other possible uses of nSequence,
// and in the spirit of "smallest possible change from prior
// behavior."
- const uint32_t nSequence{coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : CTxIn::MAX_SEQUENCE_NONFINAL};
+ bool use_anti_fee_sniping = true;
+ const uint32_t default_sequence{coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : CTxIn::MAX_SEQUENCE_NONFINAL};
for (const auto& coin : selected_coins) {
- txNew.vin.emplace_back(coin->outpoint, CScript(), nSequence);
+ std::optional<uint32_t> sequence = coin_control.GetSequence(coin->outpoint);
+ if (sequence) {
+ // If an input has a preset sequence, we can't do anti-fee-sniping
+ use_anti_fee_sniping = false;
+ }
+ txNew.vin.emplace_back(coin->outpoint, CScript{}, sequence.value_or(default_sequence));
+
+ auto scripts = coin_control.GetScripts(coin->outpoint);
+ if (scripts.first) {
+ txNew.vin.back().scriptSig = *scripts.first;
+ }
+ if (scripts.second) {
+ txNew.vin.back().scriptWitness = *scripts.second;
+ }
+ }
+ if (coin_control.m_locktime) {
+ txNew.nLockTime = coin_control.m_locktime.value();
+ // If we have a locktime set, we can't use anti-fee-sniping
+ use_anti_fee_sniping = false;
+ }
+ if (use_anti_fee_sniping) {
+ DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight());
}
- DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight());
// Calculate the transaction fee
TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control);
@@ -1172,8 +1214,8 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
// If there is a change output and we overpay the fees then increase the change to match the fee needed
- if (nChangePosInOut != -1 && fee_needed < current_fee) {
- auto& change = txNew.vout.at(nChangePosInOut);
+ if (change_pos && fee_needed < current_fee) {
+ auto& change = txNew.vout.at(*change_pos);
change.nValue += current_fee - fee_needed;
current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew);
if (fee_needed != current_fee) {
@@ -1184,11 +1226,11 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// Reduce output values for subtractFeeFromAmount
if (coin_selection_params.m_subtract_fee_outputs) {
CAmount to_reduce = fee_needed - current_fee;
- int i = 0;
+ unsigned int i = 0;
bool fFirst = true;
for (const auto& recipient : vecSend)
{
- if (i == nChangePosInOut) {
+ if (change_pos && i == *change_pos) {
++i;
}
CTxOut& txout = txNew.vout[i];
@@ -1227,7 +1269,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
// Give up if change keypool ran out and change is required
- if (scriptChange.empty() && nChangePosInOut != -1) {
+ if (scriptChange.empty() && change_pos) {
return util::Error{error};
}
@@ -1268,13 +1310,13 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
feeCalc.est.fail.start, feeCalc.est.fail.end,
(feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) > 0.0 ? 100 * feeCalc.est.fail.withinTarget / (feeCalc.est.fail.totalConfirmed + feeCalc.est.fail.inMempool + feeCalc.est.fail.leftMempool) : 0.0,
feeCalc.est.fail.withinTarget, feeCalc.est.fail.totalConfirmed, feeCalc.est.fail.inMempool, feeCalc.est.fail.leftMempool);
- return CreatedTransactionResult(tx, current_fee, nChangePosInOut, feeCalc);
+ return CreatedTransactionResult(tx, current_fee, change_pos, feeCalc);
}
util::Result<CreatedTransactionResult> CreateTransaction(
CWallet& wallet,
const std::vector<CRecipient>& vecSend,
- int change_pos,
+ std::optional<unsigned int> change_pos,
const CCoinControl& coin_control,
bool sign)
{
@@ -1290,7 +1332,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(
auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign);
TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res),
- res ? res->fee : 0, res ? res->change_pos : 0);
+ res ? res->fee : 0, res && res->change_pos.has_value() ? *res->change_pos : 0);
if (!res) return res;
const auto& txr_ungrouped = *res;
// try with avoidpartialspends unless it's enabled already
@@ -1300,16 +1342,15 @@ util::Result<CreatedTransactionResult> CreateTransaction(
tmp_cc.m_avoid_partial_spends = true;
// Reuse the change destination from the first creation attempt to avoid skipping BIP44 indexes
- const int ungrouped_change_pos = txr_ungrouped.change_pos;
- if (ungrouped_change_pos != -1) {
- ExtractDestination(txr_ungrouped.tx->vout[ungrouped_change_pos].scriptPubKey, tmp_cc.destChange);
+ if (txr_ungrouped.change_pos) {
+ ExtractDestination(txr_ungrouped.tx->vout[*txr_ungrouped.change_pos].scriptPubKey, tmp_cc.destChange);
}
auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, 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{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false};
TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(),
- txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() ? txr_grouped->change_pos : 0);
+ txr_grouped.has_value() ? txr_grouped->fee : 0, txr_grouped.has_value() && txr_grouped->change_pos.has_value() ? *txr_grouped->change_pos : 0);
if (txr_grouped) {
wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n",
txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped");
@@ -1319,7 +1360,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(
return res;
}
-bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl)
+util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl coinControl)
{
std::vector<CRecipient> vecSend;
@@ -1332,6 +1373,12 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
vecSend.push_back(recipient);
}
+ // Set the user desired locktime
+ coinControl.m_locktime = tx.nLockTime;
+
+ // Set the user desired version
+ coinControl.m_version = tx.nVersion;
+
// Acquire the locks to prevent races to the new locked unspents between the
// CreateTransaction call and LockCoin calls (when lockUnspents is true).
LOCK(wallet.cs_wallet);
@@ -1346,50 +1393,31 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
for (const CTxIn& txin : tx.vin) {
const auto& outPoint = txin.prevout;
- if (wallet.IsMine(outPoint)) {
- // The input was found in the wallet, so select as internal
- coinControl.Select(outPoint);
- } else if (coins[outPoint].out.IsNull()) {
- error = _("Unable to find UTXO for external input");
- return false;
- } else {
+ PreselectedInput& preset_txin = coinControl.Select(outPoint);
+ if (!wallet.IsMine(outPoint)) {
+ if (coins[outPoint].out.IsNull()) {
+ return util::Error{_("Unable to find UTXO for external input")};
+ }
+
// The input was not in the wallet, but is in the UTXO set, so select as external
- coinControl.SelectExternal(outPoint, coins[outPoint].out);
+ preset_txin.SetTxOut(coins[outPoint].out);
}
+ preset_txin.SetSequence(txin.nSequence);
+ preset_txin.SetScriptSig(txin.scriptSig);
+ preset_txin.SetScriptWitness(txin.scriptWitness);
}
- auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false);
+ auto res = CreateTransaction(wallet, vecSend, change_pos, coinControl, false);
if (!res) {
- error = util::ErrorString(res);
- return false;
- }
- const auto& txr = *res;
- CTransactionRef tx_new = txr.tx;
- nFeeRet = txr.fee;
- nChangePosInOut = txr.change_pos;
-
- if (nChangePosInOut != -1) {
- tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
- }
-
- // Copy output sizes from new transaction; they may have had the fee
- // subtracted from them.
- for (unsigned int idx = 0; idx < tx.vout.size(); idx++) {
- tx.vout[idx].nValue = tx_new->vout[idx].nValue;
+ return res;
}
- // Add new txins while keeping original txin scriptSig/order.
- for (const CTxIn& txin : tx_new->vin) {
- if (!coinControl.IsSelected(txin.prevout)) {
- tx.vin.push_back(txin);
-
- }
- if (lockUnspents) {
+ if (lockUnspents) {
+ for (const CTxIn& txin : res->tx->vin) {
wallet.LockCoin(txin.prevout);
}
-
}
- return true;
+ return res;
}
} // namespace wallet
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index 407627b5f1..504c078b80 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -207,9 +207,9 @@ struct CreatedTransactionResult
CTransactionRef tx;
CAmount fee;
FeeCalculation fee_calc;
- int change_pos;
+ std::optional<unsigned int> change_pos;
- CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, int _change_pos, const FeeCalculation& _fee_calc)
+ CreatedTransactionResult(CTransactionRef _tx, CAmount _fee, std::optional<unsigned int> _change_pos, const FeeCalculation& _fee_calc)
: tx(_tx), fee(_fee), fee_calc(_fee_calc), change_pos(_change_pos) {}
};
@@ -218,13 +218,13 @@ struct CreatedTransactionResult
* selected by SelectCoins(); Also create the change output, when needed
* @note passing change_pos as -1 will result in setting a random position
*/
-util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true);
+util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, std::optional<unsigned int> change_pos, const CCoinControl& coin_control, bool sign = true);
/**
* Insert additional inputs into the transaction by
* calling CreateTransaction();
*/
-bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl);
+util::Result<CreatedTransactionResult> FundTransaction(CWallet& wallet, const CMutableTransaction& tx, std::optional<unsigned int> change_pos, bool lockUnspents, const std::set<int>& setSubtractFeeFromOutputs, CCoinControl);
} // namespace wallet
#endif // BITCOIN_WALLET_SPEND_H
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index 9569210ba0..fa0dfa5556 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -1282,7 +1282,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test)
cc.m_allow_other_inputs = false;
COutput output = available_coins.All().at(0);
cc.SetInputWeight(output.outpoint, 148);
- cc.SelectExternal(output.outpoint, output.txout);
+ cc.Select(output.outpoint).SetTxOut(output.txout);
LOCK(wallet->cs_wallet);
const auto preset_inputs = *Assert(FetchSelectedInputs(*wallet, cc, cs_params));
diff --git a/src/wallet/test/fuzz/coincontrol.cpp b/src/wallet/test/fuzz/coincontrol.cpp
index 0f71f28df2..f1efbc1cb8 100644
--- a/src/wallet/test/fuzz/coincontrol.cpp
+++ b/src/wallet/test/fuzz/coincontrol.cpp
@@ -60,7 +60,7 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
},
[&] {
const CTxOut tx_out{ConsumeMoney(fuzzed_data_provider), ConsumeScript(fuzzed_data_provider)};
- (void)coin_control.SelectExternal(out_point, tx_out);
+ (void)coin_control.Select(out_point).SetTxOut(tx_out);
},
[&] {
(void)coin_control.UnSelect(out_point);
@@ -76,10 +76,7 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
(void)coin_control.SetInputWeight(out_point, weight);
},
[&] {
- // Condition to avoid the assertion in GetInputWeight
- if (coin_control.HasInputWeight(out_point)) {
- (void)coin_control.GetInputWeight(out_point);
- }
+ (void)coin_control.GetInputWeight(out_point);
});
}
}
diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp
index 82cdd32302..203ab5f606 100644
--- a/src/wallet/test/fuzz/notifications.cpp
+++ b/src/wallet/test/fuzz/notifications.cpp
@@ -156,10 +156,9 @@ struct FuzzedWallet {
coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool();
// Add solving data (m_external_provider and SelectExternal)?
- CAmount fee_out;
int change_position{fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, tx.vout.size() - 1)};
bilingual_str error;
- (void)FundTransaction(*wallet, tx, fee_out, change_position, error, /*lockUnspents=*/false, subtract_fee_from_outputs, coin_control);
+ (void)FundTransaction(*wallet, tx, change_position, /*lockUnspents=*/false, subtract_fee_from_outputs, coin_control);
}
};
diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp
index 5926d88129..b2d252b3f9 100644
--- a/src/wallet/test/spend_tests.cpp
+++ b/src/wallet/test/spend_tests.cpp
@@ -28,13 +28,12 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
// instead of the miner.
auto check_tx = [&wallet](CAmount leftover_input_amount) {
CRecipient recipient{PubKeyDestination({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true};
- constexpr int RANDOM_CHANGE_POSITION = -1;
CCoinControl coin_control;
coin_control.m_feerate.emplace(10000);
coin_control.fOverrideFeeRate = true;
// We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output
coin_control.m_change_type = OutputType::LEGACY;
- auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control);
+ auto res = CreateTransaction(*wallet, {recipient}, std::nullopt, coin_control);
BOOST_CHECK(res);
const auto& txr = *res;
BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index dd43705a84..3d1cbe36a8 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -558,8 +558,7 @@ public:
CTransactionRef tx;
CCoinControl dummy;
{
- constexpr int RANDOM_CHANGE_POSITION = -1;
- auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
+ auto res = CreateTransaction(*wallet, {recipient}, std::nullopt, dummy);
BOOST_CHECK(res);
tx = res->tx;
}
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-14 02:35:00 +0000