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

Integrate state machine/consensus/generator components - Closes #6567 and #6568 #6624

Merged

Conversation

shuse2
Copy link
Collaborator

@shuse2 shuse2 commented Aug 4, 2021

What was the problem?

This PR resolves #6567 and resolves #6568

Goal of this PR is to setup the environment to start developing all the modules consistently.
Framework integration and functional tests will not work until all the modules are implemented again.

How was it solved?

  • Update lisk-chain exposed functions and remove unused logic and functionalities
  • Remove all duplicated logics from lisk-framework/node
  • Connect the state machine / consensus / generator components in the node

How was it tested?

  • lisk-chain build and tests are updated

@shuse2 shuse2 self-assigned this Aug 4, 2021
- Expose new state store
- Expose new block and block header class
- Remove handling of account and account related types
- Remove state handling (consensus/chain state)
- Remove state validation
- Update transaction property names to follow latest standards
- Update framework module and comman to be in new format
- Update framework not to use BFT library
- Update all tests
- Temporally remove endpoint integration
- Remove account related fixture creation
- Add temporal system modules
@shuse2 shuse2 marked this pull request as ready for review August 6, 2021 09:01
@shuse2 shuse2 removed the request for review from nazarhussain August 6, 2021 09:04
@shuse2 shuse2 force-pushed the 6567-integrate_components branch 7 times, most recently from 5eda013 to b3e1d03 Compare August 6, 2021 11:45
Copy link
Contributor

@ManuGowda ManuGowda left a comment

Choose a reason for hiding this comment

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

Overall changes:

  • Concept of accounts is removed
  • Logic movement from chain to framework for validator, consensus with domain separation
  • Node registration simplification and exposing, endpoints, registry, etc..

Other comments:

  • Not sure why we have files under state_store_v1 which was not meant to be at first place elements/lisk-chain/src/state_store_v1/*
  • generatorPublicKey reference still exists in lots of places
  • There are some changes which needs to be updated in other libraries, i guess they are covered in different issues?

elements/lisk-chain/src/block_header.ts Outdated Show resolved Hide resolved
elements/lisk-chain/src/chain.ts Outdated Show resolved Hide resolved
elements/lisk-chain/src/chain.ts Show resolved Hide resolved
elements/lisk-chain/src/chain.ts Outdated Show resolved Hide resolved
elements/lisk-chain/src/schema.ts Show resolved Hide resolved
framework/src/node/consensus/consensus.ts Outdated Show resolved Hide resolved
framework/src/node/consensus/consensus.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,148 @@
/*
* Copyright © 2019 Lisk Foundation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Copyright © 2019 Lisk Foundation
* Copyright © 2021 Lisk Foundation

Comment on lines +98 to +101
const validators = await this.blockExecutor.getValidators();
const finalizedBlockSlot = this.blockExecutor.getSlotNumber(finalizedBlock.timestamp);
const currentBlockSlot = this.blockExecutor.getSlotNumber(Math.floor(Date.now() / 1000));
const threeRounds = validators.length * 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

some of these values can be made constants?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

those values cannot be constant

- Move liskBFT and validator modules to modules folder
@shuse2 shuse2 requested a review from ManuGowda August 10, 2021 16:27
@shuse2
Copy link
Collaborator Author

shuse2 commented Aug 10, 2021

Not sure why we have files under state_store_v1 which was not meant to be at first place elements/lisk-chain/src/state_store_v1/*

I think it is already deleted? it was just there to not make the breaking change in the previous PR

generatorPublicKey reference still exists in lots of places

There are still lots of module referencing generatorPublicKey, which needs to be updated during the module implementation. Also, genesis block and integration fixtures has this.

There are some changes which needs to be updated in other libraries, i guess they are covered in different issues?

I have created #6632 #6631 etc to address them, but most of the issues need to be handle after the module implementation

Copy link
Contributor

@ManuGowda ManuGowda left a comment

Choose a reason for hiding this comment

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

@shuse2 there are format issues, rest looks good.

@shuse2 shuse2 requested a review from ishantiw August 11, 2021 12:48
Copy link
Contributor

@ishantiw ishantiw left a comment

Choose a reason for hiding this comment

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

Apart from that small comment, all looks good to me

@shuse2 shuse2 merged commit 82af382 into feature/6554-improve-framework-architecture Aug 11, 2021
@shuse2 shuse2 deleted the 6567-integrate_components branch August 11, 2021 13:05
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.

3 participants