Skip to content

refactor: change a logic of usage CoreChainLockHeight#485

Merged
shotonoff merged 10 commits intov0.10-devfrom
refactor-sbe-core-chain-lock-update
Oct 19, 2022
Merged

refactor: change a logic of usage CoreChainLockHeight#485
shotonoff merged 10 commits intov0.10-devfrom
refactor-sbe-core-chain-lock-update

Conversation

@shotonoff
Copy link
Collaborator

@shotonoff shotonoff commented Oct 17, 2022

Issue being fixed or feature implemented

The changes in this PR are requested by Protocol team, which were encountered during integration of same-block execution approach.

What was done?

  • Block time must not change in a block
  • RequestPrepareProposal.CoreChainLockedHeight must use LastCoreChainLockedBlockHeight from the state
  • Add CoreChainLockUpdate to Proposal struct. The value must be consumed from ResponsePrepareProposal.CoreChainLockUpdate
  • Add the validation of Proposal.CoreChainLockUpdate
  • Remove validation of coreChainLock through querying ABCI in validateBlockChainLock function. The verification will be implemented on Drive side.

How Has This Been Tested?

unit and e2e tests

Breaking Changes

N/A

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

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

Looks good to me form logic perspective but we need @lklimek to review it as well.

tendermint.types.CoreChainLock core_chain_lock_update = 100;
// Changes to validator set (set voting power to 0 to remove).
ValidatorSetUpdate validator_set_update = 101 [(gogoproto.nullable) = true];
ValidatorSetUpdate validator_set_update = 100 [(gogoproto.nullable) = true];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please regenerate API docs with make proto-doc

state State,
updateProviders ...UpdateProvider) ([]UpdateFunc, error) {
updates := []UpdateFunc{}
func PrepareStateUpdates(updateProviders ...UpdateProvider) ([]UpdateFunc, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO this approach is obsolete and should be deprecated/removed. We don't need this anymore, as this logic is done by CurrentRoundState.

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.

please have a look at the answers

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

@shotonoff shotonoff merged commit 570e80b into v0.10-dev Oct 19, 2022
@shotonoff shotonoff deleted the refactor-sbe-core-chain-lock-update branch October 19, 2022 14:04
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