issue-#170#172
Merged
Merged
Conversation
rem1niscence
approved these changes
May 5, 2025
rem1niscence
left a comment
Collaborator
There was a problem hiding this comment.
approved! even though a lot was touched, seems like the same final behavior is kept and is more like a big refactor.
This file contains hidden or 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
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.
Describe the bug
The order book structure is stored inefficiently and needs to be redone before mainnet:
Specifically
The structure of the order books as a list vs individual unique orders is estimated to have an unacceptable space complexity
Expected behavior
Change the id of the sell order
And change the order book key to a prefix.
to
and add
then finally change the order book logic to utilize the new design, where orders are stored individually (not a slice) and order books are created dynamically by iterating over the
prefixfor the committee.Additional context
This is technically a consensus breaking issue, but because there's been no orders so far -- we should be fine to do this change on the fly.
closes #170