From 798a22af038a09900e8eb5395e4d044ca494a20e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boban=20Milo=C5=A1evi=C4=87?= Date: Sat, 15 Apr 2023 14:50:28 +0700 Subject: [PATCH 1/3] Sorting Commit List without using a conditional --- .../consensus/certificate_generation/commit_list.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/framework/src/engine/consensus/certificate_generation/commit_list.ts b/framework/src/engine/consensus/certificate_generation/commit_list.ts index 9a4575273d9..84d57dc44da 100644 --- a/framework/src/engine/consensus/certificate_generation/commit_list.ts +++ b/framework/src/engine/consensus/certificate_generation/commit_list.ts @@ -15,8 +15,8 @@ import { SingleCommit } from './types'; export enum COMMIT_SORT { - ASC = 0, - DSC, + ASC = 1, + DSC = -1, } export class CommitList { @@ -63,10 +63,8 @@ export class CommitList { } } - public getAll(ascendingHeight = COMMIT_SORT.ASC) { - return ascendingHeight === COMMIT_SORT.ASC - ? [...this._commitMap.values()].flat().sort((a, b) => a.height - b.height) - : [...this._commitMap.values()].flat().sort((a, b) => b.height - a.height); + public getAll(sortOrder = COMMIT_SORT.ASC) { + return [...this._commitMap.values()].flat().sort((a, b) => sortOrder * (a.height - b.height)); } public deleteSingle(commit: SingleCommit) { From 2e118c7eace78b0459e3829d49486fdab7d59297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boban=20Milo=C5=A1evi=C4=87?= Date: Sat, 15 Apr 2023 14:51:52 +0700 Subject: [PATCH 2/3] Remove sort order argument when using the default sort order --- .../certificate_generation/commit_pool.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/framework/src/engine/consensus/certificate_generation/commit_pool.ts b/framework/src/engine/consensus/certificate_generation/commit_pool.ts index 4281c3287e0..2aac74565f9 100644 --- a/framework/src/engine/consensus/certificate_generation/commit_pool.ts +++ b/framework/src/engine/consensus/certificate_generation/commit_pool.ts @@ -93,11 +93,8 @@ export class CommitPool { public async validateCommit(methodContext: StateStore, commit: SingleCommit): Promise { // Validation step 1 const existsInNonGossiped = this._nonGossipedCommits.exists(commit); - const existsInNonGossipedLocal = this._nonGossipedCommitsLocal.exists(commit); - const existsInGossiped = this._gossipedCommits.exists(commit); - const doesCommitExist = existsInGossiped || existsInNonGossiped || existsInNonGossipedLocal; if (doesCommitExist) { @@ -374,6 +371,7 @@ export class CommitPool { this._nonGossipedCommits.deleteByHeight(height); } this._metrics.nonGossippedCommits.set(this._nonGossipedCommits.size()); + // Clean up nonGossipedCommitsLocal const deletedNonGossipedHeightsLocal = await this._getDeleteHeights( methodContext, @@ -386,6 +384,7 @@ export class CommitPool { this._nonGossipedCommitsLocal.deleteByHeight(height); } this._metrics.nonGossippedCommitsLocal.set(this._nonGossipedCommitsLocal.size()); + // Clean up gossipedCommits const deletedGossipedHeights = await this._getDeleteHeights( methodContext, @@ -398,6 +397,7 @@ export class CommitPool { this._gossipedCommits.deleteByHeight(height); } this._metrics.gossippedCommits.set(this._gossipedCommits.size()); + // 2. Select commits to gossip const nextHeight = this._chain.lastBlock.header.height + 1; const { validators } = await this._bftMethod.getBFTParameters(methodContext, nextHeight); @@ -419,18 +419,17 @@ export class CommitPool { } } + // 2.2 Select newly created commits by generator // Non gossiped commits with descending order of height by generator const sortedNonGossipedCommitsLocal = this._nonGossipedCommitsLocal.getAll(COMMIT_SORT.DSC); - - // 2.2 Select newly created commits by generator for (const commit of sortedNonGossipedCommitsLocal) { if (selectedCommits.length >= maxSelectedCommitsLength) { break; } selectedCommits.push(commit); } - // 2.3 Select newly received commits by others + // 2.3 Select newly received commits by others // Non gossiped commits with descending order of height const sortedNonGossipedCommits = this._nonGossipedCommits.getAll(COMMIT_SORT.DSC); for (const commit of sortedNonGossipedCommits) { @@ -443,11 +442,13 @@ export class CommitPool { const encodedCommitArray = selectedCommits.map(commit => codec.encode(singleCommitSchema, commit), ); + // 3. Gossip an array of up to 2*numActiveValidators commit messages to 16 randomly chosen connected peers with at least 8 of them being outgoing peers (same parameters as block propagation) this._network.send({ event: NETWORK_EVENT_COMMIT_MESSAGES, data: codec.encode(singleCommitsNetworkPacketSchema, { commits: encodedCommitArray }), }); + // 4. Move any gossiped commit message included in nonGossipedCommits, nonGossipedCommitsLocal to gossipedCommits. for (const commit of selectedCommits) { if (!this._gossipedCommits.exists(commit)) { @@ -505,12 +506,12 @@ export class CommitPool { return blockHeader.aggregateCommit.height; } - private _getAllCommits(ascendingHeight = COMMIT_SORT.ASC): SingleCommit[] { + private _getAllCommits(): SingleCommit[] { // Flattened list of all the single commits from both gossiped and non gossiped list sorted by ascending order of height return [ - ...this._nonGossipedCommits.getAll(ascendingHeight), - ...this._nonGossipedCommitsLocal.getAll(ascendingHeight), - ...this._gossipedCommits.getAll(ascendingHeight), + ...this._nonGossipedCommits.getAll(), + ...this._nonGossipedCommitsLocal.getAll(), + ...this._gossipedCommits.getAll(), ].sort((a, b) => a.height - b.height); } } From 8c8c40798ef4254ca4f745ea90719677388de0bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boban=20Milo=C5=A1evi=C4=87?= Date: Mon, 17 Apr 2023 16:29:46 +0700 Subject: [PATCH 3/3] Remove unnecessary logic from deleteByHeight() --- .../engine/consensus/certificate_generation/commit_list.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/framework/src/engine/consensus/certificate_generation/commit_list.ts b/framework/src/engine/consensus/certificate_generation/commit_list.ts index 84d57dc44da..d7f9ab30085 100644 --- a/framework/src/engine/consensus/certificate_generation/commit_list.ts +++ b/framework/src/engine/consensus/certificate_generation/commit_list.ts @@ -55,12 +55,7 @@ export class CommitList { } public deleteByHeight(height: number) { - if (this._commitMap.delete(height)) { - // Delete empty array entry - if (this._commitMap.get(height) && this._commitMap.get(height)?.length === 0) { - this._commitMap.delete(height); - } - } + this._commitMap.delete(height); } public getAll(sortOrder = COMMIT_SORT.ASC) {