wallet_rpc_server: preserve payment ID when editing address book
What the developer wrote
Authored by selsta
wallet_rpc_server: preserve payment ID when editing address book
What changed, and why it matters
This commit fixes a bug in Monero's wallet RPC server where editing an address book entry could accidentally drop or mishandle the payment ID associated with an integrated address. The fix ensures the payment ID flag is preserved correctly, and new tests check that converting between integrated and standard addresses works as expected. In practical terms, this could have caused users to send funds without the intended payment identifier, making transactions harder to track or reconcile.
Review whether the stale payment ID state could have caused incorrect transaction construction or metadata loss in production wallets, and consider backporting the fix to maintained release branches. No immediate emergency response is indicated, but users relying on integrated addresses and payment IDs should update once a release is available.
Security signals we found
State inconsistency between payment ID flag and value in address book edit path
Conditional pointer pass could leave stale payment ID metadata attached to a non-integrated address
Functional test additions confirm behavior change for integrated/standard address transitions
Evidence from the diff
In wallet_rpc_server.cpp’s edit_address_book handler, the previous code only updated entry.m_payment_id when info.has_payment_id was true, but did not update entry.m_has_payment_id. When set_address was true but the new address was not integrated, entry.m_has_payment_id could remain true while entry.m_payment_id was stale or null, and the call to set_address_book_row used a conditional that passed a pointer to the payment ID only when both req.set_address and entry.m_has_payment_id were true. The patch now always assigns entry.m_has_payment_id and entry.m_payment_id (using crypto::null_hash8 when absent), and always passes the payment ID pointer when entry.m_has_payment_id is true, regardless of whether the address field itself is being changed. Functional tests were added to cover changing an integrated address to a standard address and to another integrated address.
Changed components
src/wallet/wallet_rpc_server.cppwallet RPC edit_address_book endpointtests/functional_tests/address_book.pyInspect captured patch +15 / −3
diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp
index f19e645..6be821e 100644
--- a/src/wallet/wallet_rpc_server.cpp
+++ b/src/wallet/wallet_rpc_server.cpp
@@ -3347,14 +3347,14 @@ namespace tools
}
entry.m_address = info.address;
entry.m_is_subaddress = info.is_subaddress;
- if (info.has_payment_id)
- entry.m_payment_id = info.payment_id;
+ entry.m_has_payment_id = info.has_payment_id;
+ entry.m_payment_id = info.has_payment_id ? info.payment_id : crypto::null_hash8;
}
if (req.set_description)
entry.m_description = req.description;
- if (!m_wallet->set_address_book_row(req.index, entry.m_address, req.set_address && entry.m_has_payment_id ? &entry.m_payment_id : NULL, entry.m_description, entry.m_is_subaddress))
+ if (!m_wallet->set_address_book_row(req.index, entry.m_address, entry.m_has_payment_id ? &entry.m_payment_id : NULL, entry.m_description, entry.m_is_subaddress))
{
er.code = WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR;
er.message = "Failed to edit address book entry";
diff --git a/tests/functional_tests/address_book.py b/tests/functional_tests/address_book.py
index c6090e2..94989a1 100755
--- a/tests/functional_tests/address_book.py
+++ b/tests/functional_tests/address_book.py
@@ -231,6 +231,18 @@ class AddressBookTest():
res = wallet.get_address_book([1])
assert len(res.entries) == 1
assert e == res.entries[0]
+ # change integrated address to standard address
+ res = wallet.edit_address_book(2, address = '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm')
+ res = wallet.get_address_book([2])
+ e = res.entries[0]
+ assert e.address == '42ey1afDFnn4886T7196doS9GPMzexD9gXpsZJDwVjeRVdFCSoHnv7KPbBeGpzJBzHRCAs9UxqeoyFQMYbqSWYTfJJQAWDm'
+ # change integrated address to another integrated address
+ res = wallet.make_integrated_address()
+ integrated_address_2 = res.integrated_address
+ res = wallet.edit_address_book(2, address = integrated_address_2)
+ res = wallet.get_address_book([2])
+ e = res.entries[0]
+ assert e.address == integrated_address_2
# empty
wallet.delete_address_book(0)
Why this scored 37/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.