Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Legacy payment ID's #46

Open
j-berman opened this issue Apr 26, 2024 · 11 comments
Open

Legacy payment ID's #46

j-berman opened this issue Apr 26, 2024 · 11 comments

Comments

@j-berman
Copy link

Would be nice if the scanner picked up payment ID's for users who use and have used the feature in the past.

Looks like it could be well suited for LegacyBasicEnoteRecord, LegacyIntermediateEnoteRecord, and LegacyEnoteRecord

Here's the logic wallet2 uses to identify and decrypt payment ID's: https://github.com/monero-project/monero/blob/c8214782fb2a769c57382a999eaf099691c836e7/src/wallet/wallet2.cpp#L2699-L2761

@UkoeHB
Copy link
Owner

UkoeHB commented Apr 26, 2024

It's part of the tx memo in the enote origin context already.

@j-berman
Copy link
Author

It's in ciphertext there. Plus it relies on other data in the tx in order to decrypt and utilize the same as wallet2.

It could be done at a different time than scanning but scanning feels like a good time to do it. We'll want that logic from wallet2 implemented somewhere.

@UkoeHB
Copy link
Owner

UkoeHB commented Apr 26, 2024

Hmm I would prefer for this to be handled by downstream consumers of enote store events. The payment ID is not strictly part of the enote, it is extra information passed along with a tx. Jamming that handling into the existing scan/enote store flow just feels like spaghettifying an already very complex design. I realize it's very tempting to just compromise a little and shove this in, but clean design like in the Seraphis library comes from being painfully strict. Otherwise you suffer death by a thousand cuts (aka wallet2).

@j-berman
Copy link
Author

It's moving the spaghetti somewhere else, but ok

@UkoeHB
Copy link
Owner

UkoeHB commented Apr 26, 2024

The enote store was never designed to be the 'complete source of all information you can ever want', because that task is simply too big. It's already quite complex just handling enote management.

The idea was always for there to be downstream consumers of enote store events that manage more user-facing information (such as payment IDs).

@UkoeHB
Copy link
Owner

UkoeHB commented Apr 26, 2024

It's moving the spaghetti somewhere else, but ok

This implies there is no solution without spaghetti, which surely isn't true. Decouple and isolate

@j-berman
Copy link
Author

I don't agree that it would spaghettify the scanner any more so than it would spagettify any other place it's placed, but will see how it goes implementing downstream

@UkoeHB
Copy link
Owner

UkoeHB commented Apr 26, 2024

That's fair. I will leave this issue open. If it is truly not a good fit for downstream, then we can revisit.

@jeffro256
Copy link

Payment IDs were the only official way to discriminate senders of received payments until subaddresses, and are still used today for integrated addresses. As such, I think that payment IDs are in scope at least for the enote store which needs to maintain balance recovery information.

@jeffro256
Copy link

They're encrypted inside tx_extra which would mean you would need to bring your private key key online to see stored payment IDs, even after the enotes are already scanned which isn't ideal IMO. I also don't like the messiness of it, but since payment IDs was an official feature of addressing, I believe it should be supported

@UkoeHB
Copy link
Owner

UkoeHB commented Sep 20, 2024

I also don't like the messiness of it, but since payment IDs was an official feature of addressing, I believe it should be supported

Architecturally the issue is don't you need a bunch of other information for the enote store to be useful to a wallet? Information the enote store can't feasibly handle without becoming overburdened. If that's the case, then you need to be thinking how to build a companion for the enote store to manage that extra data, and I assume payment IDs would fall in place there.

I forget the whole workflow, but IIRC the scanning scaffolding would let you stick more than an enote store in there.

Of course, the greatest danger is bugs from having multiple potentially unsynchronized representations of wallet state at once. It's a challenging design space and I won't claim to have it solved. But I'm definitely reluctant to expand the enote store, since it was a huge pain to get right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants