WIP Active topic management #542
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RedBird96
requested review from
kpeluso,
guilherme-brandao and
relyt29
and removed request for
kpeluso and
guilherme-brandao
kpeluso
reviewed
Aug 24, 2024
|
The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).
|
xmariachi
reviewed
Aug 29, 2024
xmariachi
reviewed
Aug 29, 2024
relyt29
reviewed
Aug 29, 2024
relyt29
reviewed
Aug 29, 2024
relyt29
reviewed
Aug 29, 2024
relyt29
reviewed
Aug 29, 2024
relyt29
reviewed
Aug 29, 2024
## Purpose of Changes and their Description This is a refactoring PR with no state changes. It renames testify TestSuites to match with the packages they are actually testing. ## Are these changes tested and documented? These changes are tested by running them / our existing tests This is an internal change that does not require documentation.
relyt29
suggested changes
Aug 29, 2024
relyt29
approved these changes
Aug 30, 2024
kpeluso
reviewed
Aug 30, 2024
| @@ -1995,6 +1963,59 @@ func (k Keeper) GetIdsOfActiveTopics(ctx context.Context, pagination *types.Simp | |||
| }, nil | |||
| } | |||
|
|
|||
| // Returns active topic ids at block using pagination | |||
| func (k Keeper) GetIdsActiveTopicAtBlock(ctx context.Context, blockHeight int64, pagination *types.SimpleCursorPaginationRequest) ([]TopicId, *types.SimpleCursorPaginationResponse, error) { | |||
There was a problem hiding this comment.
do we really need pagination here?
There was a problem hiding this comment.
resolved in #557 + removed GetActiveTopics
kpeluso
approved these changes
Aug 30, 2024
|
Approved changes I have coming in: #557 |
|
Will also do changelog in a batch to add even more color to all recent PRs including this one |
This was referenced Aug 30, 2024
Merged
kpeluso
added a commit
that referenced
this pull request
Sep 3, 2024
# v0.4.0 ### Summary Implements fixes for our [June 2024](https://github.com/sherlock-audit/2024-06-allora-judging) Sherlock.xyz audit, including important fixes for determining which topics are considered active. ### Added * [#542](#542) Add scalable management of active topics with associated queries such as `GetActiveTopicsAtBlock` and `GetNextChurningBlockByTopicId` * [#556](#556) Scores now take an exponential moving average of the score rather than using the instantaneous score value from this epoch. * [#564](#564) Topic-epoch-length-aware effective revenue drip to ensure fairness of distribution between topics of longer and shorter epochs. ### Removed * [#542](#542) As part of active topic management, we removed `GetActiveTopics` and other (especially paginated) remnants of an unpartitioned store of active topics. ### Fixed * [#544](#544) Added check against zero-rewards after conversion to cosmosInt * [#547](#547) Improve error handling on InsertPayload, fixed/added tests err handling * [#550](#550) Fix reputer window upper limit * [#555](#555) Refactor: Rename TestSuite names * [#563](#563) Fix of the reputer losses length consistency in scores calculation ### Security * See our recent [June 2024](https://github.com/sherlock-audit/2024-06-allora-judging) security audit for a full description of bugs found during that audit. * [#554](#554) Check Signature on Worker or Reputer Payload Matches Inferer/Forecaster/Reputer inside Bundle --------- Signed-off-by: web3-nodeops <[email protected]> Signed-off-by: Tobi Okedeji <[email protected]> Signed-off-by: Fernando Campos <[email protected]> Signed-off-by: Guilherme Brandão <[email protected]> Co-authored-by: web3-nodeops <[email protected]> Co-authored-by: Guilherme Brandão <[email protected]> Co-authored-by: T <[email protected]> Co-authored-by: Diego C <[email protected]> Co-authored-by: Tobi Okedeji <[email protected]> Co-authored-by: RedBird96 <[email protected]> Co-authored-by: fernandofcampos <[email protected]> Co-authored-by: Elias Rad <[email protected]> Co-authored-by: Tyler <[email protected]> Co-authored-by: michael <[email protected]> Co-authored-by: Guilherme Brandão <[email protected]>
This was referenced Sep 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of Changes and their Description
Currently active topics are in one large keyset that must be parsed in pages in order to find topics.
This is not scalable as the number of topics increases.
Furthermore, there are some minor issues with respect to how topic weight is calculated -- and EMA is often used where the non-EMA may be more appropriate.
This PR attempts to address these concerns by segmenting the list of active topics by block height of the next epoch.
So, there is no longer 1 keyset for all active topics.
Instead, we map block height to a bounded list of topics.
When a topic is (re)activated, we append it to this list but knock off the topic of lowest weight to maintain a bounded set size.
We also introduce a cache for topic weight and lowest topic weight per "active set for a block" to minimize compute.
Link(s) to Ticket(s) or Issue(s) resolved by this PR
https://linear.app/upshot/issue/ORA-2068/patch-active-topic-parsing
Are these changes tested and documented?
Unreleasedsection ofCHANGELOG.md?Still Left Todo
Fill this out if this is a Draft PR so others can help.