phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-10 03:47:29 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

wallet: return CreatedTransactionResult from FundTransaction

Browse source 
Instead of using the output parameters, return CreatedTransactionResult
from FundTransaction in the same way that CreateTransaction does.
Additionally, instead of modifying the original CMutableTransaction, the
result from CreateTransactionInternal is used.
This commit is contained in:
Andrew Chow 2022-06-01 17:04:03 -04:00 committed by Andrew Chow
parent 758501b713
commit 0295b44c25
4 changed files with 30 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
src/wallet/rpc/spend.cpp
View file

@ -488,13 +488,13 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true)
return args; 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 // 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 // the user could have gotten from another RPC command prior to now
wallet.BlockUntilSyncedToCurrentChain(); wallet.BlockUntilSyncedToCurrentChain();
change_position = -1; std::optional<unsigned int> change_position;
bool lockUnspents = false; bool lockUnspents = false;
UniValue subtractFeeFromOutputs; UniValue subtractFeeFromOutputs;
std::set<int> setSubtractFeeFromOutputs; std::set<int> setSubtractFeeFromOutputs;
@ -552,7 +552,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
} }
if (options.exists("changePosition") || options.exists("change_position")) { 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")) { if (options.exists("change_type")) {
@ -702,9 +706,6 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
if (tx.vout.size() == 0) if (tx.vout.size() == 0)
throw JSONRPCError(RPC_INVALID_PARAMETER, "TX must have at least one output"); 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++) { for (unsigned int idx = 0; idx < subtractFeeFromOutputs.size(); idx++) {
int pos = subtractFeeFromOutputs[idx].getInt<int>(); int pos = subtractFeeFromOutputs[idx].getInt<int>();
if (setSubtractFeeFromOutputs.count(pos)) if (setSubtractFeeFromOutputs.count(pos))
@ -716,11 +717,11 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out,
setSubtractFeeFromOutputs.insert(pos); setSubtractFeeFromOutputs.insert(pos);
} }
bilingual_str error; auto txr = FundTransaction(wallet, tx, change_position, lockUnspents, setSubtractFeeFromOutputs, coinControl);
if (!txr) {
if (!FundTransaction(wallet, tx, fee_out, change_position, error, lockUnspents, setSubtractFeeFromOutputs, coinControl)) { throw JSONRPCError(RPC_WALLET_ERROR, ErrorString(txr).original);
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
} }
return *txr;
} }
static void SetOptionsInputWeights(const UniValue& inputs, UniValue& options) static void SetOptionsInputWeights(const UniValue& inputs, UniValue& options)
@ -843,17 +844,15 @@ RPCHelpMan fundrawtransaction()
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
} }
CAmount fee;
int change_position;
CCoinControl coin_control; CCoinControl coin_control;
// Automatically select (additional) coins. Can be overridden by options.add_inputs. // Automatically select (additional) coins. Can be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = true; 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); UniValue result(UniValue::VOBJ);
result.pushKV("hex", EncodeHexTx(CTransaction(tx))); result.pushKV("hex", EncodeHexTx(*txr.tx));
result.pushKV("fee", ValueFromAmount(fee)); result.pushKV("fee", ValueFromAmount(txr.fee));
result.pushKV("changepos", change_position); result.pushKV("changepos", txr.change_pos ? (int)*txr.change_pos : -1);
return result; return result;
}, },
@ -1275,8 +1274,6 @@ RPCHelpMan send()
PreventOutdatedOptions(options); PreventOutdatedOptions(options);
CAmount fee;
int change_position;
bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf};
CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf); CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf);
CCoinControl coin_control; CCoinControl coin_control;
@ -1284,9 +1281,9 @@ RPCHelpMan send()
// be overridden by options.add_inputs. // be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
SetOptionsInputWeights(options["inputs"], options); 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));
} }
}; };
} }
@ -1711,8 +1708,6 @@ RPCHelpMan walletcreatefundedpsbt()
UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]}; UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]};
CAmount fee;
int change_position;
const UniValue &replaceable_arg = options["replaceable"]; const UniValue &replaceable_arg = options["replaceable"];
const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()}; 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); CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf);
@ -1721,10 +1716,10 @@ RPCHelpMan walletcreatefundedpsbt()
// be overridden by options.add_inputs. // be overridden by options.add_inputs.
coin_control.m_allow_other_inputs = rawTx.vin.size() == 0; coin_control.m_allow_other_inputs = rawTx.vin.size() == 0;
SetOptionsInputWeights(request.params[0], options); 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 // Make a blank psbt
PartiallySignedTransaction psbtx(rawTx); PartiallySignedTransaction psbtx(CMutableTransaction(*txr.tx));
// Fill transaction with out data but don't sign // Fill transaction with out data but don't sign
bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool(); bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool();
@ -1740,8 +1735,8 @@ RPCHelpMan walletcreatefundedpsbt()
UniValue result(UniValue::VOBJ); UniValue result(UniValue::VOBJ);
result.pushKV("psbt", EncodeBase64(ssTx.str())); result.pushKV("psbt", EncodeBase64(ssTx.str()));
result.pushKV("fee", ValueFromAmount(fee)); result.pushKV("fee", ValueFromAmount(txr.fee));
result.pushKV("changepos", change_position); result.pushKV("changepos", txr.change_pos ? (int)*txr.change_pos : -1);
return result; return result;
}, },
}; };

36
src/wallet/spend.cpp
View file

@ -1360,7 +1360,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(
return res; 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; std::vector<CRecipient> vecSend;
@ -1396,8 +1396,7 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
PreselectedInput& preset_txin = coinControl.Select(outPoint); PreselectedInput& preset_txin = coinControl.Select(outPoint);
if (!wallet.IsMine(outPoint)) { if (!wallet.IsMine(outPoint)) {
if (coins[outPoint].out.IsNull()) { if (coins[outPoint].out.IsNull()) {
error = _("Unable to find UTXO for external input"); return util::Error{_("Unable to find UTXO for external input")};
return false;
} }
// The input was not in the wallet, but is in the UTXO set, so select as external // The input was not in the wallet, but is in the UTXO set, so select as external
@ -1408,38 +1407,17 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet,
preset_txin.SetScriptWitness(txin.scriptWitness); preset_txin.SetScriptWitness(txin.scriptWitness);
} }
auto res = CreateTransaction(wallet, vecSend, nChangePosInOut == -1 ? std::nullopt : std::optional<unsigned int>((unsigned int)nChangePosInOut), coinControl, false); auto res = CreateTransaction(wallet, vecSend, change_pos, coinControl, false);
if (!res) { if (!res) {
error = util::ErrorString(res); return res;
return false;
}
const auto& txr = *res;
CTransactionRef tx_new = txr.tx;
nFeeRet = txr.fee;
nChangePosInOut = txr.change_pos ? *txr.change_pos : -1;
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 if (lockUnspents) {
// subtracted from them. for (const CTxIn& txin : res->tx->vin) {
for (unsigned int idx = 0; idx < tx.vout.size(); idx++) {
tx.vout[idx].nValue = tx_new->vout[idx].nValue;
}
// 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) {
wallet.LockCoin(txin.prevout); wallet.LockCoin(txin.prevout);
} }
} }
return true; return res;
} }
} // namespace wallet } // namespace wallet

2
src/wallet/spend.h
View file

@ -224,7 +224,7 @@ util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const
* Insert additional inputs into the transaction by * Insert additional inputs into the transaction by
* calling CreateTransaction(); * 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 } // namespace wallet
#endif // BITCOIN_WALLET_SPEND_H #endif // BITCOIN_WALLET_SPEND_H

3
src/wallet/test/fuzz/notifications.cpp
View file

@ -156,10 +156,9 @@ struct FuzzedWallet {
coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool(); coin_control.fOverrideFeeRate = fuzzed_data_provider.ConsumeBool();
// Add solving data (m_external_provider and SelectExternal)? // Add solving data (m_external_provider and SelectExternal)?
CAmount fee_out;
int change_position{fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, tx.vout.size() - 1)}; int change_position{fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, tx.vout.size() - 1)};
bilingual_str error; 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);
} }
}; };