Skip to content

feat!: include state id in block signature#478

Merged
lklimek merged 32 commits intov0.10-devfrom
sbe-stateid-deprecate
Nov 10, 2022
Merged

feat!: include state id in block signature#478
lklimek merged 32 commits intov0.10-devfrom
sbe-stateid-deprecate

Conversation

@lklimek
Copy link
Collaborator

@lklimek lklimek commented Oct 12, 2022

Issue being fixed or feature implemented

With introduction of Same-Block Execution, maintaining seaparate StateID structure doesn't make sense.
We want to remove it and include into the block hash. It means we will:

  • get rid of redundant information
  • remove separate state id signature

What was done?

  1. Removed state_id_signature.
  2. Added block_id (hash of several fields from block header) to block ID.
  3. Changed how block_signature is calculated
  4. Refactored MaxDataBytes and related logic to reflect the fact that in same-block execution model, we know last commit size upfront.
  5. Removed apphash from commits and votes, as it is indirectly included through block_id.state_id.
  6. Moved VoteBlockSignBytes() to proto.Vote.SignBytes()

How Has This Been Tested?

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@lklimek lklimek changed the title feat: replace state id with HeaderEssentials feat!: replace state id with HeaderEssentials Oct 12, 2022
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

LGTM
I would only suggest following names HeaderLight (or HeaderSimple) for essentials data and HeaderExtended (or HeaderFull) for details

@lklimek lklimek force-pushed the sbe-stateid-deprecate branch from dfee420 to a209aa1 Compare October 21, 2022 09:55
@lklimek lklimek changed the title feat!: replace state id with HeaderEssentials feat!: include state id in block signature Oct 24, 2022
@lklimek lklimek requested a review from shotonoff October 26, 2022 08:09
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

LGTM
please have a look at the comments

@lklimek lklimek requested a review from shotonoff November 9, 2022 15:50
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple more questions

@lklimek lklimek merged commit 1e4faac into v0.10-dev Nov 10, 2022
@lklimek lklimek deleted the sbe-stateid-deprecate branch November 10, 2022 15:38
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