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

Conversation

sitetester
Copy link
Contributor

@sitetester sitetester commented Mar 7, 2023

What was the problem?

This PR resolves #8151 (ONLY mainchain)
Sidechain PR: #8256

How was it solved?

Verification checks applied per LIP
Relevant PR: #8239

Note:

  • To reduce size of PR, this task is split into multiple PRs
  • This PR only covers mainchain part

How was it tested?

  • Test cases added to module.spec.ts
  • To check/verify schema at example path:
    • cd lisk-sdk/examples/pos-mainchain
    • ./bin/run genesis-block:create --output=config/default --assets-file=./config/default/genesis_assets.json
  • To run example app
    • ./bin/run start --api-ipc --api-http --port 9001
    • if it fails with Error: Genesis block does not match., run rm -rf ~/.lisk/pos-mainchain (Note:, it's not rm -rf ~/.lisk/dpos-mainchain)

@sitetester sitetester changed the title Update genesis asset schema interoperability without terminatedStateAccounts and terminatedOutboxAccounts Update genesis asset schema interoperability WITHOUT terminatedStateAccounts and terminatedOutboxAccounts Mar 7, 2023
@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Merging #8238 (85c26f5) into 8151-update-genesis-asset-schema-interoperability (de0b926) will decrease coverage by 0.01%.
The diff coverage is 97.93%.

❗ Current head 85c26f5 differs from pull request most recent head 958e791. Consider uploading reports for the commit 958e791 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@                                  Coverage Diff                                  @@
##           8151-update-genesis-asset-schema-interoperability    #8238      +/-   ##
=====================================================================================
- Coverage                                              83.06%   83.06%   -0.01%     
=====================================================================================
  Files                                                    588      588              
  Lines                                                  21747    21679      -68     
  Branches                                                3170     3150      -20     
=====================================================================================
- Hits                                                   18065    18008      -57     
+ Misses                                                  3682     3671      -11     
Impacted Files Coverage Δ
...s/interoperability/base_interoperability_module.ts 96.55% <75.00%> (+4.24%) ⬆️
...k/src/modules/interoperability/mainchain/module.ts 95.38% <98.78%> (+5.58%) ⬆️
framework/src/modules/interoperability/schemas.ts 100.00% <100.00%> (ø)
...c/modules/interoperability/stores/chain_account.ts 100.00% <100.00%> (ø)
framework/src/modules/interoperability/utils.ts 91.71% <100.00%> (+0.04%) ⬆️
framework/src/testing/create_contexts.ts 93.81% <100.00%> (-0.37%) ⬇️

@sitetester sitetester marked this pull request as ready for review March 7, 2023 18:28
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.

Overall looks good, couple of minor comments but approving

Copy link

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Just caught few inconsistencies, looks good otherwise.

@Phanco Phanco requested a review from gkoumout March 21, 2023 13:58
Copy link

@gkoumout gkoumout left a comment

Choose a reason for hiding this comment

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

Good job Franco! Fine, for me, just left one minor comment for you.

@@ -152,20 +107,19 @@
"type": "array",
"items": {
"dataType": "bytes",
"minLength": 32,
"maxLength": 32
"length": 32
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well, we need to use "minLength", "maxLength"

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

…ithub.com:LiskHQ/lisk-sdk into 8151-update-without-terminatedStateAccounts-and-terminatedOutboxAccounts
@ishantiw ishantiw merged commit e9eb5c3 into 8151-update-genesis-asset-schema-interoperability Mar 24, 2023
@ishantiw ishantiw deleted the 8151-update-without-terminatedStateAccounts-and-terminatedOutboxAccounts branch March 24, 2023 09:41
ishantiw added a commit that referenced this pull request Mar 24, 2023
…unts and terminatedOutboxAccounts (#8239)

### What was the problem?
This PR resolves #8151 (ONLY mainchain)

Relevant PR: #8238

### How was it solved?
Verification checks applied per
[LIP](https://github.com/LiskHQ/lips/blob/main/proposals/lip-0045.md#mainchain)


### How was it tested?

Test cases added to `module.spec.ts`
@sitetester sitetester restored the 8151-update-without-terminatedStateAccounts-and-terminatedOutboxAccounts branch March 25, 2023 11:57
@sitetester sitetester deleted the 8151-update-without-terminatedStateAccounts-and-terminatedOutboxAccounts branch March 27, 2023 06:59
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.

4 participants