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: Find voted/revoked tickets with GCS filters #413

Merged
merged 2 commits into from
Aug 26, 2023

Conversation

jholdstock
Copy link
Member

@jholdstock jholdstock commented Aug 24, 2023

Use dcrd and GCS filters to find voted/revoked tickets rather than using the dcrwallet TicketInfo RPC.

Using TicketInfo was a bit flakey because wallets do not always correctly detect votes/revokes, and as a result VSP admins may notice that with this change vspd detects some historic voted/revoked tickets which TicketInfo never detected.

Contributes to #268

cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
rpc/dcrd.go Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
Simnet params were added in the very first commit of vspd purely because
of copy/pasting from another project.

vspd has never actually been tested on simnet, so its possible
(probable) that it won't even operate correctly without additional dev
work. If we want simnet to be a properly supported network it can be
re-added and tested in the future.
@jholdstock
Copy link
Member Author

jholdstock commented Aug 25, 2023

Thanks @davecgh. Performance isn't critical here, but its great to have your suggestions because there are (rare) cases where vspd might need to rescan several thousand blocks. I added some temporary logs for timing and the improvement is notable.

Before

[DBG] VSP: Scanning 46552 blocks for 477 spent tickets
[DBG] VSP: took 2m10.727347019s

After

[DBG] VSP: Scanning 46554 blocks for 477 spent tickets
[DBG] VSP: took 28.316309518s

This PR now contains three commits - the first is a new one to remove simnet params because they are completely unused/untested and I was too lazy to figure out how to handle the DCP-0005 activation height. The middle commit contains exactly the same changes you already reviewed, and the third is a new one to implement your suggestions.

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice. Glad to hear the suggestions helped.

cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
cmd/vspd/spentticket.go Outdated Show resolved Hide resolved
@davecgh
Copy link
Member

davecgh commented Aug 25, 2023

... I was too lazy to figure out how to handle the DCP-0005 activation height.

That one is easy. It's always active on simnet.

Use dcrd and GCS filters to find voted/revoked tickets rather than using
the dcrwallet TicketInfo RPC.

Using TicketInfo was a bit flakey because wallets do not always
correctly detect votes/revokes, and as a result VSP admins may notice
that with this change vspd detects some historic voted/revoked tickets
which TicketInfo never detected.
@jholdstock jholdstock merged commit 9be203c into decred:master Aug 26, 2023
2 checks passed
@davecgh
Copy link
Member

davecgh commented Aug 26, 2023

Nice job overall and it looks good now. It's much more robust than what it's replacing and with the latest updates should stay reasonably efficient.

As an aside, if you ultimately end up needing more performance, you could use filter.MatchAny instead because it saves a lot of time on the filter matching itself when looking for multiple things like this. To do that though, you'd need to restructure your code slightly in this section to use slices instead of a map. That's why I didn't think it was necessary for this PR, but figured I'd mention it in case you ultimately need to speed it up more.

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.

2 participants