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(Clarity2) Get burnchain_header_hash by burnchain_height #2770

Merged
merged 90 commits into from
Apr 29, 2022
Merged

Conversation

gregorycoppola
Copy link
Contributor

@gregorycoppola gregorycoppola commented Jul 15, 2021

Fixes #2663

Description

This change allows a clarity contract to get the header hash of a burnchain block using the burchain height. Currently, you can get the header hash of the burnchain only using the Stacks height.

Testing

Tests have been added in the following files:

M	src/chainstate/stacks/boot/contract_tests.rs
M	src/vm/analysis/arithmetic_checker/tests.rs
M	src/vm/analysis/type_checker/tests/mod.rs
M	src/vm/tests/contracts.rs
M	src/vm/tests/costs.rs

A TODO was added to modify the following integration test. We avoid doing this now because it will require having a way to configure the Stacks Epoch for the Mocknet miner.

M	testnet/stacks-node/src/tests/integrations.rs

@project-bot project-bot bot added this to Review in progress in Stacks Blockchain Board Jul 15, 2021
@gregorycoppola gregorycoppola changed the base branch from master to next July 15, 2021 21:03
Copy link
Contributor Author

@gregorycoppola gregorycoppola left a comment

Choose a reason for hiding this comment

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

added some pointers to implementations

src/vm/database/clarity_db.rs Outdated Show resolved Hide resolved
src/clarity_vm/database/mod.rs Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #2770 (34ab863) into next (2747901) will increase coverage by 0.05%.
The diff coverage is 53.14%.

@@            Coverage Diff             @@
##             next    #2770      +/-   ##
==========================================
+ Coverage   83.75%   83.80%   +0.05%     
==========================================
  Files         265      266       +1     
  Lines      207954   208569     +615     
==========================================
+ Hits       174166   174793     +627     
+ Misses      33788    33776      -12     
Impacted Files Coverage Δ
clarity/src/vm/analysis/arithmetic_checker/mod.rs 92.64% <ø> (ø)
clarity/src/vm/analysis/read_only_checker/mod.rs 86.44% <ø> (ø)
clarity/src/vm/database/key_value_wrapper.rs 96.83% <ø> (+0.31%) ⬆️
clarity/src/vm/test_util/mod.rs 61.71% <0.00%> (-5.81%) ⬇️
clarity/src/vm/tests/contracts.rs 98.04% <ø> (ø)
clarity/src/vm/tests/mod.rs 100.00% <ø> (ø)
src/chainstate/burn/db/sortdb.rs 95.21% <ø> (-0.02%) ⬇️
src/chainstate/burn/mod.rs 97.54% <ø> (ø)
src/chainstate/burn/sortition.rs 89.86% <0.00%> (-5.97%) ⬇️
src/chainstate/stacks/index/marf.rs 83.93% <ø> (ø)
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9407d62...34ab863. Read the comment docs.

@jcnelson
Copy link
Member

I've merged #3110 into this PR, since I need some of its test infrastructure. Please merge that one first.

@jcnelson jcnelson requested review from lgalabru and obycode and removed request for reedrosenbluth April 27, 2022 17:56
@jcnelson
Copy link
Member

This is now ready for review @kantai @gregorycoppola @lgalabru @obycode

clarity/src/vm/analysis/type_checker/natives/mod.rs Outdated Show resolved Hide resolved
clarity/src/vm/analysis/type_checker/natives/mod.rs Outdated Show resolved Hide resolved
clarity/src/vm/analysis/type_checker/tests/mod.rs Outdated Show resolved Hide resolved
clarity/src/vm/database/clarity_db.rs Outdated Show resolved Hide resolved
clarity/src/vm/docs/mod.rs Outdated Show resolved Hide resolved
clarity/src/vm/functions/database.rs Show resolved Hide resolved
Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

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

LGTM!

Stacks Blockchain Board automation moved this from Review in progress to Reviewer approved Apr 28, 2022
@jcnelson jcnelson merged commit cc07c63 into next Apr 29, 2022
Stacks Blockchain Board automation moved this from Reviewer approved to Done Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Extension: report any burnchain block hash
7 participants