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.

Sign up for GitHub

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

Jump to bottom

[VALIDATED_STATE] - Make Committable a supertrait of BlockHeader and merge changes in develop #2169

Merged
merged 4 commits into from
Dec 6, 2023
Merged

[VALIDATED_STATE] - Make Committable a supertrait of BlockHeader and merge changes in develop #2169

merged 4 commits into from
Dec 6, 2023

Conversation

shenkeyao
Copy link
Member

@shenkeyao shenkeyao commented Dec 6, 2023 •
edited
Loading

Closes #2167. Closes #2169.

This PR:

  • Makes Committable a supertrait of BlockHeader.
    • Adds :Committable to BlockHeader.
    • Implements Committable for TestBlockHeader.
    • Updates commitment tags.
  • Merges develop into `main.

dependabot bot and others added 3 commits December 5, 2023 07:40
@dependabot
Bump clap from 4.4.10 to 4.4.11 (#2162)
e821332 
Bumps [clap](https://github.com/clap-rs/clap) from 4.4.10 to 4.4.11.
- [Release notes](https://github.com/clap-rs/clap/releases)
- [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md)
- [Commits](clap-rs/clap@v4.4.10...v4.4.11)

---
updated-dependencies:
- dependency-name: clap
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@QuentinI
#2059: Organize non-prod code (#2134)
119b760 
* Move example/test implementations to hotshot_testing

Closes #2059

* Warn when using demo impls in production builds

* Refactor demo/test types

Collapsed three virtually identical `NodeType` implementations into one,
split code from `testing::demo` into more descriptive modules and
renamed various test types to conform to a single naming scheme.

* Remove 'demo' features

Those weren't actually gating anything anymore
@shenkeyao
Make Committable a subtrait of BlockHeader
095d6ed
@shenkeyao shenkeyao requested a review from jbearer December 6, 2023 01:05
@shenkeyao shenkeyao marked this pull request as ready for review December 6, 2023 01:05
@shenkeyao shenkeyao changed the title Make Committable a supertrait of BlockHeader [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader Dec 6, 2023
@shenkeyao shenkeyao changed the title [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader Dec 6, 2023
jbearer
jbearer previously approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@jbearer jbearer left a comment

Choose a reason for hiding this comment

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

LGTM

@jbearer
Copy link
Member

jbearer commented Dec 6, 2023

Oh can we merge this into main? Or merge develop into main right after merging this into develop? This is a blocker for the downstream integration

@shenkeyao
Copy link
Member Author

Oh can we merge this into main? Or merge develop into main right after merging this into develop? This is a blocker for the downstream integration

Sure I'll do the former! I think in general the latter is the better workflow, but since this is simply adding a super-trait it shouldn't cause any conflict between develop and main.

@shenkeyao shenkeyao changed the base branch from develop to main December 6, 2023 17:43
@shenkeyao shenkeyao dismissed jbearer’s stale review December 6, 2023 17:43

The base branch was changed.

jbearer
jbearer previously approved these changes Dec 6, 2023
View reviewed changes
Copy link
Member

@jbearer jbearer left a comment

Choose a reason for hiding this comment

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

This also brings in Artemii's reorganization of test code. That's a good thing IMO, just making sure that was intended

@shenkeyao
Copy link
Member Author

shenkeyao commented Dec 6, 2023 •
edited
Loading

This also brings in Artemii's reorganization of test code. That's a good thing IMO, just making sure that was intended

@jbearer Good catch! Since this code was originally based on develop, merging it into main is essentially merging develop into main. Let me double-check with the team--we discussed last week about getting rid of develop.

@jbearer
Copy link
Member

jbearer commented Dec 6, 2023

If this is all that's on develop right now, merging it into main should be fine

@shenkeyao shenkeyao changed the title [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader and merge changes in develop Dec 6, 2023
@shenkeyao shenkeyao changed the title [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader and merge changes in develop [VALIDATED_STATE] - Make Committable a supertrait of BlockHeader and merge changes in develop Dec 6, 2023
@shenkeyao shenkeyao merged commit f1eec4f into main Dec 6, 2023
7 of 8 checks passed
@shenkeyao shenkeyao deleted the keyao/committable-subtrait branch December 6, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rob-maron rob-maron rob-maron approved these changes

@jbearer jbearer jbearer approved these changes

@elliedavidson elliedavidson Awaiting requested review from elliedavidson

@bfish713 bfish713 Awaiting requested review from bfish713 bfish713 is a code owner

Assignees
No one assigned
Labels
validated-state
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[VALIDATED_STATE] - Make Committable a super-trait of BlockHeader
4 participants
@shenkeyao @jbearer @rob-maron @QuentinI