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

fix: various tree.rs issues and unit tests #7

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Conversation

distractedm1nd
Copy link
Contributor

@distractedm1nd distractedm1nd commented Apr 28, 2024

Set base as cleanup for now to make diff viewing easier.

tree.rs

Fixes:

  • double_tree_size panics when called from insert_node
  • generate_non_membership_proof returns a non-membership proof for members
  • update_node allows for modification of all fields, instead of just the value
  • missing API function to find node index by label without indirection

Unit Tests

  • test_new_with_size
  • test_membership_proofs
  • test_find_node_index
  • test_insert_node
  • test_update_node
  • test_find_leaf_by_label
  • test_double_tree_size
  • test_insert_node_doubles_tree_size
  • new
  • rehash_inner_nodes
  • rebuild_tree_from_leaves
  • calculate_root
  • get_root
  • get_commitment
  • find_leaf_by_label (Although used in tests, no direct test aims at this method)
  • generate_membership_proof (Though used in a test, it may lack a direct-focused test)
  • generate_non_membership_proof (Though used, direct-focused tests might be beneficial)
  • generate_proof (No corresponding method explicitly named, but related proof methods exist and have been partially tested)
  • insert_node (Tested, but more in-depth cases could be beneficial, e.g., edge cases)
  • update_node (Tested, but variations or edge cases might not be covered)
  • set_left_sibling_status_for_nodes (Utility function used but not directly tested)
  • resort_nodes_by_input_order

node.rs

Unit Tests

  • test_leaf_node_creation
  • test_inner_node_creation
  • test_node_default
  • test_node_is_active
  • test_node_set_left_sibling
  • test_leaf_node_activation
  • test_update_next_pointer
  • InnerNode::new - Implicitly tested through test_inner_node_creation, but no direct tests on certain edge cases (e.g., handling of extreme index values).
  • LeafNode::new - Implicitly tested through test_leaf_node_creation, however direct tests for hash generation logic or edge cases are missing.
  • Node::new_leaf - No direct unit test, but its outcome is tested indirectly within other tests.
  • Node::new_inner - Similarly, no direct test exists, but its result is implicitly checked.
  • Node::get_hash - Not directly tested, though its correctness is inferred through other tests.
  • Node::is_left_sibling - While test_node_set_left_sibling verifies setting this attribute, direct testing of initial values or changes over operations is missing.
  • Node::is_active - Covered by test_node_is_active, but scenarios of changing activity status aren't tested.
  • Node::set_left_sibling_value - Tested by test_node_set_left_sibling.
  • Node::set_node_active - Covered by test_leaf_node_activation.
  • Node::add_left and Node::add_right - The functionalities of these methods haven't been directly tested. They play a crucial role in tree restructuring but lack individual verification.
  • Node::update_next_pointer - Directly tested by test_update_next_pointer.
  • Node::generate_hash - Absence of direct tests, particularly concerning the accurate composition and subsequent hash generation for both inner and leaf nodes.

@distractedm1nd distractedm1nd self-assigned this Apr 28, 2024
Base automatically changed from cleanup to main May 1, 2024 06:22
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