Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Update ABI not to pass the consensus information #7677

Merged
merged 12 commits into from
Nov 3, 2022

Conversation

shuse2
Copy link
Collaborator

@shuse2 shuse2 commented Oct 20, 2022

What was the problem?

This PR resolves #7567

How was it solved?

Update engine and application based on LiskArchive/lips#177

  • Update validators module to store consensus information before passing to ABI
  • Move getSlotTime, getSlotNumber and getGeneratorAtTimestamp to BFT method, and update its usages
  • Update DPoS module to use validators module instead of replying directly through context
  • Update block header to add impliesMaxPrevote
  • Update consensus to verify impliesMaxPrevote
  • Update generator to add impliesMaxPrevote
  • Add genesis block validation for impliesMaxPrevote
  • Update dpos module unlock to use aggregateCommit.height for checking certified

How was it tested?

  • Add more test case impliesMaxPrevote method
  • Update the usage and test for the consensus params

Additional Information

Currently genesis timestamp is not stored in BFT module LiskArchive/lips#177 (comment)

@shuse2 shuse2 self-assigned this Oct 20, 2022
@shuse2 shuse2 requested review from has5aan and Incede October 20, 2022 21:22
- Update validators module to store consensus information before passing
  to ABI
- Update DPoS module to use validators module instead of replying
  directly
- Update block header to add impliesMaxPrevotes
- Update consensus to verify impliesMaxPrevotes
- Update generator to add impliesMaxPrevotes
Copy link
Contributor

@Incede Incede left a comment

Choose a reason for hiding this comment

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

Did not review the tests yet but maybe move the bft tests from module to engine

framework/src/engine/bft/method.ts Outdated Show resolved Hide resolved
framework/src/engine/bft/method.ts Outdated Show resolved Hide resolved
framework/src/engine/bft/method.ts Show resolved Hide resolved
framework/src/engine/bft/method.ts Show resolved Hide resolved
framework/src/engine/bft/method.ts Outdated Show resolved Hide resolved
framework/src/engine/bft/utils.ts Show resolved Hide resolved
Copy link
Contributor

@Incede Incede left a comment

Choose a reason for hiding this comment

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

As per the current logic of 'setBFTParameters' the test 'should store BFT parameters with height latest blockBFTInfo + 1' should fail but it's passing, I am not sure why ?

elements/lisk-chain/src/block_header.ts Outdated Show resolved Hide resolved
- Update getValidatorAccount to getValidatorKeys
- Update impliesMaxPrevote to impliesMaxPrevotes
- Remove _computeValidatorsHash and use the the same util function
- Update setValidatorsParam to handle setting the keys
@shuse2 shuse2 requested a review from Incede November 2, 2022 11:52
@shuse2 shuse2 changed the title Update ABI not to pass the consensus information - Closes #7567 Update ABI not to pass the consensus information Nov 2, 2022
@shuse2 shuse2 merged commit 0d7cb39 into development Nov 3, 2022
@shuse2 shuse2 deleted the 7567-update_validator branch November 3, 2022 09:36
Copy link

@janhack janhack left a comment

Choose a reason for hiding this comment

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

The function impliesMaximalPrevotes looks all good.

ishantiw pushed a commit that referenced this pull request Nov 15, 2022
This PR resolves #7567

Update engine and application based on
LiskArchive/lips#177

- Update validators module to store consensus information before passing
to ABI
- Move `getSlotTime`, `getSlotNumber` and `getGeneratorAtTimestamp` to
BFT method, and update its usages
- Update DPoS module to use validators module instead of replying
directly through context
- Update block header to add impliesMaxPrevote
- Update consensus to verify impliesMaxPrevote
- Update generator to add impliesMaxPrevote
- Add genesis block validation for impliesMaxPrevote
- Update dpos module unlock to use aggregateCommit.height for checking
certified

- Add more test case impliesMaxPrevote method
- Update the usage and test for the consensus params

Currently genesis timestamp is not stored in BFT module
LiskArchive/lips#177 (comment)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update validators module to detach engine and application
4 participants