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

Add/update block height keywords in Clarity #4745

Merged
merged 42 commits into from
May 15, 2024
Merged

Conversation

obycode
Copy link
Contributor

@obycode obycode commented May 3, 2024

Description

Adding support for stacks-block-height and tenure-height in Clarity 3. This also includes using the tenure-height logic for block-height logic in Clarity 1 and 2.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

Also update the docs for `block-height` to explain its removal in
Clarity 3.
In addition to specifying a minimum version, when a builtin is first
available, we can now also specify a maximum version, after which the
builtin is no longer available. This is needed for `block-height`, which
will not be available starting in Clarity 3.
`block-height` can no longer be used in Clarity 3 and above and will
cause an analysis error. In its place is `stacks-block-height`.
Handle the `tenure-height` keyword in Clarity3, and using the same logic
for `block-height` in Clarity 1 or 2.
@obycode obycode marked this pull request as ready for review May 4, 2024 03:39
@obycode obycode requested a review from kantai May 4, 2024 20:06
@obycode
Copy link
Contributor Author

obycode commented May 4, 2024

I'm still checking on a couple of the integration tests, but @kantai, when you have some time, could you take a look at the approach here and let me know if you see anything troublesome. 🙏

@friedger
Copy link
Collaborator

friedger commented May 6, 2024

Somehow related is the get-tenure-info #4716
Is that in scope for Clarity 3?

With this commit, undo the MARF changes following the mechanism used for
`block-height` and replace it with a Clarity based solution.
clarity/src/vm/contexts.rs Outdated Show resolved Hide resolved
@obycode obycode requested a review from kantai May 9, 2024 18:04
@obycode obycode enabled auto-merge May 9, 2024 18:28
kantai
kantai previously approved these changes May 13, 2024
Also including some other improvements from PR review.
This is necessary after the new check was added to ensure that the
tenure height is not set before epoch 3.0.
Ensure new keywords are still handled as expected.
@obycode obycode requested review from a team as code owners May 15, 2024 15:24
@obycode obycode requested review from jcnelson and kantai May 15, 2024 16:36
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

Is there a test document somewhere that we can update so we can remember to put this PR through its paces on the Nakamoto testnet?

@obycode obycode added this pull request to the merge queue May 15, 2024
@obycode
Copy link
Contributor Author

obycode commented May 15, 2024

Is there a test document somewhere that we can update so we can remember to put this PR through its paces on the Nakamoto testnet?

We probably should start a checklist like #4620. cc @saralab

Merged via the queue into develop with commit 83d338d May 15, 2024
1 check passed
@obycode obycode deleted the feat/stacks-block-height branch May 15, 2024 22:30
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.

None yet

5 participants