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: add block height to commit-block event #260

Merged
merged 1 commit into from
Apr 20, 2023
Merged

Conversation

obycode
Copy link
Member

@obycode obycode commented Apr 18, 2023

The event generated by a commit-block (on the L1) now includes the
block height for the block that is being committed, in the
block-height key of the event tuple. The target burnchain block height
now uses the key target-burn-block-height. This is useful for
debugging, making it easy to match the L1 transactions with the L2
blocks. Note that this field is not checked in the tests since it is
purely for convenience and could be left out of some implementations.

@obycode obycode requested a review from kantai April 18, 2023 21:06
Copy link
Contributor

@kantai kantai left a comment

Choose a reason for hiding this comment

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

This LGTM, my only feedback is on the variable names in the contract (which would filter out to the rest of the code changes)

@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #260 (5002792) into master (7d373f0) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
+ Coverage   93.47%   93.49%   +0.01%     
==========================================
  Files           6        6              
  Lines         337      338       +1     
==========================================
+ Hits          315      316       +1     
  Misses         22       22              
Impacted Files Coverage Δ
core-contracts/contracts/multi-miner.clar 97.05% <100.00%> (ø)
core-contracts/contracts/subnet.clar 96.52% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

The event generated by a `commit-block` (on the L1) now includes the
block height for the block that is being committed, in the
`block-height` key of the event tuple. The target burnchain block height
now uses the key `target-burn-block-height`. This is useful for
debugging, making it easy to match the L1 transactions with the L2
blocks. Note that this field is not checked in the tests since it is
purely for convenience and could be left out of some implementations.
@obycode
Copy link
Member Author

obycode commented Apr 19, 2023

The last force push fixed the failing test. I'll merge once the tests all pass with this latest version.

@obycode obycode merged commit 3a0ef04 into master Apr 20, 2023
@obycode obycode deleted the add-block-height branch April 20, 2023 01:56
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