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

Commit

Permalink
Improve logging and handling of syncing node in chain connector plugin (
Browse files Browse the repository at this point in the history 
#9121)

* ♻️ Handle syncing node and error logging

* Update framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts

Co-authored-by: shuse2 <shuse2@users.noreply.github.com>

* 💅🏻 Fix format error

* 🔥 Remove _syncing property

---------

Co-authored-by: shuse2 <shuse2@users.noreply.github.com>
  • Loading branch information
@ishantiw @shuse2
ishantiw and shuse2 authored Oct 23, 2023
1 parent a943cd0 commit 4994353
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 31 deletions.
26 changes: 21 additions & 5 deletions framework-plugins/lisk-framework-chain-connector-plugin/src/certificate_generation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ export const getCertificateFromAggregateCommit = (
const blockHeader = blockHeaders.find(header => header.height === aggregateCommit.height);

if (!blockHeader) {
throw new Error('No block header found for the given aggregate height.');
throw new Error(
`No block header found for the given aggregate height ${aggregateCommit.height} when calling getCertificateFromAggregateCommit.`,
);
}

return {
Expand All @@ -55,7 +57,11 @@ export const checkChainOfTrust = (
): boolean => {
const blockHeader = blockHeaders.find(header => header.height === aggregateCommit.height - 1);
if (!blockHeader) {
throw new Error('No block header found for the given aggregate height.');
throw new Error(
`No block header found for the given the previous height ${
aggregateCommit.height - 1
} of aggregate commit at height ${aggregateCommit.height} when calling checkChainOfTrust.`,
);
}

// Certificate signers and certificate threshold for aggregateCommit are those authenticated by the last certificate
Expand All @@ -68,7 +74,11 @@ export const checkChainOfTrust = (
data.validatorsHash.equals(blockHeader.validatorsHash),
);
if (!validatorData) {
throw new Error('No validators data found for the given validatorsHash.');
throw new Error(
`No validators data found for the given validatorsHash ${blockHeader.validatorsHash.toString(
'hex',
)}.`,
);
}

for (let i = 0; i < validatorData.validators.length; i += 1) {
Expand Down Expand Up @@ -100,14 +110,20 @@ export const getNextCertificateFromAggregateCommits = (
header => header.height === lastCertificate.height,
);
if (!blockHeaderAtLastCertifiedHeight) {
throw new Error('No block header found for the last certified height.');
throw new Error(
`No block header found for the last certified height ${lastCertificate.height}.`,
);
}

const validatorDataAtLastCertifiedHeight = validatorsHashPreimage.find(data =>
data.validatorsHash.equals(blockHeaderAtLastCertifiedHeight?.validatorsHash),
);
if (!validatorDataAtLastCertifiedHeight) {
throw new Error('No validatorsHash preimage data present for the given validatorsHash.');
throw new Error(
`No validatorsHash preimage data present for the given validatorsHash ${blockHeaderAtLastCertifiedHeight?.validatorsHash.toString(
'hex',
)}.`,
);
}

const blsKeyToBFTWeight: Record<string, bigint> = {};
Expand Down
62 changes: 37 additions & 25 deletions framework-plugins/lisk-framework-chain-connector-plugin/src/chain_connector_plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ export class ChainConnectorPlugin extends BasePlugin<ChainConnectorPluginConfig>
let chainAccountJSON: ChainAccountJSON;
// Save blockHeader, aggregateCommit, validatorsData and cross chain messages if any.
try {
const nodeInfo = await this._sendingChainClient.node.getNodeInfo();
// Fetch last certificate from the receiving chain and update the _lastCertificate
try {
chainAccountJSON = await this._receivingChainClient.invoke<ChainAccountJSON>(
Expand All @@ -232,7 +233,7 @@ export class ChainConnectorPlugin extends BasePlugin<ChainConnectorPluginConfig>
await this._saveDataOnNewBlock(newBlockHeader);
await this._initializeReceivingChainClient();
this.logger.error(
error,
{ err: error as Error },
'Error occurred while accessing receivingChainAPIClient but all data is saved on newBlock.',
);

Expand All @@ -243,34 +244,41 @@ export class ChainConnectorPlugin extends BasePlugin<ChainConnectorPluginConfig>
const { aggregateCommits, blockHeaders, validatorsHashPreimage, crossChainMessages } =
await this._saveDataOnNewBlock(newBlockHeader);

// When all the relevant data is saved successfully then try to create CCU
if (this._ccuFrequency <= newBlockHeader.height - this._lastCertificate.height) {
const computedCCUParams = await this._computeCCUParams(
blockHeaders,
aggregateCommits,
validatorsHashPreimage,
crossChainMessages,
const numOfBlocksSinceLastCertificate = newBlockHeader.height - this._lastCertificate.height;
if (nodeInfo.syncing || this._ccuFrequency > numOfBlocksSinceLastCertificate) {
this.logger.debug(
{
syncing: nodeInfo.syncing,
ccuFrequency: this._ccuFrequency,
nextPossibleCCUHeight: this._ccuFrequency - numOfBlocksSinceLastCertificate,
},
'No attempt to create CCU either due to ccuFrequency or the node is syncing',
);

if (computedCCUParams) {
try {
await this._submitCCU(codec.encode(ccuParamsSchema, computedCCUParams.ccuParams));
// If CCU was sent successfully then save the lastSentCCM if any
// TODO: Add function to check on the receiving chain whether last sent CCM was accepted or not
if (computedCCUParams.lastCCMToBeSent) {
await this._chainConnectorStore.setLastSentCCM(computedCCUParams.lastCCMToBeSent);
}
} catch (error) {
this.logger.info(
{ err: error },
`Error occured while submitting CCU for the blockHeader at height: ${newBlockHeader.height}`,
);
return;
return;
}
// When all the relevant data is saved successfully then try to create CCU
const computedCCUParams = await this._computeCCUParams(
blockHeaders,
aggregateCommits,
validatorsHashPreimage,
crossChainMessages,
);

if (computedCCUParams) {
try {
await this._submitCCU(codec.encode(ccuParamsSchema, computedCCUParams.ccuParams));
// If CCU was sent successfully then save the lastSentCCM if any
// TODO: Add function to check on the receiving chain whether last sent CCM was accepted or not
if (computedCCUParams.lastCCMToBeSent) {
await this._chainConnectorStore.setLastSentCCM(computedCCUParams.lastCCMToBeSent);
}
} else {
} catch (error) {
this.logger.info(
`No valid CCU can be generated for the height: ${newBlockHeader.height}`,
{ err: error },
`Error occured while submitting CCU for the blockHeader at height: ${newBlockHeader.height}`,
);
return;
}
}
} catch (error) {
Expand Down Expand Up @@ -453,7 +461,11 @@ export class ChainConnectorPlugin extends BasePlugin<ChainConnectorPluginConfig>

if (this._lastCertificate.height === 0) {
for (const aggregateCommit of aggregateCommits) {
if (aggregateCommit.height < this._registrationHeight) {
// If blockHeader corresponding to aggregateCommit height does not exist then try with the next aggregCommit.
const blockHeaderExist = blockHeaders.find(
header => header.height === aggregateCommit.height,
);
if (!blockHeaderExist || aggregateCommit.height < this._registrationHeight) {
continue;
}

Expand Down
4 changes: 3 additions & 1 deletion ...rk-plugins/lisk-framework-chain-connector-plugin/test/unit/certificate_generation.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ describe('certificate generation', () => {
[lastCertifiedBlock],
[validatorsDataAtLastCertifiedHeight],
),
).toThrow('No block header found for the given aggregate height');
).toThrow(
'No block header found for the given the previous height 5 of aggregate commit at height 6 when calling checkChainOfTrust.',
);
});

it('should throw error when there is no validatorsData at {aggregateCommit.height - 1}', () => {
Expand Down
47 changes: 47 additions & 0 deletions framework-plugins/lisk-framework-chain-connector-plugin/test/unit/plugin.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ describe('ChainConnectorPlugin', () => {
invoke: jest.fn(),
subscribe: jest.fn(),
connect: jest.fn(),
node: { getNodeInfo: jest.fn() },
});

const chainConnectorStoreMock = {
Expand Down Expand Up @@ -228,6 +229,10 @@ describe('ChainConnectorPlugin', () => {
chainID: ownChainID.toString('hex'),
});

jest.spyOn(sendingChainAPIClientMock.node, 'getNodeInfo').mockResolvedValue({
syncing: false,
} as never);

when(receivingChainAPIClientMock.invoke)
.calledWith('interoperability_getOwnChainAccount')
.mockResolvedValue({
Expand Down Expand Up @@ -677,6 +682,47 @@ describe('ChainConnectorPlugin', () => {
expect(chainConnectorPlugin['_submitCCU']).toHaveBeenCalled();
});

it('should not computeCCUParams if node is syncing', async () => {
jest
.spyOn(certificateGenerationUtil, 'getNextCertificateFromAggregateCommits')
.mockReturnValue(sampleNextCertificate);

jest.spyOn(testing.mocks.loggerMock, 'debug');
await initChainConnectorPlugin(chainConnectorPlugin, defaultConfig);
chainConnectorPlugin['_apiClient'] = sendingChainAPIClientMock;
await chainConnectorPlugin.load();

const saveDataOnNewBlockMock = jest.fn();
chainConnectorPlugin['_saveDataOnNewBlock'] = saveDataOnNewBlockMock;
saveDataOnNewBlockMock.mockResolvedValue({
aggregateCommits: [],
blockHeaders: [],
validatorsHashPreimage: [],
crossChainMessages: [],
});
jest.spyOn(sendingChainAPIClientMock.node, 'getNodeInfo').mockResolvedValue({
syncing: true,
} as never);
await chainConnectorPlugin['_newBlockHandler']({
blockHeader: block.header.toJSON(),
});

const numOfBlocksSinceLastCertificate =
block.header.height - chainConnectorPlugin['_lastCertificate'].height;
expect(chainConnectorPlugin['logger'].debug).toHaveBeenCalledWith(
{
syncing: true,
ccuFrequency: chainConnectorPlugin['_ccuFrequency'],
nextPossibleCCUHeight:
chainConnectorPlugin['_ccuFrequency'] - numOfBlocksSinceLastCertificate,
},
'No attempt to create CCU either due to ccuFrequency or the node is syncing',
);
// For chain_newBlock and chain_deleteBlock
expect(sendingChainAPIClientMock.subscribe).toHaveBeenCalledTimes(2);
expect(chainConnectorPlugin['_submitCCU']).not.toHaveBeenCalled();
});

it('should invoke "chain_getEvents" on _sendingChainClient', async () => {
jest
.spyOn(certificateGenerationUtil, 'getNextCertificateFromAggregateCommits')
Expand Down Expand Up @@ -830,6 +876,7 @@ describe('ChainConnectorPlugin', () => {
* 5. consensus_getBFTHeights
*/
expect(sendingChainAPIClientMock.invoke).toHaveBeenCalledTimes(5);
expect(sendingChainAPIClientMock.node.getNodeInfo).toHaveBeenCalledTimes(1);
/**
* Two calls to below RPC through receivingChainAPIClient
* 1. interoperability_getChainAccount: in load() function
Expand Down

0 comments on commit 4994353

Please sign in to comment.