wallet: Drop mapValue from CommitTransaction
What the developer wrote
Authored by Ava Chow
wallet: Drop mapValue from CommitTransaction
The values previously passed in mapValue are now parameters to
CommitTransaction so there is no need for mapValue to be passed.
What changed, and why it matters
This is a routine internal code cleanup in Bitcoin Core's wallet. It removes an old way of passing extra transaction details (called mapValue) and replaces it with explicit named parameters. The change does not alter what the wallet records or how transactions are broadcast; it only makes the code easier to maintain. There is no indication of a security bug being fixed.
No security action required. Treat as normal code-quality refactor. Reviewers may optionally verify that all former mapValue keys (comment, to, replaces_txid) are still preserved in the new explicit parameters, which the diff confirms.
Security signals we found
No security-relevant behavior change: same mapValue keys are still written to the wallet transaction record
Refactor-only signature change: removal of generic mapValue parameter in favor of explicit optional parameters
No validation, cryptography, consensus, or networking code modified
No bug, crash, or vulnerability described in commit message or diff
Evidence from the diff
The commit refactors CWallet::CommitTransaction and the interfaces::Wallet::commitTransaction method so that the generic mapValue_t mapValue parameter is removed. Former callers that passed mapValue now pass the specific values they care about—comment, comment_to, and replaces_txid—as explicit std::optional parameters. The implementation still writes the same keys into wtx.mapValue inside AddToWallet’s callback, preserving behavior. The change touches the wallet interface, Qt wallet model, fee bumper, RPC spend helpers, and tests. It is a pure API/structural refactor with no functional change to transaction commit logic.
Changed components
src/interfaces/wallet.hsrc/qt/walletmodel.cppsrc/wallet/feebumper.cppsrc/wallet/interfaces.cppsrc/wallet/rpc/spend.cppsrc/wallet/test/wallet_tests.cppsrc/wallet/wallet.cppsrc/wallet/wallet.hInspect captured patch +8 / −12
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index d2116317..4d965ec5 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -151,7 +151,6 @@ public:
//! Commit transaction.
virtual void commitTransaction(CTransactionRef tx,
- WalletValueMap value_map,
WalletOrderForm order_form) = 0;
//! Return whether transaction can be abandoned.
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index a9142b93..add876ae 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -246,7 +246,7 @@ void WalletModel::sendCoins(WalletModelTransaction& transaction)
}
auto& newTx = transaction.getWtx();
- wallet().commitTransaction(newTx, /*value_map=*/{}, std::move(vOrderForm));
+ wallet().commitTransaction(newTx, std::move(vOrderForm));
DataStream ssTx;
ssTx << TX_WITH_WITNESS(*newTx);
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index 57a5b42f..5ccaf896 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -370,7 +370,9 @@ Result CommitTransaction(CWallet& wallet, const Txid& txid, CMutableTransaction&
// commit/broadcast the tx
CTransactionRef tx = MakeTransactionRef(std::move(mtx));
- wallet.CommitTransaction(tx, oldWtx.mapValue, oldWtx.vOrderForm, oldWtx.GetHash());
+ std::optional<std::string> comment = oldWtx.mapValue.contains("comment") ? std::optional(oldWtx.mapValue.at("comment")) : std::nullopt;
+ std::optional<std::string> comment_to = oldWtx.mapValue.contains("to") ? std::optional(oldWtx.mapValue.at("to")) : std::nullopt;
+ wallet.CommitTransaction(tx, oldWtx.vOrderForm, oldWtx.GetHash(), comment, comment_to);
// mark the original tx as bumped
bumped_txid = tx->GetHash();
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 4eee155c..800e52a4 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -266,11 +266,10 @@ public:
return CreateTransaction(*m_wallet, recipients, change_pos, coin_control, sign);
}
void commitTransaction(CTransactionRef tx,
- WalletValueMap value_map,
WalletOrderForm order_form) override
{
LOCK(m_wallet->cs_wallet);
- m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));
+ m_wallet->CommitTransaction(std::move(tx), std::move(order_form));
}
bool transactionCanBeAbandoned(const Txid& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
bool abandonTransaction(const Txid& txid) override
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 796349a5..42b7eeb3 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -139,7 +139,7 @@ static UniValue FinishTransaction(const std::shared_ptr<CWallet> pwallet, const
CTransactionRef tx(MakeTransactionRef(std::move(mtx)));
result.pushKV("txid", tx->GetHash().GetHex());
if (add_to_wallet && !psbt_opt_in) {
- pwallet->CommitTransaction(tx, {}, /*orderForm=*/{});
+ pwallet->CommitTransaction(tx, /*orderForm=*/{});
} else {
result.pushKV("hex", hex);
}
@@ -191,7 +191,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original);
}
const CTransactionRef& tx = res->tx;
- wallet.CommitTransaction(tx, /*mapValue=*/{}, /*orderForm=*/{}, /*replaces_txid=*/std::nullopt, comment, comment_to);
+ wallet.CommitTransaction(tx, /*orderForm=*/{}, /*replaces_txid=*/std::nullopt, comment, comment_to);
if (verbose) {
UniValue entry(UniValue::VOBJ);
entry.pushKV("txid", tx->GetHash().GetHex());
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 9a0531ca..41fadb8c 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -402,7 +402,7 @@ public:
BOOST_CHECK(res);
tx = res->tx;
}
- wallet->CommitTransaction(tx, {}, {});
+ wallet->CommitTransaction(tx, {});
CMutableTransaction blocktx;
{
LOCK(wallet->cs_wallet);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 28c055f4..5f529a52 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2326,7 +2326,6 @@ OutputType CWallet::TransactionChangeType(const std::optional<OutputType>& chang
void CWallet::CommitTransaction(
CTransactionRef tx,
- mapValue_t mapValue,
std::vector<std::pair<std::string, std::string>> orderForm,
std::optional<Txid> replaces_txid,
std::optional<std::string> comment,
@@ -2341,7 +2340,6 @@ void CWallet::CommitTransaction(
CWalletTx* wtx = AddToWallet(tx, TxStateInactive{}, [&](CWalletTx& wtx, bool new_tx) {
CHECK_NONFATAL(wtx.mapValue.empty());
CHECK_NONFATAL(wtx.vOrderForm.empty());
- wtx.mapValue = std::move(mapValue);
if (replaces_txid) wtx.mapValue["replaces_txid"] = replaces_txid->ToString();
if (comment) wtx.mapValue["comment"] = *comment;
if (comment_to) wtx.mapValue["to"] = *comment_to;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index f9bfaa79..080046cd 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -690,7 +690,6 @@ public:
* broadcasting the transaction.
*
* @param[in] tx The transaction to be broadcast.
- * @param[in] mapValue key-values to be set on the transaction.
* @param[in] orderForm BIP 70 / BIP 21 order form details to be set on the transaction.
* @param[in] replaces_txid The txid of the transaction that this transaction replaces
* @param[in] comment The user's comment for this transaction
@@ -698,7 +697,6 @@ public:
*/
void CommitTransaction(
CTransactionRef tx,
- mapValue_t mapValue,
std::vector<std::pair<std::string, std::string>> orderForm,
std::optional<Txid> replaces_txid = std::nullopt,
std::optional<std::string> comment = std::nullopt,
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.