wallet2: fix edge case where tx's ki's remain marked unspent
What the developer wrote
Authored by j-berman
wallet2: fix edge case where tx's ki's remain marked unspent
If a tx is marked as failed (because it never shows up in the
daemon's pool), its key images get reset back to unspent so they
can be used in future txs.
If the tx re-enters the daemon's pool (e.g. it's removed from the
pool and then relayed back), then the wallet incorrectly maintains
that the tx's key images are unspent.
This change ensures the wallet re-marks the tx's key images as
spent if the tx re-appears in the node's pool.
What changed, and why it matters
This patch fixes a bookkeeping bug in the Monero wallet. When a transaction temporarily disappears from the network's pending pool and is later re-broadcast, the wallet could wrongly treat the coins it spends as still available. That could let the wallet try to spend the same coins twice, producing a conflict that prevents any of those follow-up transactions from confirming until the wallet state is manually repaired.
Apply the patch. Users running affected wallet versions should refresh/rescan if they observe failed or stuck transactions after a previously failed tx re-enters the mempool. Wallet RPC/services should monitor for duplicate key-image attempts and alert rather than blindly re-spend.
Security signals we found
Double-spend risk from inconsistent local key-image state
Transaction failure / denial of service for wallet-created transactions
State synchronization bug between wallet and daemon mempool
No cryptographic weakness or consensus bypass
Evidence from the diff
wallet2::process_unconfirmed_transfer() previously reset key images to unspent when a pending tx was marked failed (not seen in the daemon’s pool for the timeout period). If that tx later re-appeared in the pool, the wallet updated the tx state to pending_in_pool but did not re-mark its inputs as spent. The patch adds set_tx_key_images_spent(), called both when a tx is seen in the pool (spent=true) and when it is marked failed (spent=false), keeping the spent/unspent state consistent with the tx’s actual presence in the pool.
Changed components
src/wallet/wallet2.cppwallet2::process_unconfirmed_transferwallet2::accept_pool_tx_for_processing (caller context)Inspect captured patch +34 / −17
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index e149a91..b7ec6d2 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -3656,6 +3656,35 @@ bool wallet2::accept_pool_tx_for_processing(const crypto::hash &txid)
// Process an unconfirmed transfer after we know whether it's in the pool or not
void wallet2::process_unconfirmed_transfer(bool incremental, const crypto::hash &txid, wallet2::unconfirmed_transfer_details &tx_details, bool seen_in_pool, std::chrono::system_clock::time_point now, bool refreshed)
{
+ const auto set_tx_key_images_spent = [&](const bool spent)
+ {
+ for (size_t vini = 0; vini < tx_details.m_tx.vin.size(); ++vini)
+ {
+ if (tx_details.m_tx.vin[vini].type() != typeid(txin_to_key))
+ continue;
+
+ const crypto::key_image &key_image = boost::get<txin_to_key>(tx_details.m_tx.vin[vini]).k_image;
+ const auto it_ki = m_key_images.find(key_image);
+ if (it_ki == m_key_images.end())
+ continue;
+
+ const std::size_t i = it_ki->second;
+ if (i >= m_transfers.size())
+ continue;
+ const transfer_details &td = m_transfers.at(i);
+ if (td.m_key_image != key_image)
+ continue;
+ if (td.m_spent == spent)
+ continue;
+
+ LOG_PRINT_L1("Resetting spent status for output " << vini << ": " << key_image << " (spent=" << spent << ")");
+ if (spent)
+ set_spent(i, 0);
+ else
+ set_unspent(i);
+ }
+ };
+
// TODO: set tx_propagation_timeout to CRYPTONOTE_DANDELIONPP_EMBARGO_AVERAGE * 3 / 2 after v15 hardfork
constexpr const std::chrono::seconds tx_propagation_timeout{500};
if (seen_in_pool)
@@ -3665,6 +3694,10 @@ void wallet2::process_unconfirmed_transfer(bool incremental, const crypto::hash
tx_details.m_state = wallet2::unconfirmed_transfer_details::pending_in_pool;
MINFO("Pending txid " << txid << " seen in pool, marking as pending in pool");
}
+
+ // The inputs are spent, they're in the pool! It's possible the tx was previously marked as failed, so we
+ // make sure to re-mark the outputs as spent.
+ set_tx_key_images_spent(true/*spent*/);
}
else
{
@@ -3690,23 +3723,7 @@ void wallet2::process_unconfirmed_transfer(bool incremental, const crypto::hash
tx_details.m_state = wallet2::unconfirmed_transfer_details::failed;
// the inputs aren't spent anymore, since the tx failed
- for (size_t vini = 0; vini < tx_details.m_tx.vin.size(); ++vini)
- {
- if (tx_details.m_tx.vin[vini].type() == typeid(txin_to_key))
- {
- txin_to_key &tx_in_to_key = boost::get<txin_to_key>(tx_details.m_tx.vin[vini]);
- for (size_t i = 0; i < m_transfers.size(); ++i)
- {
- const transfer_details &td = m_transfers[i];
- if (td.m_key_image == tx_in_to_key.k_image)
- {
- LOG_PRINT_L1("Resetting spent status for output " << vini << ": " << td.m_key_image);
- set_unspent(i);
- break;
- }
- }
- }
- }
+ set_tx_key_images_spent(false/*spent*/);
}
}
}
Why this scored 57/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.