What the developer wrote
Authored by tobtoht
Merge pull request #11460
5937d5c simplewallet: classify payment IDs by their payload (selsta)
ACKs: jpk68, tobtoht, plowsof
What changed, and why it matters
This commit simplifies how Monero's simplewallet decides whether a payment ID is a real encrypted ID or a dummy placeholder. Previously, the wallet checked both the payment ID value and whether any destination was an integrated address, and it would abort with an error if those two signals disagreed. Now it trusts only the actual payment ID value (a zero/empty value means dummy). The change removes a safety check that could reject otherwise-valid transactions, which could matter if the old check was too strict or could be triggered by unusual wallet/address combinations.
Review whether the removed assertion protected against any malformed or maliciously crafted transaction files. If the old check could be triggered by an attacker-supplied unsigned/loaded transaction, consider whether the payload-only check is sufficient. Otherwise, treat as a normal correctness/refactoring change and include it in routine testing.
Security signals we found
Removal of a CHECK_AND_ASSERT_MES consistency check between destination flags and payment ID value
Change from dual-factor classification (address type + payload) to payload-only classification
Potential for previously rejected transactions to be accepted if the old check was overly strict
Evidence from the diff
The patch removes a consistency assertion in accept_loaded_tx() that required the ‘is_integrated’ flag on transaction destinations to match whether the encrypted payment ID was the null hash. It replaces that two-factor check with a single check: payment_id8 == crypto::null_hash8. The removed code also iterated over all destinations to determine if any were integrated addresses. The new logic classifies the payment ID purely by its payload. This is described as a fix for classification of payment IDs, but the commit message does not frame it as a security fix.
Changed components
src/simplewallet/simplewallet.cppsimple_wallet::accept_loaded_tx()encrypted payment ID handling in transaction loadingInspect captured patch +1 / −9
### src/simplewallet/simplewallet.cpp
@@ -7728,15 +7728,7 @@ bool simple_wallet::accept_loaded_tx(const std::function<size_t()> get_num_txes,
if (!payment_id_string.empty())
payment_id_string += ", ";
- // if none of the addresses are integrated addresses, it's a dummy one
- bool is_dummy = true;
- for (const auto &e: cd.dests)
- if (e.is_integrated)
- is_dummy = false;
-
- CHECK_AND_ASSERT_MES(is_dummy == (payment_id8 == crypto::null_hash8), false, "Bad loaded tx: mismatched payment ID info");
-
- if (is_dummy)
+ if (payment_id8 == crypto::null_hash8)
{
payment_id_string += std::string("dummy encrypted payment ID");
}Why this scored 26/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.