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

multi: Make JSON-RPC rescan docs match reality. #3032

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Dec 2, 2022

The semantics for discovering transactions that involve specific addresses and outpoints that a client is interested in changed some time ago to be based on loading transaction filters.

It appears that the JSON-RPC API documentation was not updated to match the new reality at that time, so this updates the documentation accordingly.

In particular, it updates the JSON-RPC API documentation for the loadtxfilter, notifynewtransactions, and rescan methods as well as the blockconnected and blockdisconnected notifications to match the correct parameters and semantics.

Next, since rescan now blocks until it finishes and therefore no longer is involved with or sends notifications, this removes references to rescan from all notifications and removes the longer available rescanprogress and rescanfinished notifications.

Finally, it adds documentation for the releveanttxaccepted notification sent in response to a transaction that matches the loaded transaction filter being added to the mempool.

@davecgh davecgh added the documentation Issues and/or pull requests related to documentation. label Dec 2, 2022
@davecgh davecgh added this to the 1.8.0 milestone Dec 2, 2022
@davecgh davecgh changed the title multi: Make JSON-RPC rescan docs to match reality. multi: Make JSON-RPC rescan docs match reality. Dec 2, 2022
@davecgh davecgh force-pushed the multi_update_rescan_help_docs branch from c17f2ad to f07dbec Compare December 2, 2022 02:55
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

LGTM.

Orthogonal to this PR, but I feel we don't really need rescan in dcrd anymore, now that we have CFilters (we can't drop it yet though, because dcrw still uses it in rpc mode).

@davecgh
Copy link
Member Author

davecgh commented Dec 2, 2022

LGTM.

Orthogonal to this PR, but I feel we don't really need rescan in dcrd anymore, now that we have CFilters (we can't drop it yet though, because dcrw still uses it in rpc mode).

Perhaps. I will note that recsan is more efficient than using cfilters though because cfilters don't tell you which transaction matched, only than one of them in the block matched w.h.p. You then need to acquire the entire block from somewhere and loop all of the inputs and outputs of all of the transactions in that block to find which ones are relevant. Rescan, on the other hand, essentially directly does the latter server side and gives the actual serialized transactions that match.

@matheusd
Copy link
Member

matheusd commented Dec 2, 2022

Perhaps. I will note that recsan is more efficient than using cfilters though because cfilters don't tell you which transaction matched, only than one of them in the block matched w.h.p. Rescan, on the other hand, gives the actual serialized transactions that match.

We'd have to measure to figure out the performance points of when it makes sense to use cfilters vs rescan, in terms of size of the txFilter, expected frequency of match, average size of blocks, etc.

@davecgh
Copy link
Member Author

davecgh commented Dec 2, 2022

I haven't measured it, but I can tell you from actual usage experience that wallets that are super busy (and hence have matches in almost every block) become extremely slow over SPV while they remain quite snappy with rescan+RPC.

The semantics for discovering transactions that involve specific
addresses and outpoints that a client is interested in changed some time
ago to be based on loading transaction filters.

It appears that the JSON-RPC API documentation was not updated to match
the new reality at that time, so this updates the documentation
accordingly.

In particular, it updates the JSON-RPC API documentation for the
loadtxfilter, notifynewtransactions, and rescan methods as well as the
blockconnected and blockdisconnected notifications to match the correct
parameters and semantics.

Next, since rescan now blocks until it finishes and therefore no
longer is involved with or sends notifications, this removes references
to rescan from all notifications and removes the longer available
rescanprogress and rescanfinished notifications.

Finally, it adds documentation for the releveanttxaccepted notification
sent in response to a transaction that matches the loaded transaction
filter being added to the mempool.
@davecgh davecgh force-pushed the multi_update_rescan_help_docs branch from f07dbec to 21f1365 Compare December 5, 2022 17:23
@davecgh davecgh merged commit 21f1365 into decred:master Dec 5, 2022
@davecgh davecgh deleted the multi_update_rescan_help_docs branch December 5, 2022 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues and/or pull requests related to documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants