Skip to content

feat(abci)!: put full block in RequestFinalizeBlock #505

Merged
shotonoff merged 7 commits intov0.10-devfrom
td-129-support-commit-on-finalize-block-in-kvstore
Nov 18, 2022
Merged

feat(abci)!: put full block in RequestFinalizeBlock #505
shotonoff merged 7 commits intov0.10-devfrom
td-129-support-commit-on-finalize-block-in-kvstore

Conversation

@shotonoff
Copy link
Collaborator

@shotonoff shotonoff commented Nov 17, 2022

Issue being fixed or feature implemented

Need to add block commit verification to kvstore on finalize block
Add whole bock to RequestFinalizeBlock instead of several fields from its

What was done?

  • Replaced block's fields on a block in RequestFinalizeBlock
  • Added commit verification on demand, by default is enabled for e2e tests
  • Cleaned up the code a bit

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

Copy link
Collaborator

@lklimek lklimek left a comment

Choose a reason for hiding this comment

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

LGTM, some minor things:

  1. Title of PR - maybe sth like: feat(abci)!: put full block in RequestFinalizeBlock - this is the most significant change
  2. Please review linter comments.

@shotonoff shotonoff changed the title feat(kvstore): add block commit verification to kvstore on demand feat(abci)!: put full block in RequestFinalizeBlock Nov 17, 2022
Copy link
Collaborator Author

@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.

applied suggestions and answered on questions

Copy link
Collaborator

@lklimek lklimek left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

Proto looks good to me. Thank you.

@shotonoff shotonoff merged commit 602757c into v0.10-dev Nov 18, 2022
@shotonoff shotonoff deleted the td-129-support-commit-on-finalize-block-in-kvstore branch November 18, 2022 12:07
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.

3 participants