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

Change interoperability genesis asset schema #256

Merged

janhack merged 13 commits into main from fix-interop-genesis-assets

Feb 17, 2023

Contributor

sergeyshemyakov commented Feb 10, 2023

Closes #248.


          Changed genesis asset schema, verification and genesis state initiali…

97deedb

…zation

sergeyshemyakov added the lip modification label

sergeyshemyakov requested review from ricott1 and AndreasKendziorra

February 10, 2023 16:42

sergeyshemyakov self-assigned this

sergeyshemyakov changed the title ~~Change genesis asset schema~~ Change interoperability genesis asset schema

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 reviewed

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 suggested changes

View reviewed changes

Contributor

ricott1 left a comment

I left several comments. My main ones are about precisely defining 'empty' substores.

ricott1 assigned ricott1 and unassigned sergeyshemyakov

ricott1 requested a review from gkoumout

February 14, 2023 09:08

Contributor

ricott1 commented Feb 14, 2023 •

edited

Loading

Currently, we allow for direct channels in the genesis block, but maybe we shouldn't as this is not possible to do with the current protocol. If we don't, the protocol would become drastically simpler.

ricott1 requested a review from ishantiw

February 14, 2023 10:06

ricott1 added 3 commits

February 14, 2023 11:29


          Fix typo; rename ownChainInfo to ownChainAccount; add lexicographical…

c842816

… order of chainInfos entries


          remove todo comments

74d523d


          Rewrite genesis verification

9f57608

ricott1 requested review from ricott1 and removed request for ricott1

February 14, 2023 13:02

ricott1 approved these changes

View reviewed changes

ricott1 requested review from ricott1 and removed request for ricott1

February 14, 2023 13:03

ricott1 added 3 commits

February 14, 2023 14:48


          minor extra check

2355a72


          fix initialization own chain account

82dbffd


          Add check for escrow account

982986a

ishantiw mentioned this pull request

Update genesis asset schema for interoperability LiskArchive/lisk-sdk#8151

Closed

ishantiw approved these changes

View reviewed changes

Contributor

ishantiw left a comment

Created related issue on SDK LiskArchive/lisk-sdk#8151

AndreasKendziorra suggested changes

View reviewed changes

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

ricott1 and others added 2 commits

February 16, 2023 15:17


          Apply suggestions from code review

752c0b3

Co-authored-by: AndreasKendziorra <[email protected]>


          Address Anderas' review

38dacff

ricott1 requested a review from AndreasKendziorra

February 17, 2023 07:50

AndreasKendziorra suggested changes

View reviewed changes

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved

proposals/lip-0045.md Outdated Show resolved Hide resolved


          Apply suggestions from code review

75c36c6

Co-authored-by: AndreasKendziorra <[email protected]>

AndreasKendziorra approved these changes

View reviewed changes

gkoumout approved these changes

View reviewed changes

Contributor

gkoumout left a comment

Looks good. I left some minor comments/recommendations, but did not find something critical. It's ok to me to be merged with the current version.

proposals/lip-0045.md

               | `MIN_CHAIN_NAME_LENGTH` | uint32 | 1 | The minimum length of a string specifying the name of a chain. |
               | `MAX_CHAIN_NAME_LENGTH` | uint32 | 32 | The maximum length of a string specifying the name of a chain. |
-              We further use the utility function `getMainchainID()` defined in [LIP 0037][lip-0037#getMainchainID] to obtain the chain ID of the mainchain.
+              We further use the utility function `getMainchainID` defined in [LIP 0037][lip-0037#getMainchainID] to obtain the chain ID of the mainchain.
               #### Empty Cross-chain Message

Contributor

gkoumout Feb 17, 2023

I just checked this by accident. I think description of empty ccm in the table should be improved since it does not fully follow the ccmSchema.

Contributor

ricott1 Feb 17, 2023

edited

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved

proposals/lip-0045.md Show resolved Hide resolved


          address Greg's review

ddf301b

ricott1 requested a review from janhack

February 17, 2023 17:03

janhack added 2 commits

February 17, 2023 18:38


          Merge branch 'main' into fix-interop-genesis-assets

2bfebec


          Editorial edits

3161ad6

janhack approved these changes

View reviewed changes

Contributor

janhack left a comment

Editorial approval

janhack merged commit bf6143b into main

janhack deleted the fix-interop-genesis-assets branch

February 17, 2023 17:41

sergeyshemyakov mentioned this pull request

Update Interoperability genesis asset processing in LIP 0063 #269

Closed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

ricott1 ricott1 approved these changes

ishantiw ishantiw approved these changes

janhack janhack approved these changes

AndreasKendziorra AndreasKendziorra approved these changes

gkoumout gkoumout approved these changes

Labels

lip modification