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

Improve logging and handling of syncing node in chain connector plugin #9121

Merged
merged 5 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
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