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

Create CCU - Closes #7224 #7438

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Create CCU - Closes #7224 #7438

merged 1 commit into from
Oct 6, 2022

Conversation

has5aan
Copy link
Contributor

@has5aan has5aan commented Aug 22, 2022 •
edited
Loading

What was the problem?

This PR resolves #7224

How was it solved?

Implementation of _createCCUParams, _calculateInboxUpdate, _rawStateStoreKey, _validateCertificate and _verifyLiveness functions.

How was it tested?

Implemented unit tests.

@has5aan has5aan self-assigned this Aug 22, 2022
@has5aan has5aan requested a review from ishantiw August 22, 2022 14:28
@milenagojkovic milenagojkovic linked an issue Aug 23, 2022 that may be closed by this pull request
Implement createCCU function for chain connector plugin #7224
Closed
@ishantiw ishantiw force-pushed the feature/6918-chain-connector branch from 1dbd883 to 88dfc1d Compare August 23, 2022 08:57
@ishantiw ishantiw force-pushed the feature/6918-chain-connector branch from 88dfc1d to 8f6e0d3 Compare August 23, 2022 09:37
ishantiw
ishantiw reviewed Aug 29, 2022
View reviewed changes
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.

I think overall logic looks good to me, some general comments

framework-plugins/lisk-framework-chain-connector-plugin/package.json Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/test/unit/plugin.spec.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
@ishantiw ishantiw mentioned this pull request Aug 29, 2022
Closed
@ricott1 ricott1 self-requested a review August 29, 2022 14:16
ricott1
ricott1 suggested changes Aug 29, 2022
View reviewed changes
Copy link
Contributor

@ricott1 ricott1 left a comment

Choose a reason for hiding this comment

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

I reviewed only _calculateInboxUpdate for the moment.

framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
@ishantiw ishantiw mentioned this pull request Aug 30, 2022
Closed
@has5aan has5aan marked this pull request as ready for review August 31, 2022 07:39
@ishantiw ishantiw mentioned this pull request Sep 1, 2022
Closed
@has5aan has5aan changed the title Create CCU - Closes 7224 Create CCU - Closes #7224 Sep 2, 2022
ishantiw
ishantiw reviewed Sep 22, 2022
View reviewed changes
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.

I think overall implementation looks good to me, there is a comment and also there is a recent change:

  1. CCU Params schema change => newCertificateThreshold->certificateThreshold
  2. networkIdentifier is removed and chainID is used instead

framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
shuse2
shuse2 suggested changes Sep 26, 2022
View reviewed changes
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
framework/src/index.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/test/unit/utils.spec.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/test/unit/plugin.spec.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Show resolved Hide resolved
ishantiw
ishantiw suggested changes Oct 3, 2022
View reviewed changes
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.

Some comments but still reviewing the logic and will add more comments if needed

elements/lisk-tree/src/index.ts Outdated Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/utils.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/src/utils.ts Show resolved Hide resolved
framework-plugins/lisk-framework-chain-connector-plugin/test/unit/plugin.spec.ts Outdated Show resolved Hide resolved
@ishantiw ishantiw merged commit 9813fb6 into feature/6918-chain-connector Oct 6, 2022
@ishantiw ishantiw deleted the 7224-CreateCCU branch October 6, 2022 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ricott1 ricott1 ricott1 requested changes

@shuse2 shuse2 shuse2 approved these changes

@ishantiw ishantiw ishantiw approved these changes

Assignees

@has5aan has5aan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement createCCU function for chain connector plugin
4 participants
@has5aan @shuse2 @ishantiw @ricott1