From e3fb6e85bd55f28552ee813b4f34197f0346deb1 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Tue, 10 Oct 2023 16:56:02 +0300 Subject: [PATCH 1/7] _verifyTerminatedStateAccountsCommon doesn't need to be inside loop --- .../interoperability/mainchain/module.ts | 62 +++++++++++-------- .../interoperability/mainchain/module.spec.ts | 37 +++++++++++ 2 files changed, 74 insertions(+), 25 deletions(-) diff --git a/framework/src/modules/interoperability/mainchain/module.ts b/framework/src/modules/interoperability/mainchain/module.ts index 8dbf6e9f613..289f061df9f 100644 --- a/framework/src/modules/interoperability/mainchain/module.ts +++ b/framework/src/modules/interoperability/mainchain/module.ts @@ -367,32 +367,44 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule 'For each chainInfo with status terminated there should be a corresponding entry in terminatedStateAccounts.', ); } + } + } - this._verifyTerminatedStateAccountsCommon(terminatedStateAccounts, mainchainID); - - // For each entry stateAccount in terminatedStateAccounts holds - // stateAccount.stateRoot == chainData.lastCertificate.stateRoot, - // stateAccount.mainchainStateRoot == EMPTY_HASH, and - // stateAccount.initialized == True. - // Here chainData is the corresponding entry (i.e., with chainID == stateAccount.chainID) in chainInfos. - const stateAccount = terminatedAccount.terminatedStateAccount; - if (stateAccount) { - if (!stateAccount.stateRoot.equals(chainInfo.chainData.lastCertificate.stateRoot)) { - throw new Error( - "stateAccount.stateRoot doesn't match chainInfo.chainData.lastCertificate.stateRoot.", - ); - } - - if (!stateAccount.mainchainStateRoot.equals(EMPTY_HASH)) { - throw new Error( - `stateAccount.mainchainStateRoot is not equal to ${EMPTY_HASH.toString('hex')}.`, - ); - } - - if (!stateAccount.initialized) { - throw new Error('stateAccount is not initialized.'); - } - } + this._verifyTerminatedStateAccountsCommon(terminatedStateAccounts, mainchainID); + + // For each entry stateAccount in terminatedStateAccounts holds + // stateAccount.stateRoot == chainData.lastCertificate.stateRoot, + // stateAccount.mainchainStateRoot == EMPTY_HASH, and + // stateAccount.initialized == True. + // Here chainData is the corresponding entry (i.e., with chainID == stateAccount.chainID) in chainInfos. + + for (const terminatedStateAccountWithChainID of terminatedStateAccounts) { + const stateAccount = terminatedStateAccountWithChainID.terminatedStateAccount; + // For each entry stateAccount in terminatedStateAccounts holds + // stateAccount.stateRoot == chainData.lastCertificate.stateRoot, + // stateAccount.mainchainStateRoot == EMPTY_HASH, and + // stateAccount.initialized == True. + // Here chainData is the corresponding entry (i.e., with chainID == stateAccount.chainID) in chainInfos. + const correspondingChainInfo = chainInfos.find(chainInfo => + chainInfo.chainID.equals(terminatedStateAccountWithChainID.chainID), + ) as ChainInfo; // at this point, it's not undefined, since similar check already applied above + + if ( + !stateAccount.stateRoot.equals(correspondingChainInfo.chainData.lastCertificate.stateRoot) + ) { + throw new Error( + "stateAccount.stateRoot doesn't match chainInfo.chainData.lastCertificate.stateRoot.", + ); + } + + if (!stateAccount.mainchainStateRoot.equals(EMPTY_HASH)) { + throw new Error( + `stateAccount.mainchainStateRoot is not equal to ${EMPTY_HASH.toString('hex')}.`, + ); + } + + if (!stateAccount.initialized) { + throw new Error('stateAccount is not initialized.'); } } } diff --git a/framework/test/unit/modules/interoperability/mainchain/module.spec.ts b/framework/test/unit/modules/interoperability/mainchain/module.spec.ts index b62c2eaef71..4a7561c6b40 100644 --- a/framework/test/unit/modules/interoperability/mainchain/module.spec.ts +++ b/framework/test/unit/modules/interoperability/mainchain/module.spec.ts @@ -473,6 +473,43 @@ describe('initGenesisState', () => { ); }); + it('should call _verifyTerminatedStateAccountsCommon', async () => { + jest.spyOn(interopMod, '_verifyTerminatedStateAccountsCommon' as any); + + const context = createInitGenesisStateContext( + { + ...genesisInteroperability, + chainInfos: [ + { + ...chainInfo, + chainData: { + ...chainData, + status: ChainStatus.TERMINATED, + lastCertificate: { + ...lastCertificate, + validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), + }, + }, + chainValidators: { + activeValidators, + certificateThreshold, + }, + }, + ], + terminatedStateAccounts: [ + { + chainID: chainInfo.chainID, + terminatedStateAccount, + }, + ], + }, + params, + ); + + await expect(interopMod.initGenesisState(context)).resolves.toBeUndefined(); + expect(interopMod['_verifyTerminatedStateAccountsCommon']).toHaveBeenCalledTimes(1); + }); + it('should throw error if some stateAccount in terminatedStateAccounts have stateRoot not equal to chainData.lastCertificate.stateRoot', async () => { const context = createInitGenesisStateContext( { From 97e6fee4abb8d1fab4feb907cad0579af98ba4d5 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Wed, 11 Oct 2023 19:13:15 +0300 Subject: [PATCH 2/7] Apply review comments --- .../modules/interoperability/base_interoperability_module.ts | 2 +- framework/src/modules/interoperability/sidechain/module.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/src/modules/interoperability/base_interoperability_module.ts b/framework/src/modules/interoperability/base_interoperability_module.ts index 01f784c9740..6ad892f46f7 100644 --- a/framework/src/modules/interoperability/base_interoperability_module.ts +++ b/framework/src/modules/interoperability/base_interoperability_module.ts @@ -147,7 +147,7 @@ export abstract class BaseInteroperabilityModule extends BaseInteroperableModule } // for each validator in activeValidators, validator.bftWeight > 0 must hold - if (activeValidators.filter(v => v.bftWeight <= 0).length > 0) { + if (activeValidators.filter(v => v.bftWeight <= BigInt(0)).length > 0) { throw new Error(`validator.bftWeight must be > 0.`); } diff --git a/framework/src/modules/interoperability/sidechain/module.ts b/framework/src/modules/interoperability/sidechain/module.ts index eb528d789f8..2d7780e6fc8 100644 --- a/framework/src/modules/interoperability/sidechain/module.ts +++ b/framework/src/modules/interoperability/sidechain/module.ts @@ -290,7 +290,7 @@ export class SidechainInteroperabilityModule extends BaseInteroperabilityModule } // mainchainInfo.chainID == getMainchainID(); const mainchainInfo = chainInfos[0]; - const mainchainID = getMainchainID(mainchainInfo.chainID); + const mainchainID = getMainchainID(ctx.chainID); if (!mainchainInfo.chainID.equals(mainchainID)) { throw new Error(`mainchainInfo.chainID must be equal to ${mainchainID.toString('hex')}.`); } From 24cdc25d44b35c9c406079c7c8d795bb571bd5e6 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Thu, 12 Oct 2023 18:47:08 +0300 Subject: [PATCH 3/7] Use util functions & apply performance steps to avoid extra loops --- .../base_interoperability_module.ts | 37 +++------ .../interoperability/mainchain/module.ts | 81 ++++++++----------- .../interoperability/sidechain/module.ts | 5 +- .../base_interoperability_module.spec.ts | 2 +- .../interoperability/mainchain/module.spec.ts | 57 +++++++------ 5 files changed, 77 insertions(+), 105 deletions(-) diff --git a/framework/src/modules/interoperability/base_interoperability_module.ts b/framework/src/modules/interoperability/base_interoperability_module.ts index 6ad892f46f7..69c5e7a55f3 100644 --- a/framework/src/modules/interoperability/base_interoperability_module.ts +++ b/framework/src/modules/interoperability/base_interoperability_module.ts @@ -35,12 +35,7 @@ import { OwnChainAccountStore } from './stores/own_chain_account'; import { RegisteredNamesStore } from './stores/registered_names'; import { TerminatedOutboxStore } from './stores/terminated_outbox'; import { TerminatedStateStore } from './stores/terminated_state'; -import { - ChainInfo, - GenesisInteroperability, - OwnChainAccount, - TerminatedStateAccountWithChainID, -} from './types'; +import { ChainInfo, GenesisInteroperability, OwnChainAccount } from './types'; import { computeValidatorsHash, getTokenIDLSK } from './utils'; import { genesisInteroperabilitySchema } from './schemas'; import { CcmProcessedEvent } from './events/ccm_processed'; @@ -133,15 +128,12 @@ export abstract class BaseInteroperabilityModule extends BaseInteroperableModule } // activeValidators must be ordered lexicographically by blsKey property - const sortedByBlsKeys = [...activeValidators].sort((a, b) => a.blsKey.compare(b.blsKey)); - for (let i = 0; i < activeValidators.length; i += 1) { - if (!activeValidators[i].blsKey.equals(sortedByBlsKeys[i].blsKey)) { - throw new Error('activeValidators must be ordered lexicographically by blsKey property.'); - } + const blsKeys = activeValidators.map(v => v.blsKey); + if (!objectUtils.isBufferArrayOrdered(blsKeys)) { + throw new Error('activeValidators must be ordered lexicographically by blsKey property.'); } // all blsKey properties must be pairwise distinct - const blsKeys = activeValidators.map(v => v.blsKey); if (!objectUtils.bufferArrayUniqueItems(blsKeys)) { throw new Error(`All blsKey properties must be pairwise distinct.`); } @@ -189,28 +181,19 @@ export abstract class BaseInteroperabilityModule extends BaseInteroperableModule } } - protected _verifyTerminatedStateAccountsCommon( - terminatedStateAccounts: TerminatedStateAccountWithChainID[], - mainchainID: Buffer, - ) { + protected _verifyTerminatedStateAccountsIDs(chainIDs: Buffer[], mainchainID: Buffer) { // Each entry stateAccount in terminatedStateAccounts has a unique stateAccount.chainID - const chainIDs = terminatedStateAccounts.map(a => a.chainID); if (!objectUtils.bufferArrayUniqueItems(chainIDs)) { throw new Error(`terminatedStateAccounts don't hold unique chainID.`); } // terminatedStateAccounts is ordered lexicographically by stateAccount.chainID - const sortedByChainID = [...terminatedStateAccounts].sort((a, b) => - a.chainID.compare(b.chainID), - ); - - for (let i = 0; i < terminatedStateAccounts.length; i += 1) { - const stateAccountWithChainID = terminatedStateAccounts[i]; - if (!stateAccountWithChainID.chainID.equals(sortedByChainID[i].chainID)) { - throw new Error('terminatedStateAccounts must be ordered lexicographically by chainID.'); - } + if (!objectUtils.isBufferArrayOrdered(chainIDs)) { + throw new Error('terminatedStateAccounts must be ordered lexicographically by chainID.'); + } - this._verifyChainID(stateAccountWithChainID.chainID, mainchainID, 'stateAccount.'); + for (const chainID of chainIDs) { + this._verifyChainID(chainID, mainchainID, 'stateAccount.'); } } diff --git a/framework/src/modules/interoperability/mainchain/module.ts b/framework/src/modules/interoperability/mainchain/module.ts index 289f061df9f..1d60ddf5eab 100644 --- a/framework/src/modules/interoperability/mainchain/module.ts +++ b/framework/src/modules/interoperability/mainchain/module.ts @@ -269,7 +269,7 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule throw new Error(`ownChainNonce must be positive if chainInfos is not empty.`); } - this._verifyChainInfos(ctx, chainInfos); + this._verifyChainInfos(ctx, chainInfos, terminatedStateAccounts); this._verifyTerminatedStateAccounts(chainInfos, terminatedStateAccounts, mainchainID); this._verifyTerminatedOutboxAccounts( chainInfos, @@ -281,7 +281,11 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule } // https://github.com/LiskHQ/lips/blob/main/proposals/lip-0045.md#mainchain - private _verifyChainInfos(ctx: GenesisBlockExecuteContext, chainInfos: ChainInfo[]) { + private _verifyChainInfos( + ctx: GenesisBlockExecuteContext, + chainInfos: ChainInfo[], + terminatedStateAccounts: TerminatedStateAccountWithChainID[], + ) { // Each entry chainInfo in chainInfos has a unique chainInfo.chainID const chainIDs = chainInfos.map(info => info.chainID); if (!objectUtils.bufferArrayUniqueItems(chainIDs)) { @@ -289,11 +293,8 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule } // chainInfos should be ordered lexicographically by chainInfo.chainID - const sortedByChainID = [...chainInfos].sort((a, b) => a.chainID.compare(b.chainID)); - for (let i = 0; i < chainInfos.length; i += 1) { - if (!chainInfos[i].chainID.equals(sortedByChainID[i].chainID)) { - throw new Error('chainInfos is not ordered lexicographically by chainID.'); - } + if (!objectUtils.isBufferArrayOrdered(chainIDs)) { + throw new Error('chainInfos is not ordered lexicographically by chainID.'); } // The entries chainData.name must be pairwise distinct @@ -307,13 +308,17 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule // verify root level properties for (const chainInfo of chainInfos) { this._verifyChainID(chainInfo.chainID, mainchainID, 'chainInfo.'); - this._verifyChainData(ctx, chainInfo); + this._verifyChainData(ctx, chainInfo, terminatedStateAccounts); this._verifyChannelData(ctx, chainInfo); this._verifyChainValidators(chainInfo); } } - private _verifyChainData(ctx: GenesisBlockExecuteContext, chainInfo: ChainInfo) { + private _verifyChainData( + ctx: GenesisBlockExecuteContext, + chainInfo: ChainInfo, + terminatedStateAccounts: TerminatedStateAccountWithChainID[], + ) { const validStatuses = [ChainStatus.REGISTERED, ChainStatus.ACTIVE, ChainStatus.TERMINATED]; const { chainData } = chainInfo; @@ -332,6 +337,17 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule if (!validStatuses.includes(chainData.status)) { throw new Error(`chainData.status must be one of ${validStatuses.join(', ')}`); } + + if (chainData.status === ChainStatus.TERMINATED) { + const accountWithChainID = terminatedStateAccounts.find(accountWithChainIDTemp => + accountWithChainIDTemp.chainID.equals(chainInfo.chainID), + ); + if (!accountWithChainID) { + throw new Error( + 'For each chainInfo with status terminated there should be a corresponding entry in terminatedStateAccounts.', + ); + } + } } // https://github.com/LiskHQ/lips/blob/main/proposals/lip-0045.md#mainchain @@ -340,10 +356,15 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule terminatedStateAccounts: TerminatedStateAccountWithChainID[], mainchainID: Buffer, ) { + this._verifyTerminatedStateAccountsIDs( + terminatedStateAccounts.map(a => a.chainID), + mainchainID, + ); + // Sanity check to fulfill if-and-only-if situation - for (const account of terminatedStateAccounts) { + for (const terminatedStateAccountWithChainID of terminatedStateAccounts) { const correspondingChainInfo = chainInfos.find(chainInfo => - chainInfo.chainID.equals(account.chainID), + chainInfo.chainID.equals(terminatedStateAccountWithChainID.chainID), ); if ( !correspondingChainInfo || @@ -353,42 +374,13 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule 'For each terminatedStateAccount there should be a corresponding chainInfo at TERMINATED state.', ); } - } - - for (const chainInfo of chainInfos) { - // For each entry chainInfo in chainInfos, chainInfo.chainData.status == CHAIN_STATUS_TERMINATED - // if and only if a corresponding entry (i.e., with chainID == chainInfo.chainID) exists in terminatedStateAccounts. - if (chainInfo.chainData.status === ChainStatus.TERMINATED) { - const terminatedAccount = terminatedStateAccounts.find(tAccount => - tAccount.chainID.equals(chainInfo.chainID), - ); - if (!terminatedAccount) { - throw new Error( - 'For each chainInfo with status terminated there should be a corresponding entry in terminatedStateAccounts.', - ); - } - } - } - this._verifyTerminatedStateAccountsCommon(terminatedStateAccounts, mainchainID); - - // For each entry stateAccount in terminatedStateAccounts holds - // stateAccount.stateRoot == chainData.lastCertificate.stateRoot, - // stateAccount.mainchainStateRoot == EMPTY_HASH, and - // stateAccount.initialized == True. - // Here chainData is the corresponding entry (i.e., with chainID == stateAccount.chainID) in chainInfos. - - for (const terminatedStateAccountWithChainID of terminatedStateAccounts) { const stateAccount = terminatedStateAccountWithChainID.terminatedStateAccount; // For each entry stateAccount in terminatedStateAccounts holds // stateAccount.stateRoot == chainData.lastCertificate.stateRoot, // stateAccount.mainchainStateRoot == EMPTY_HASH, and // stateAccount.initialized == True. // Here chainData is the corresponding entry (i.e., with chainID == stateAccount.chainID) in chainInfos. - const correspondingChainInfo = chainInfos.find(chainInfo => - chainInfo.chainID.equals(terminatedStateAccountWithChainID.chainID), - ) as ChainInfo; // at this point, it's not undefined, since similar check already applied above - if ( !stateAccount.stateRoot.equals(correspondingChainInfo.chainData.lastCertificate.stateRoot) ) { @@ -422,13 +414,8 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule } // terminatedOutboxAccounts is ordered lexicographically by outboxAccount.chainID - const sortedByChainID = [...terminatedOutboxAccounts].sort((a, b) => - a.chainID.compare(b.chainID), - ); - for (let i = 0; i < terminatedOutboxAccounts.length; i += 1) { - if (!terminatedOutboxAccounts[i].chainID.equals(sortedByChainID[i].chainID)) { - throw new Error('terminatedOutboxAccounts must be ordered lexicographically by chainID.'); - } + if (!objectUtils.isBufferArrayOrdered(chainIDs)) { + throw new Error('terminatedOutboxAccounts must be ordered lexicographically by chainID.'); } // Furthermore, an entry outboxAccount in terminatedOutboxAccounts must have a corresponding entry diff --git a/framework/src/modules/interoperability/sidechain/module.ts b/framework/src/modules/interoperability/sidechain/module.ts index 2d7780e6fc8..072ed94cb71 100644 --- a/framework/src/modules/interoperability/sidechain/module.ts +++ b/framework/src/modules/interoperability/sidechain/module.ts @@ -320,7 +320,10 @@ export class SidechainInteroperabilityModule extends BaseInteroperabilityModule terminatedStateAccounts: TerminatedStateAccountWithChainID[], mainchainID: Buffer, ) { - this._verifyTerminatedStateAccountsCommon(terminatedStateAccounts, mainchainID); + this._verifyTerminatedStateAccountsIDs( + terminatedStateAccounts.map(a => a.chainID), + mainchainID, + ); for (const stateAccount of terminatedStateAccounts) { // and stateAccount.chainID != OWN_CHAIN_ID. diff --git a/framework/test/unit/modules/interoperability/base_interoperability_module.spec.ts b/framework/test/unit/modules/interoperability/base_interoperability_module.spec.ts index 0a92b2ed446..4c15a320356 100644 --- a/framework/test/unit/modules/interoperability/base_interoperability_module.spec.ts +++ b/framework/test/unit/modules/interoperability/base_interoperability_module.spec.ts @@ -445,7 +445,7 @@ must NOT have more than ${MAX_NUM_VALIDATORS} items`, }); }); - describe('_verifyTerminatedStateAccountsCommon', () => { + describe('_verifyTerminatedStateAccountsIDs', () => { certificateThreshold = BigInt(10); const validChainInfos = [ { diff --git a/framework/test/unit/modules/interoperability/mainchain/module.spec.ts b/framework/test/unit/modules/interoperability/mainchain/module.spec.ts index 4a7561c6b40..a75e2eb4a5c 100644 --- a/framework/test/unit/modules/interoperability/mainchain/module.spec.ts +++ b/framework/test/unit/modules/interoperability/mainchain/module.spec.ts @@ -300,19 +300,17 @@ describe('initGenesisState', () => { ].join(', ')}`, ); }); - }); - describe('terminatedStateAccounts', () => { - it('should not throw error if length of terminatedStateAccounts is zero', async () => { + it('should throw if chainInfo.chainData.status === TERMINATED exists but no terminateStateAccount', async () => { const context = createInitGenesisStateContext( { ...genesisInteroperability, - // this is needed to verify `validatorsHash` related tests (above) chainInfos: [ { ...chainInfo, chainData: { ...chainData, + status: ChainStatus.TERMINATED, lastCertificate: { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), @@ -324,24 +322,29 @@ describe('initGenesisState', () => { }, }, ], + // No terminatedStateAccount terminatedStateAccounts: [], }, params, ); - await expect(interopMod.initGenesisState(context)).resolves.not.toThrow(); + await expect(interopMod.initGenesisState(context)).rejects.toThrow( + `For each chainInfo with status terminated there should be a corresponding entry in terminatedStateAccounts.`, + ); }); + }); - it('should throw if chainInfo.chainData.status===TERMINATED exists but no terminateStateAccount', async () => { + describe('terminatedStateAccounts', () => { + it('should not throw error if length of terminatedStateAccounts is zero', async () => { const context = createInitGenesisStateContext( { ...genesisInteroperability, + // this is needed to verify `validatorsHash` related tests (above) chainInfos: [ { ...chainInfo, chainData: { ...chainData, - status: ChainStatus.TERMINATED, lastCertificate: { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), @@ -353,18 +356,17 @@ describe('initGenesisState', () => { }, }, ], - // No terminatedStateAccount terminatedStateAccounts: [], }, params, ); - await expect(interopMod.initGenesisState(context)).rejects.toThrow( - `For each chainInfo with status terminated there should be a corresponding entry in terminatedStateAccounts.`, - ); + await expect(interopMod.initGenesisState(context)).resolves.not.toThrow(); }); - it('should throw if there is an entry in terminateStateAccounts for a chainID that is ACTIVE in chainInfos', async () => { + it('should call _verifyTerminatedStateAccountsIDs', async () => { + jest.spyOn(interopMod, '_verifyTerminatedStateAccountsIDs' as any); + const context = createInitGenesisStateContext( { ...genesisInteroperability, @@ -373,7 +375,7 @@ describe('initGenesisState', () => { ...chainInfo, chainData: { ...chainData, - status: ChainStatus.ACTIVE, + status: ChainStatus.TERMINATED, lastCertificate: { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), @@ -395,21 +397,20 @@ describe('initGenesisState', () => { params, ); - await expect(interopMod.initGenesisState(context)).rejects.toThrow( - `For each terminatedStateAccount there should be a corresponding chainInfo at TERMINATED state`, - ); + await expect(interopMod.initGenesisState(context)).resolves.toBeUndefined(); + expect(interopMod['_verifyTerminatedStateAccountsIDs']).toHaveBeenCalledTimes(1); }); - it('should throw error if chainInfo.chainID exists in terminatedStateAccounts & chainInfo.chainData.status !== CHAIN_STATUS_TERMINATED', async () => { + it('should throw error if chainInfo.chainID exists in terminatedStateAccounts & chainInfo.chainData.status is ACTIVE', async () => { const context = createInitGenesisStateContext( { ...genesisInteroperability, - // this is needed to verify `validatorsHash` related tests (above) chainInfos: [ { ...chainInfo, chainData: { ...chainData, + status: ChainStatus.ACTIVE, lastCertificate: { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), @@ -436,7 +437,7 @@ describe('initGenesisState', () => { ); }); - it('should throw error if chainID in terminatedStateAccounts does not exist in chainInfo', async () => { + it('should throw error if chainInfo.chainID exists in terminatedStateAccounts & chainInfo.chainData.status is REGISTERED', async () => { const context = createInitGenesisStateContext( { ...genesisInteroperability, @@ -450,7 +451,6 @@ describe('initGenesisState', () => { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), }, - status: ChainStatus.TERMINATED, }, chainValidators: { activeValidators, @@ -460,7 +460,7 @@ describe('initGenesisState', () => { ], terminatedStateAccounts: [ { - chainID: Buffer.from([0, 0, 0, 2]), + chainID: chainInfo.chainID, terminatedStateAccount, }, ], @@ -469,22 +469,20 @@ describe('initGenesisState', () => { ); await expect(interopMod.initGenesisState(context)).rejects.toThrow( - 'For each terminatedStateAccount there should be a corresponding chainInfo at TERMINATED state', + `For each terminatedStateAccount there should be a corresponding chainInfo at TERMINATED state`, ); }); - it('should call _verifyTerminatedStateAccountsCommon', async () => { - jest.spyOn(interopMod, '_verifyTerminatedStateAccountsCommon' as any); - + it('should throw error if chainID in terminatedStateAccounts does not exist in chainInfo', async () => { const context = createInitGenesisStateContext( { ...genesisInteroperability, + // this is needed to verify `validatorsHash` related tests (above) chainInfos: [ { ...chainInfo, chainData: { ...chainData, - status: ChainStatus.TERMINATED, lastCertificate: { ...lastCertificate, validatorsHash: computeValidatorsHash(activeValidators, certificateThreshold), @@ -498,7 +496,7 @@ describe('initGenesisState', () => { ], terminatedStateAccounts: [ { - chainID: chainInfo.chainID, + chainID: Buffer.from([0, 0, 0, 2]), terminatedStateAccount, }, ], @@ -506,8 +504,9 @@ describe('initGenesisState', () => { params, ); - await expect(interopMod.initGenesisState(context)).resolves.toBeUndefined(); - expect(interopMod['_verifyTerminatedStateAccountsCommon']).toHaveBeenCalledTimes(1); + await expect(interopMod.initGenesisState(context)).rejects.toThrow( + 'For each terminatedStateAccount there should be a corresponding chainInfo at TERMINATED state', + ); }); it('should throw error if some stateAccount in terminatedStateAccounts have stateRoot not equal to chainData.lastCertificate.stateRoot', async () => { From 7b7321598ee899f4129137da37d249d5a95539e6 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Fri, 13 Oct 2023 12:38:33 +0300 Subject: [PATCH 4/7] Use BigInt --- framework/src/modules/interoperability/mainchain/module.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/modules/interoperability/mainchain/module.ts b/framework/src/modules/interoperability/mainchain/module.ts index 1d60ddf5eab..3cb4ec72678 100644 --- a/framework/src/modules/interoperability/mainchain/module.ts +++ b/framework/src/modules/interoperability/mainchain/module.ts @@ -265,7 +265,7 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule // If chainInfos is non-empty, ownChainNonce > 0 if (chainInfos.length === 0 && ownChainNonce !== BigInt(0)) { throw new Error(`ownChainNonce must be 0 if chainInfos is empty.`); - } else if (chainInfos.length !== 0 && ownChainNonce <= 0) { + } else if (chainInfos.length !== 0 && ownChainNonce <= BigInt(0)) { throw new Error(`ownChainNonce must be positive if chainInfos is not empty.`); } From e13d544120327bc12dd3d5bc21dc03f2cce372d5 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Fri, 13 Oct 2023 13:11:11 +0300 Subject: [PATCH 5/7] Refactor to avoid loop --- .../interoperability/base_interoperability_module.ts | 6 +----- framework/src/modules/interoperability/mainchain/module.ts | 7 +++---- framework/src/modules/interoperability/sidechain/module.ts | 7 +++---- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/framework/src/modules/interoperability/base_interoperability_module.ts b/framework/src/modules/interoperability/base_interoperability_module.ts index 69c5e7a55f3..1986f2f3226 100644 --- a/framework/src/modules/interoperability/base_interoperability_module.ts +++ b/framework/src/modules/interoperability/base_interoperability_module.ts @@ -181,7 +181,7 @@ export abstract class BaseInteroperabilityModule extends BaseInteroperableModule } } - protected _verifyTerminatedStateAccountsIDs(chainIDs: Buffer[], mainchainID: Buffer) { + protected _verifyTerminatedStateAccountsIDs(chainIDs: Buffer[]) { // Each entry stateAccount in terminatedStateAccounts has a unique stateAccount.chainID if (!objectUtils.bufferArrayUniqueItems(chainIDs)) { throw new Error(`terminatedStateAccounts don't hold unique chainID.`); @@ -191,10 +191,6 @@ export abstract class BaseInteroperabilityModule extends BaseInteroperableModule if (!objectUtils.isBufferArrayOrdered(chainIDs)) { throw new Error('terminatedStateAccounts must be ordered lexicographically by chainID.'); } - - for (const chainID of chainIDs) { - this._verifyChainID(chainID, mainchainID, 'stateAccount.'); - } } // https://github.com/LiskHQ/lips/blob/main/proposals/lip-0045.md#genesis-state-processing diff --git a/framework/src/modules/interoperability/mainchain/module.ts b/framework/src/modules/interoperability/mainchain/module.ts index 3cb4ec72678..9620ed015fd 100644 --- a/framework/src/modules/interoperability/mainchain/module.ts +++ b/framework/src/modules/interoperability/mainchain/module.ts @@ -356,13 +356,12 @@ export class MainchainInteroperabilityModule extends BaseInteroperabilityModule terminatedStateAccounts: TerminatedStateAccountWithChainID[], mainchainID: Buffer, ) { - this._verifyTerminatedStateAccountsIDs( - terminatedStateAccounts.map(a => a.chainID), - mainchainID, - ); + this._verifyTerminatedStateAccountsIDs(terminatedStateAccounts.map(a => a.chainID)); // Sanity check to fulfill if-and-only-if situation for (const terminatedStateAccountWithChainID of terminatedStateAccounts) { + this._verifyChainID(terminatedStateAccountWithChainID.chainID, mainchainID, 'stateAccount.'); + const correspondingChainInfo = chainInfos.find(chainInfo => chainInfo.chainID.equals(terminatedStateAccountWithChainID.chainID), ); diff --git a/framework/src/modules/interoperability/sidechain/module.ts b/framework/src/modules/interoperability/sidechain/module.ts index 072ed94cb71..6a3cd0a93d1 100644 --- a/framework/src/modules/interoperability/sidechain/module.ts +++ b/framework/src/modules/interoperability/sidechain/module.ts @@ -320,12 +320,11 @@ export class SidechainInteroperabilityModule extends BaseInteroperabilityModule terminatedStateAccounts: TerminatedStateAccountWithChainID[], mainchainID: Buffer, ) { - this._verifyTerminatedStateAccountsIDs( - terminatedStateAccounts.map(a => a.chainID), - mainchainID, - ); + this._verifyTerminatedStateAccountsIDs(terminatedStateAccounts.map(a => a.chainID)); for (const stateAccount of terminatedStateAccounts) { + this._verifyChainID(stateAccount.chainID, mainchainID, 'stateAccount.'); + // and stateAccount.chainID != OWN_CHAIN_ID. if (stateAccount.chainID.equals(ctx.chainID)) { throw new Error(`stateAccount.chainID must not be equal to OWN_CHAIN_ID.`); From 0f7f63eeda4211f17c77044546e00c74e9fa063c Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Tue, 17 Oct 2023 17:57:10 +0300 Subject: [PATCH 6/7] Update genesis block --- .../config/default/genesis_assets.json | 82 ++++++++++++++++-- .../config/default/genesis_block.blob | Bin 7293 -> 7874 bytes 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json b/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json index 343f2580f36..df0b6ad9b36 100644 --- a/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json +++ b/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json @@ -4,10 +4,72 @@ "module": "interoperability", "data": { "ownChainName": "lisk_mainchain", - "ownChainNonce": 0, - "chainInfos": [], - "terminatedStateAccounts": [], - "terminatedOutboxAccounts": [] + "ownChainNonce": "123", + "chainInfos": [ + { + "chainID": "04000001", + "chainData": { + "name": "dummy", + "lastCertificate": { + "height": 567467, + "timestamp": 1000, + "stateRoot": "0000000000000000000000000000000000000000000000000000000000000000", + "validatorsHash": "a5a053d50182ea0c33bc03594cf4760f11d67cdba407d16bc0512fabb468253a" + }, + "status": 2 + }, + "channelData": { + "inbox": { + "appendPath": [ + "0000000000000000000000000000000000000000000000000000000000000000", + "0000000000000000000000000000000000000000000000000000000000000000" + ], + "size": 18, + "root": "9e37ffe87f08f6b7952d82acdb3376046792533ebf2ba8908dda3ebc813dac9a" + }, + "outbox": { + "appendPath": [ + "0000000000000000000000000000000000000000000000000000000000000000", + "0000000000000000000000000000000000000000000000000000000000000000" + ], + "size": 18, + "root": "221a0c844883022e3a54e78eb5dcdc2ed8faa85f648c40659cf5fd2054f36500" + }, + "partnerChainOutboxRoot": "851faa36d87411d625fb4416c33c6a44795cb84155637f9991fcef06d1de7155", + "messageFeeTokenID": "0400000000000000", + "minReturnFeePerByte": "1000" + }, + "chainValidators": { + "activeValidators": [ + { + "blsKey": "3c1e6f29e3434f816cd6697e56cc54bc8d80927bf65a1361b383aa338cd3f63cbf82ce801b752cb32f8ecb3f8cc16835", + "bftWeight": "10" + } + ], + "certificateThreshold": "10" + } + } + ], + "terminatedStateAccounts": [ + { + "chainID": "04000001", + "terminatedStateAccount": { + "stateRoot": "0000000000000000000000000000000000000000000000000000000000000000", + "mainchainStateRoot": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "initialized": true + } + } + ], + "terminatedOutboxAccounts": [ + { + "chainID": "04000001", + "terminatedOutboxAccount": { + "outboxRoot": "0aed892d544980f5b806dbce4bcb65517acf57a7af012c55c0b2f80b188fa290", + "outboxSize": 1, + "partnerChainInboxSize": 1 + } + } + ] }, "schema": { "$id": "/interoperability/module/genesis", @@ -916,11 +978,17 @@ "totalSupply": "10300000000000000" } ], - "escrowSubstore": [], + "escrowSubstore": [ + { + "escrowChainID": "04000001", + "tokenID": "0400000000000000", + "amount": "0" + } + ], "supportedTokensSubstore": [ { - "chainID": "", - "supportedTokenIDs": [] + "chainID": "04000001", + "supportedTokenIDs": ["0400000100000000"] } ] }, diff --git a/examples/interop/pos-mainchain-fast/config/default/genesis_block.blob b/examples/interop/pos-mainchain-fast/config/default/genesis_block.blob index 6d83ad495db344ab90cc09744a31df45abd374bb..6b8bb7c78840ae306937acdb33b3d1b3faae220c 100644 GIT binary patch delta 549 zcmexsambdR>klIbgTVg1yH>JEFeoWZ6qK8&YolPPWnz2tRR65`-CR8-rBinbm734z zWr)9Ubo;Bb(fa3{CU&YQJPX=!WZz!lEmm7L1n+R_ux z8Z(_>nas#2Y`B?)i!Ud$I6FQ!F*7eY14sx|OU+~EVqsxmU=#}DVofQ{&8-yj=2(4E zN#F&$6v!F63_#~BT@ZYgvFR0$@gC+#pD$(ng4b$pFJZr!eIQVO^_C1(D+wm4W+tu- zE|3PGSzua=fk8k>N@1S){}=Tf-?mTHZCZ2NxQr!zQn1~A?G+PxZ`ti>v|Tex2vt`- zM3<5jPm4!0lb%(`^S-Tj?&#h4wIV*H$02pj*S`uOpHms66k6q1ncXN6yr%lwMeMLm zmP=*K4#&{s`k52|yl1<3uP{`J1LzPi(0IYFWWi;^Wnd$huld;7zcJ@pW?k5skUhN( zld8W(2`6rDUS-^K`J2uDrgIH4(xp0^_4`iS_Z-YH72p!!Qu2caj~m!m%nS@d6WtVz zBp9{ypb~0a3S4hHbwfNGzV2YVea`!IYGBp*@a5|nbwUqp`oS&Hzi5I0qXgq-bw+P4 np4CcRtR?x`sd+*Rl{VLL{}xb_5`vlocQ7yk2q1|uf_wo0q{Xzy delta 166 zcmV;X09pUSJ^eTi3j6^G01(@q$fpJv03wr+0U?nuIv`{M+wZaKNn<*m)PojJjnVz4 zKhdT*dvZ{J>;Q4h9c@h!|4s2<2YhP_)X>Ma^VQFp<0JBO1O9~8$A_@g` UZ);_468|Byp$+>G1R??o06h0UumAu6 From 9bd35aa9160168f60970acb5184d638c033ffea3 Mon Sep 17 00:00:00 2001 From: Khalid Hameed Date: Wed, 18 Oct 2023 10:39:53 +0300 Subject: [PATCH 7/7] Update chainID --- .../config/default/genesis_assets.json | 12 ++++++------ .../config/default/genesis_block.blob | Bin 7874 -> 7874 bytes 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json b/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json index df0b6ad9b36..54ed68a6a19 100644 --- a/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json +++ b/examples/interop/pos-mainchain-fast/config/default/genesis_assets.json @@ -7,7 +7,7 @@ "ownChainNonce": "123", "chainInfos": [ { - "chainID": "04000001", + "chainID": "04123456", "chainData": { "name": "dummy", "lastCertificate": { @@ -52,7 +52,7 @@ ], "terminatedStateAccounts": [ { - "chainID": "04000001", + "chainID": "04123456", "terminatedStateAccount": { "stateRoot": "0000000000000000000000000000000000000000000000000000000000000000", "mainchainStateRoot": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", @@ -62,7 +62,7 @@ ], "terminatedOutboxAccounts": [ { - "chainID": "04000001", + "chainID": "04123456", "terminatedOutboxAccount": { "outboxRoot": "0aed892d544980f5b806dbce4bcb65517acf57a7af012c55c0b2f80b188fa290", "outboxSize": 1, @@ -980,15 +980,15 @@ ], "escrowSubstore": [ { - "escrowChainID": "04000001", + "escrowChainID": "04123456", "tokenID": "0400000000000000", "amount": "0" } ], "supportedTokensSubstore": [ { - "chainID": "04000001", - "supportedTokenIDs": ["0400000100000000"] + "chainID": "04123456", + "supportedTokenIDs": ["0412345600000000"] } ] }, diff --git a/examples/interop/pos-mainchain-fast/config/default/genesis_block.blob b/examples/interop/pos-mainchain-fast/config/default/genesis_block.blob index 6b8bb7c78840ae306937acdb33b3d1b3faae220c..c4b5dfdc624d3bef880bc6f301b008221be29d12 100644 GIT binary patch delta 164 zcmX?Pd&riL>klIbgTRZa`&P0^Fepu)z^FJ;-%4S_wCuVc35zVYJ-azQ;z#JKkO-#Q zQn5{6Y7PWPKJGG}FtJlb;f>}_>*K{o>`NA3E1G9B+0!GNYx1naZJ#7+p54(lZ*G}* zHi=otBy6)0<6K5oAVbJ)@5d)GSQ2@U# BI8guq delta 164 zcmX?Pd&riL>klIbgTVg1yH>JEFepu)z^FJ;-%7z!%f$BPss35>ySaKwN~i7=Dm9;?DwWTMX zO=4zXVBBoPIG2$X$PjXyypd69@;4?;u&@$yy|f}on1h7@0)X}laIpY+5HUuOC;+