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

feat: kamt: Add public fn clear to reset and clear all entries in KAMT #2092

Merged
merged 4 commits into from
Dec 4, 2024

Conversation

snissn
Copy link
Contributor

@snissn snissn commented Dec 4, 2024

This PR introduces a new clear method to the Kamt data structure, allowing users to reset the root to an empty node and clear all entries.

Changes:

  • Added pub fn clear to Kamt to reset and clear all entries.
  • Updated the test suite with test_clear to verify:
    • Entries exist before clearing.
    • Entries are removed after calling clear.
    • The Kamt remains functional post-clear.

This addition enhances usability by providing an efficient way to clear the Kamt.

This functionality is introduced to support the implementation of Transient Storage (EIP-1153) in builtin-actors, enabling efficient clearing of the KAMT while adhering to Rust's RAII expectations and best practices. The previous approach attempts of using take/delete followed by new was unsuitable in Rust due to its ownership model and memory safety guarantees. The new clear method resolves this limitation, providing a safe and efficient way to reset the KAMT without requiring a full reallocation.

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.86%. Comparing base (2ded891) to head (04fee90).
Report is 2 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2092      +/-   ##
==========================================
+ Coverage   75.76%   75.86%   +0.09%     
==========================================
  Files         154      154              
  Lines       15712    15743      +31     
==========================================
+ Hits        11904    11943      +39     
+ Misses       3808     3800       -8     
Files with missing lines Coverage Δ
ipld/kamt/src/kamt.rs 89.09% <100.00%> (+4.30%) ⬆️

... and 3 files with indirect coverage changes

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

One nit but otherwise this seems reasonable.

Also, I'd love to have the same method on the HAMT/AMT.

ipld/kamt/src/kamt.rs Show resolved Hide resolved
@Stebalien Stebalien enabled auto-merge (squash) December 4, 2024 16:59
@Stebalien Stebalien merged commit e9eef91 into master Dec 4, 2024
14 checks passed
@Stebalien Stebalien deleted the mikers/KAMT-clear branch December 4, 2024 17:04
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.

3 participants