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

Fix range of single commit generation in init #8095

Merged
merged 7 commits into from
Jan 31, 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
7 changes: 0 additions & 7 deletions framework/src/engine/consensus/consensus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,13 +366,6 @@ export class Consensus {
this._commitPool.addCommit(singleCommit, true);
}

public async getMaxRemovalHeight(): Promise<number> {
janhack marked this conversation as resolved.
Show resolved Hide resolved
const finalizedBlockHeader = await this._chain.dataAccess.getBlockHeaderByHeight(
this._chain.finalizedHeight,
);
return finalizedBlockHeader.aggregateCommit.height;
}

public isSynced(height: number, maxHeightPrevoted: number): boolean {
const lastBlockHeader = this._chain.lastBlock.header;
if (lastBlockHeader.version === 0) {
Expand Down
36 changes: 16 additions & 20 deletions framework/src/engine/generator/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,17 @@ export class Generator {
});

const stateStore = new StateStore(this._blockchainDB);
const maxRemovalHeight = await this._consensus.getMaxRemovalHeight();
const { maxHeightPrecommitted } = await this._bft.method.getBFTHeights(stateStore);
await Promise.all(this._handleFinalizedHeightChanged(maxRemovalHeight, maxHeightPrecommitted));

// On node start, it re generates certificate from maxHeightCertified to maxHeightPrecommitted.
// in the _handleFinalizedHeightChanged, it loops between maxHeightCertified + 1 and maxHeightPrecommitted.
// maxHeightCertified is skipped because it has been already certified.
// @see https://github.com/LiskHQ/lips/blob/main/proposals/lip-0061.md#initial-single-commit-creation
const { maxHeightPrecommitted, maxHeightCertified } = await this._bft.method.getBFTHeights(
stateStore,
);
await Promise.all(
shuse2 marked this conversation as resolved.
Show resolved Hide resolved
this._handleFinalizedHeightChanged(maxHeightCertified, maxHeightPrecommitted),
);
}

public get endpoint(): Endpoint {
Expand Down Expand Up @@ -622,10 +630,13 @@ export class Generator {
}

private _handleFinalizedHeightChanged(from: number, to: number): Promise<void>[] {
if (from >= to) {
return [];
}
const promises = [];
const stateStore = new StateStore(this._blockchainDB);
for (const [address, pairs] of this._keypairs.entries()) {
for (let height = from + 1; height <= to; height += 1) {
for (let height = from + 1; height < to; height += 1) {
shuse2 marked this conversation as resolved.
Show resolved Hide resolved
promises.push(
shuse2 marked this conversation as resolved.
Show resolved Hide resolved
this._certifySingleCommitForChangedHeight(
stateStore,
Expand All @@ -635,9 +646,7 @@ export class Generator {
),
);
}
promises.push(
this._certifySingleCommitForFinalizedHeight(stateStore, to, address, pairs.blsSecretKey),
);
promises.push(this._certifySingleCommit(stateStore, to, address, pairs.blsSecretKey));
}
return promises;
}
Expand All @@ -655,19 +664,6 @@ export class Generator {
await this._certifySingleCommit(stateStore, height, generatorAddress, blsSK);
}

private async _certifySingleCommitForFinalizedHeight(
stateStore: StateStore,
height: number,
generatorAddress: Buffer,
blsSK: Buffer,
): Promise<void> {
const paramExist = await this._bft.method.existBFTParameters(stateStore, height + 1);
if (paramExist) {
return;
}
await this._certifySingleCommit(stateStore, height, generatorAddress, blsSK);
}

private async _certifySingleCommit(
stateStore: StateStore,
height: number,
Expand Down
1 change: 0 additions & 1 deletion framework/src/engine/generator/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export interface Consensus {
finalizedHeight: () => number;
getAggregateCommit: (stateStore: StateStore) => Promise<AggregateCommit>;
certifySingleCommit: (blockHeader: BlockHeader, validatorInfo: ValidatorInfo) => void;
getMaxRemovalHeight: () => Promise<number>;
readonly events: EventEmitter;
}

Expand Down
1 change: 0 additions & 1 deletion framework/test/unit/engine/engine.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe('engine', () => {
jest
.spyOn(Chain.prototype, 'lastBlock', 'get')
.mockReturnValue({ header: { height: 300 } } as never);
jest.spyOn(Consensus.prototype, 'getMaxRemovalHeight').mockResolvedValue(0);
jest
.spyOn(BFTMethod.prototype, 'getBFTHeights')
.mockResolvedValue({ maxHeightPrecommitted: 0 } as never);
Expand Down
8 changes: 3 additions & 5 deletions framework/test/unit/engine/generator/generator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,8 @@ describe('generator', () => {
consensusEvent = new EventEmitter();
consensus = {
execute: jest.fn(),

getAggregateCommit: jest.fn(),
certifySingleCommit: jest.fn(),
getMaxRemovalHeight: jest.fn().mockResolvedValue(0),
getConsensusParams: jest.fn().mockResolvedValue({
currentValidators: [],
implyMaxPrevote: true,
Expand Down Expand Up @@ -199,12 +197,11 @@ describe('generator', () => {
expect(generator['_keypairs'].values()).toHaveLength(103);
});

it('should handle finalized height change between max removal height and max height precommitted', async () => {
it('should handle finalized height change between maxHeightCertified + 1 and max height precommitted', async () => {
jest.spyOn(generator, '_handleFinalizedHeightChanged' as any).mockReturnValue([] as never);
jest.spyOn(generator['_consensus'], 'getMaxRemovalHeight').mockResolvedValue(313);
jest
.spyOn(generator['_bft'].method, 'getBFTHeights')
.mockResolvedValue({ maxHeightPrecommitted: 515 } as never);
.mockResolvedValue({ maxHeightPrecommitted: 515, maxHeightCertified: 313 } as never);

await generator.init({
blockchainDB,
Expand All @@ -214,6 +211,7 @@ describe('generator', () => {
expect(generator['_handleFinalizedHeightChanged']).toHaveBeenCalledWith(313, 515);
});
});

describe('saveKeysFromFile', () => {
it('should not store any data when generator.keys.fromFile is not defined', async () => {
generator['_config'].generator = { keys: {} };
Expand Down