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

Include finalization committee and block height in block hash #1104

Merged
merged 15 commits into from
Jan 5, 2024
Merged

Include finalization committee and block height in block hash #1104

merged 15 commits into from
Jan 5, 2024

Conversation

limemloh
Copy link
Contributor

@limemloh limemloh commented Dec 5, 2023 •
edited
Loading

Purpose

Closes #1067

The purpose of this PR is to make the block hash to be computed from more information, specifically:

  • Weight and BLS verify key of the current and next finalization committee.
  • Absolute block height
  • Relative block height
  • Genesis index.

This PR should be merged after Concordium/concordium-base#490.

Changes

Starting from protocol version 7:

  • The baked block no longer contains the hash of the transaction outcomes and the hash of the block state, instead these are combined into one hash (BlockResultHash) which is also computed from the new information listed above.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@limemloh limemloh force-pushed the block-hashing-pv-height branch 2 times, most recently from deed100 to 7b79cf8 Compare December 6, 2023 06:40
@limemloh limemloh force-pushed the block-hashing-pv-height branch from 7b79cf8 to 632100b Compare December 6, 2023 07:28
@limemloh limemloh mentioned this pull request Dec 6, 2023
Merged
5 tasks
@limemloh limemloh marked this pull request as ready for review December 6, 2023 07:42
abizjak
abizjak reviewed Dec 8, 2023
View reviewed changes
concordium-consensus/tests/consensus/ConcordiumTests/KonsensusV1/TreeStateTest.hs Outdated Show resolved Hide resolved
concordium-consensus/tests/consensus/ConcordiumTests/KonsensusV1/TreeStateTest.hs Outdated Show resolved Hide resolved
concordium-consensus/tests/consensus/ConcordiumTests/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/tests/consensus/ConcordiumTests/KonsensusV1/Consensus/Blocks.hs Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Show resolved Hide resolved
td202
td202 reviewed Dec 13, 2023
View reviewed changes
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/TreeState/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Outdated Show resolved Hide resolved
MilkywayPirate
MilkywayPirate approved these changes Dec 22, 2023
View reviewed changes
Copy link
Contributor

@MilkywayPirate MilkywayPirate left a comment

Choose a reason for hiding this comment

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

Great job!

I presume that the things mentioned will be taken care of by #1111 and future PRs.

concordium-consensus/src/Concordium/KonsensusV1/Types.hs Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Types.hs Show resolved Hide resolved
@limemloh
Merge pull request #1111 from Concordium/block-hashing-tests
5a7fff5 
Convert P6 tests to also run for P7 and onwards
td202
td202 approved these changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@td202 td202 left a comment

Choose a reason for hiding this comment

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

Looks good. Just computeBlockResultHash is a concern for me.

concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
concordium-consensus/src/Concordium/KonsensusV1/Consensus/Blocks.hs Outdated Show resolved Hide resolved
@limemloh limemloh merged commit c7a7f37 into main Jan 5, 2024
3 checks passed
@limemloh limemloh deleted the block-hashing-pv-height branch January 5, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abizjak abizjak abizjak approved these changes

@td202 td202 td202 approved these changes

@MilkywayPirate MilkywayPirate MilkywayPirate approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Block hash: Include missing information needed for light client
4 participants
@limemloh @abizjak @td202 @MilkywayPirate