← Watch feed
simplewallet: use passed decrypted payment ID from wallet2 for notifications
Public commit record
65/100 · Adequate
What the developer wrote
Authored by jeffro256
simplewallet: use passed decrypted payment ID from wallet2 for notifications
Adds payment ID paramater to `on_money_received()` wallet callback.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides an explanatory body
What changed, and why it matters
This Monero commit changes how payment IDs are handled when the wallet receives money. Previously, the user-facing wallet interface (simplewallet) decrypted the payment ID itself from transaction data. Now, the core wallet engine (wallet2) decrypts or extracts the payment ID earlier in processing and passes the already-decrypted value to the interface. This is mostly a code cleanup and consistency improvement. It does not appear to be a security fix on its own, but it touches privacy-sensitive payment ID handling and removes duplicated decryption logic.
Recommended action
Treat as a routine refactor with no immediate security action required. Reviewers should verify that the moved payment ID extraction preserves behavior for all transaction types (encrypted 8-byte IDs, obsolete unencrypted 256-bit IDs, and transactions without payment IDs), and that downstream consumers of on_money_received() correctly handle the new parameter. If this commit is part of a larger series, evaluate the combined effect on payment ID privacy and correctness.
Security signals we found
01
Refactors payment ID decryption flow between wallet core and UI layer
02
Removes duplicated payment ID extraction logic from simplewallet callback
03
Adds new parameter to i_wallet2_callback::on_money_received() interface
04
Touches privacy-relevant metadata (payment IDs) but does not change cryptographic operations
05
No bounds-checking, memory-safety, or input-validation changes evident
Evidence from the diff
The commit refactors payment ID extraction in wallet2::process_new_transaction(). It moves the tx_extra_nonce parsing and payment ID decryption from the later tx_money_got_in_outs block to an earlier point where transaction public keys are available, then passes the resulting crypto::hash payment_id through the i_wallet2_callback::on_money_received() signature to simplewallet and the wallet API. simplewallet now uses the passed payment_id directly via memcpy instead of re-deriving and decrypting it from tx_pub_key and view key. The change is structural: it centralizes payment ID handling in wallet2 and ensures callbacks receive a consistent value. No obvious vulnerability is fixed or introduced in the diff, though the refactor could be preparatory for later fixes.
Changed components
src/wallet/wallet2.cppsrc/wallet/wallet2.hsrc/simplewallet/simplewallet.cppsrc/simplewallet/simplewallet.hsrc/wallet/api/wallet.cppInspect captured patch +60 / −57
diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp
index 7ccc22e..9c2b135 100644
--- a/src/simplewallet/simplewallet.cpp
+++ b/src/simplewallet/simplewallet.cpp
@@ -5544,7 +5544,10 @@ void simple_wallet::on_new_block(uint64_t height, const cryptonote::block& block
m_refresh_progress_reporter.update(height, false);
}
//----------------------------------------------------------------------------------------------------
-void simple_wallet::on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, bool is_change, uint64_t unlock_time)
+void simple_wallet::on_money_received(uint64_t height, const crypto::hash &txid,
+ const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt,
+ const cryptonote::subaddress_index& subaddr_index, const crypto::hash &payment_id, bool is_change,
+ uint64_t unlock_time)
{
if (m_locked)
return;
@@ -5564,16 +5567,13 @@ void simple_wallet::on_money_received(uint64_t height, const crypto::hash &txid,
std::vector<tx_extra_field> tx_extra_fields;
parse_tx_extra(tx.extra, tx_extra_fields); // failure ok
tx_extra_nonce extra_nonce;
- tx_extra_pub_key extra_pub_key;
crypto::hash8 payment_id8 = crypto::null_hash8;
- if (find_tx_extra_field_by_type(tx_extra_fields, extra_pub_key))
{
- const crypto::public_key &tx_pub_key = extra_pub_key.pub_key;
if (find_tx_extra_field_by_type(tx_extra_fields, extra_nonce))
{
if (get_encrypted_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id8))
{
- m_wallet->get_account().get_device().decrypt_payment_id(payment_id8, tx_pub_key, m_wallet->get_account().get_keys().m_view_secret_key);
+ memcpy(payment_id8.data, payment_id.data, sizeof(payment_id8));
}
}
}
diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h
index 76ece1b..651a7a0 100644
--- a/src/simplewallet/simplewallet.h
+++ b/src/simplewallet/simplewallet.h
@@ -338,7 +338,7 @@ namespace cryptonote
//----------------- i_wallet2_callback ---------------------
virtual void on_new_block(uint64_t height, const cryptonote::block& block);
- virtual void on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, bool is_change, uint64_t unlock_time);
+ virtual void on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, const crypto::hash &payment_id, bool is_change, uint64_t unlock_time);
virtual void on_unconfirmed_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, const cryptonote::subaddress_index& subaddr_index);
virtual void on_money_spent(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& in_tx, uint64_t amount, const cryptonote::transaction& spend_tx, const cryptonote::subaddress_index& subaddr_index);
virtual void on_skip_transaction(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx);
diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp
index 96ac45a..e398c1a 100644
--- a/src/wallet/api/wallet.cpp
+++ b/src/wallet/api/wallet.cpp
@@ -194,7 +194,10 @@ struct Wallet2CallbackImpl : public tools::i_wallet2_callback
}
}
- virtual void on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, bool is_change, uint64_t unlock_time)
+ virtual void on_money_received(uint64_t height, const crypto::hash &txid,
+ const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt,
+ const cryptonote::subaddress_index& subaddr_index, const crypto::hash &payment_id,
+ bool is_change, uint64_t unlock_time)
{
std::string tx_hash = epee::string_tools::pod_to_hex(txid);
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index e149a91..20d29a3 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -2323,6 +2323,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
}
}
const std::vector<tx_extra_field> &tx_extra_fields = tx_cache_data.tx_extra_fields.empty() ? local_tx_extra_fields : tx_cache_data.tx_extra_fields;
+ crypto::hash payment_id = crypto::null_hash;
// Don't try to extract tx public key if tx has no ouputs
size_t pk_index = 0;
@@ -2490,6 +2491,52 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
}
}
+ tx_extra_nonce extra_nonce;
+ if (find_tx_extra_field_by_type(tx_extra_fields, extra_nonce))
+ {
+ crypto::hash8 payment_id8 = null_hash8;
+ if(get_encrypted_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id8))
+ {
+ // We got a payment ID to go with this tx
+ LOG_PRINT_L2("Found encrypted payment ID: " << payment_id8);
+ MINFO("Consider using subaddresses instead of encrypted payment IDs");
+ if (tx_pub_key != null_pkey)
+ {
+ if (!m_account.get_device().decrypt_payment_id(payment_id8, tx_pub_key, m_account.get_keys().m_view_secret_key))
+ {
+ LOG_PRINT_L0("Failed to decrypt payment ID: " << payment_id8);
+ }
+ else
+ {
+ LOG_PRINT_L2("Decrypted payment ID: " << payment_id8);
+ // put the 64 bit decrypted payment id in the first 8 bytes
+ memcpy(payment_id.data, payment_id8.data, 8);
+ // rest is already 0, but guard against code changes above
+ memset(payment_id.data + 8, 0, 24);
+ }
+ }
+ else
+ {
+ LOG_PRINT_L1("No public key found in tx, unable to decrypt payment id");
+ }
+ }
+ else if (get_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id))
+ {
+ bool ignore = block_version >= IGNORE_LONG_PAYMENT_ID_FROM_BLOCK_VERSION;
+ if (ignore)
+ {
+ LOG_PRINT_L2("Found unencrypted payment ID in tx " << txid << " (ignored)");
+ MWARNING("Found OBSOLETE AND IGNORED unencrypted payment ID: these are bad for privacy, use subaddresses instead");
+ payment_id = crypto::null_hash;
+ }
+ else
+ {
+ LOG_PRINT_L2("Found unencrypted payment ID: " << payment_id);
+ MWARNING("Found unencrypted payment ID: these are bad for privacy, consider using subaddresses instead");
+ }
+ }
+ }
+
for(size_t o: outs)
{
THROW_WALLET_EXCEPTION_IF(tx.vout.size() <= o, error::wallet_internal_error, "wrong out in transaction: internal index=" +
@@ -2570,7 +2617,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
}
LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid);
if (!ignore_callbacks && 0 != m_callback)
- m_callback->on_money_received(height, txid, tx, td.m_amount, 0, td.m_subaddr_index, spends_one_of_ours(tx), td.m_tx.unlock_time);
+ m_callback->on_money_received(height, txid, tx, td.m_amount, 0, td.m_subaddr_index, payment_id, spends_one_of_ours(tx), td.m_tx.unlock_time);
}
total_received_1 += amount;
notify = true;
@@ -2648,7 +2695,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid);
if (!ignore_callbacks && 0 != m_callback)
- m_callback->on_money_received(height, txid, tx, td.m_amount, burnt, td.m_subaddr_index, spends_one_of_ours(tx), td.m_tx.unlock_time);
+ m_callback->on_money_received(height, txid, tx, td.m_amount, burnt, td.m_subaddr_index, payment_id, spends_one_of_ours(tx), td.m_tx.unlock_time);
}
total_received_1 += extra_amount;
notify = true;
@@ -2836,53 +2883,6 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
// create payment_details for each incoming transfer to a subaddress index
if (tx_money_got_in_outs.size() > 0)
{
- tx_extra_nonce extra_nonce;
- crypto::hash payment_id = null_hash;
- if (find_tx_extra_field_by_type(tx_extra_fields, extra_nonce))
- {
- crypto::hash8 payment_id8 = null_hash8;
- if(get_encrypted_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id8))
- {
- // We got a payment ID to go with this tx
- LOG_PRINT_L2("Found encrypted payment ID: " << payment_id8);
- MINFO("Consider using subaddresses instead of encrypted payment IDs");
- if (tx_pub_key != null_pkey)
- {
- if (!m_account.get_device().decrypt_payment_id(payment_id8, tx_pub_key, m_account.get_keys().m_view_secret_key))
- {
- LOG_PRINT_L0("Failed to decrypt payment ID: " << payment_id8);
- }
- else
- {
- LOG_PRINT_L2("Decrypted payment ID: " << payment_id8);
- // put the 64 bit decrypted payment id in the first 8 bytes
- memcpy(payment_id.data, payment_id8.data, 8);
- // rest is already 0, but guard against code changes above
- memset(payment_id.data + 8, 0, 24);
- }
- }
- else
- {
- LOG_PRINT_L1("No public key found in tx, unable to decrypt payment id");
- }
- }
- else if (get_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id))
- {
- bool ignore = block_version >= IGNORE_LONG_PAYMENT_ID_FROM_BLOCK_VERSION;
- if (ignore)
- {
- LOG_PRINT_L2("Found unencrypted payment ID in tx " << txid << " (ignored)");
- MWARNING("Found OBSOLETE AND IGNORED unencrypted payment ID: these are bad for privacy, use subaddresses instead");
- payment_id = crypto::null_hash;
- }
- else
- {
- LOG_PRINT_L2("Found unencrypted payment ID: " << payment_id);
- MWARNING("Found unencrypted payment ID: these are bad for privacy, consider using subaddresses instead");
- }
- }
- }
-
uint64_t total_received_2 = sub_change;
for (const auto& i : tx_money_got_in_outs)
total_received_2 += i.second;
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index ff6111f..0dc35fc 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -131,7 +131,7 @@ private:
// Full wallet callbacks
virtual void on_new_block(uint64_t height, const cryptonote::block& block) {}
virtual void on_reorg(uint64_t height, uint64_t blocks_detached, size_t transfers_detached) {}
- virtual void on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, bool is_change, uint64_t unlock_time) {}
+ virtual void on_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, uint64_t burnt, const cryptonote::subaddress_index& subaddr_index, const crypto::hash &payment_id, bool is_change, uint64_t unlock_time) {}
virtual void on_unconfirmed_money_received(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx, uint64_t amount, const cryptonote::subaddress_index& subaddr_index) {}
virtual void on_money_spent(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& in_tx, uint64_t amount, const cryptonote::transaction& spend_tx, const cryptonote::subaddress_index& subaddr_index) {}
virtual void on_skip_transaction(uint64_t height, const crypto::hash &txid, const cryptonote::transaction& tx) {}
Risk score
Our methodology →Why this scored 27/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.