-
Notifications
You must be signed in to change notification settings - Fork 0
feat: reintroduce TopN #9
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
Conversation
repeated interchain_security.ccv.v1.ConsumerPacketData list = 1 | ||
[ (gogoproto.nullable) = false ]; | ||
} | ||
syntax = "proto3"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this invalid duplicate got here (I haven't looked on git blame yet) but proto was erroring out and I deleted to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it looks like you added the minimum infrastructure necessary to get TopN in there. And the pr is very modular and clean.
|
||
return !iterator.Valid() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good 👍 and I'm assuming you didn't change anything in them just brought them back.
Restore TopN Functionality for AtomOne
What this PR does
Brings back TopN functionality from v5.2.0 after we stripped out all the PSS stuff. This gives us:
What we intentionally left out
We're NOT including these PSS features because of the subset problem concerns:
These add complexity and have fundamental issues that need more thought.
Key changes
Protos
top_N
andmin_power_in_top_N
to Chain query messageCore logic
partial_set_security.go
- restored just the TopN computation partsgrpc_query.go
- GetConsumerChain returns TopN infoFor AtomOne