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

[Sumtree]: Implement prefix sum algorithm #83

Merged
merged 14 commits into from
Apr 3, 2024
Merged

Conversation

AlpinYukseloglu
Copy link
Collaborator

Closes: #76

What is the purpose of the change

This PR implements the prefix sum algorithm as defined by our spec. Specifically, it implements the function get_prefix_sum which gives the sum of all leaves up until the passed in ETAS value.

Testing and Verifying

[WIP] The intended testing strategy here is:
Testing strategy:

  1. Create a sumtree
  2. Run a bunch of insertions as in test_node_deletion_valid setup
  3. Whenever an order is inserted with an ETAS <= our target for the test case, add its amount to the expected prefix sum
  4. Run prefix sum at target ETAS on tree
  5. Ensure equal to expected amount

Hopefully we can do a follow up PR to fuzz generate test cases here

@AlpinYukseloglu AlpinYukseloglu changed the title [WIP][Sumtree]: Implement prefix sum algorithm [Sumtree]: Implement prefix sum algorithm Apr 2, 2024
@@ -562,12 +562,12 @@ impl TreeNode {
/// have been performed. It checks the balance factor of the current node and performs rotations
/// as necessary to bring the tree back into balance.
pub fn rebalance(&mut self, storage: &mut dyn Storage) -> ContractResult<()> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: all the changes in node.rs and test_node.rs are driveby cleanup from tree debugging w @crnbarr93

Copy link
Collaborator

@crnbarr93 crnbarr93 left a comment

Choose a reason for hiding this comment

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

Looks good! Comments were extremely useful in following the implementation. Two small nit changes otherwise no problems.

contracts/sumtree-orderbook/src/sumtree/test/test_tree.rs Outdated Show resolved Hide resolved
contracts/sumtree-orderbook/src/sumtree/tree.rs Outdated Show resolved Hide resolved
@crnbarr93 crnbarr93 merged commit f89a55e into main Apr 3, 2024
2 checks passed
@crnbarr93 crnbarr93 deleted the alpo/prefix-sum branch April 3, 2024 20:38
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.

[Sumtree]: Implement prefix sum query for sumtree
2 participants