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

Support retrieving newer txs only in light wallet REST API #603

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ndorf
Copy link
Contributor

@ndorf ndorf commented Aug 23, 2021

Currently, the light wallet REST API only supports retrieving the entire transaction history. This proposed extension to the protocol allows clients to request only those transactions newer than the ones they already know.

Two optional fields are added to the get_address_txs request object: since_tx_id, the highest transaction id known to the client, and since_tx_block_hash, the block_hash of the same transaction. If these fields are absent, the entire transaction history is returned, as before.

A since_tx_id field is also added to the response object; its value may differ from the value in the request, and clients must be sure to honor the former, by forgetting any txs with a higher id before processing the response.

This logic, along with the new block hash fields, allow detection and correct handling of blockchain reorgs. Upon receiving the request, the server compares the provided tx id and block_hash with the current blockchain state. If they match, then the server can simply return the newer txs, exactly as requested, and the since_tx_id of the response should be the same as the request. If they don't match, then the specified transaction must have been part of a blockchain reorg, in which case some or all of the previous history must be returned, and the since_tx_id of the response will differ from the request.

If the server keeps track of reorged blocks, and where they previously were in the blockchain, then it can return only those txs that are newer than the split. (In this case the returned since_tx_id should be the highest id in the last block common to both chains.) However, it is not obligated to do so: it can simply return the entire transaction history anytime a reorg is detected (in which case the returned since_tx_id should be zero.)

Requesting feedback from @vtnerd and @moneroexamples

@paulshapiro
Copy link

I support deprecating the full list and calling this standard in clients.

@ndorf ndorf force-pushed the since_tx branch 2 times, most recently from b89b684 to 2e02650 Compare August 23, 2021 02:44
@moneroexamples
Copy link

Yes, I was proposing something similar like that before as well. There is no reason to scan entire blockchain if you know that your wallet is 3 days old. You could also perhaps just return always at minim last 10 translations or so, to protect against reorgs?

@ndorf
Copy link
Contributor Author

ndorf commented Aug 28, 2021

@moneroexamples this is orthogonal to scanning, it only relates to the client's retrieval of tx history.

@AAH20
Copy link

AAH20 commented Aug 30, 2021

check this monero-project/monero#7896
to enforce your contributions with latest api security standards , also feel free to share your opinion on the overall security and transparency situation of the project.

Copy link
Contributor

@vtnerd vtnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is sensible, and should be possible in monero-lws without much hassle. The implementation will probably do a full re-sync on a reorg, as tracking fork points might be more trouble than its worth.

api/lightwallet_rest.md Outdated Show resolved Hide resolved
@j-berman
Copy link

j-berman commented Sep 11, 2021

Adding: I think this is a good idea! Spec looks good to me.

EDIT: nevermind below, sorry. It seems it would come at the cost of an additional DB read, so not worth it.

Small thing, unless I'm misunderstanding, it seems a little confusing that in the reorg case, the updated since_tx_id could be the highest tx in a block. Might it be a tiny bit more intuitive to return the user's highest tx_id from before the split (edit: and 0 if none)? I feel like a consumer of the API would find it marginally easier to grok, but doesn't really make a big difference either way.

@ndorf
Copy link
Contributor Author

ndorf commented Jan 8, 2022

it can simply return the entire transaction history anytime a reorg is detected (in which case the returned since_tx_id should be zero.)

Changed the wording to explicitly allow simply omitting this field altogether; the client needs to handle that case anyway, for backward compatibility.

@ndorf ndorf marked this pull request as ready for review January 8, 2022 01:36
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

Successfully merging this pull request may close these issues.

6 participants