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

incrementalmerkletree: Enable use in no_std environments. #126

Merged
merged 5 commits into from
Jan 28, 2025

Conversation

nuttycom
Copy link
Contributor

No description provided.

@nuttycom nuttycom force-pushed the incrementalmerkletree_no_std branch from a39e7ba to 931112e Compare January 27, 2025 23:03
@nuttycom nuttycom force-pushed the incrementalmerkletree_no_std branch from 931112e to e9fe0de Compare January 28, 2025 04:31
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK e9fe0de. I confirmed that this PR contains the same commit as was merged to the hotfix branch in #125. However, I would have preferred this PR to merge the hotfix branch back into main instead of creating a new merge commit.

@@ -673,14 +674,15 @@ impl<H: Hashable + Clone, const DEPTH: u8> CommitmentTree<H, DEPTH> {
}
}

#[cfg(any(test, feature = "test-dependencies"))]
#[cfg(any(all(test, feature = "std"), feature = "test-dependencies"))]
Copy link
Contributor

@daira daira Jan 28, 2025

Choose a reason for hiding this comment

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

This is confusing to me. cargo test seems to run the same tests with or without --features std, but given this and line 768 below, I don't understand how it is doing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can see the difference here:

~/work/incrementalmerkletree/canon on ᚠincrementalmerkletree_no_std [$] via 🦀 v1.64.0 on ☁️  [email protected](us-east1) took 6s
λ cargo test -p incrementalmerkletree
   Compiling incrementalmerkletree v0.8.1 (/home/nuttycom/work/incrementalmerkletree/canon/incrementalmerkletree)
    Finished test [unoptimized + debuginfo] target(s) in 0.44s
     Running unittests src/lib.rs (target/debug/deps/incrementalmerkletree-062d7fb7497f60ec)

running 14 tests
test tests::addr_above_position ... ok
test tests::addr_context ... ok
test tests::addr_children ... ok
test tests::addr_common_ancestor ... ok
test tests::addr_is_ancestor ... ok
test tests::addr_is_ancestor_of ... ok
test tests::addr_position_range ... ok
test tests::address_current_incomplete ... ok
test tests::address_next_incomplete_parent ... ok
test tests::merkle_path_root ... ok
test tests::position_is_complete_subtree ... ok
test tests::position_past_ommer_count ... ok
test tests::position_root_level ... ok
test tests::position_witness_addrs ... ok

test result: ok. 14 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests incrementalmerkletree

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s


~/work/incrementalmerkletree/canon on ᚠincrementalmerkletree_no_std [$] via 🦀 v1.64.0 on ☁️  [email protected](us-east1)
λ cargo test --all-features -p incrementalmerkletree
    Finished test [unoptimized + debuginfo] target(s) in 0.02s
     Running unittests src/lib.rs (target/debug/deps/incrementalmerkletree-388dc0338e6c93b4)

running 25 tests
test frontier::tests::frontier_from_parts ... ok
test frontier::tests::frontier_root ... ok
test frontier::tests::nonempty_frontier_root ... ok
test frontier::tests::frontier_witness ... ok
test frontier::tests::nonempty_frontier_witness ... ok
test frontier::tests::test_commitment_tree_complete ... ok
test tests::addr_above_position ... ok
test frontier::tests::test_commitment_tree_roundtrip ... ok
test tests::addr_children ... ok
test tests::addr_common_ancestor ... ok
test tests::addr_context ... ok
test tests::addr_is_ancestor ... ok
test tests::addr_is_ancestor_of ... ok
test frontier::tests::test_random_frontier_structure ... ok
test tests::addr_position_range ... ok
test tests::address_current_incomplete ... ok
test tests::address_next_incomplete_parent ... ok
test tests::merkle_path_root ... ok
test tests::position_is_complete_subtree ... ok
test tests::position_past_ommer_count ... ok
test tests::position_root_level ... ok
test tests::position_witness_addrs ... ok
test witness::tests::witness_tip_position ... ok
test frontier::tests::prop_commitment_tree_roundtrip ... ok
test frontier::tests::prop_commitment_tree_roundtrip_str ... ok

test result: ok. 25 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.08s

   Doc-tests incrementalmerkletree

running 1 test
test src/witness.rs - witness::IncrementalWitness (line 18) ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.39s

@nuttycom
Copy link
Contributor Author

utACK e9fe0de. I confirmed that this PR contains the same commit as was merged to the hotfix branch in #125. However, I would have preferred this PR to merge the hotfix branch back into main instead of creating a new merge commit.

The merge commit in this history is a merge from the hotfix branch into (a branch from) main.

@nuttycom nuttycom merged commit b822170 into main Jan 28, 2025
15 checks passed
@nuttycom nuttycom deleted the incrementalmerkletree_no_std branch January 28, 2025 17:02
@str4d
Copy link
Contributor

str4d commented Jan 28, 2025

utACK e9fe0de. I confirmed that this PR contains the same commit as was merged to the hotfix branch in #125. However, I would have preferred this PR to merge the hotfix branch back into main instead of creating a new merge commit.

The merge commit in this history is a merge from the hotfix branch into (a branch from) main.

No it is not. The tip of the hotfix branch is 60e7e18, which is not present in this PR.

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