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

Identify commits created by generator - Closes #6969 #6974

Merged
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
46 changes: 33 additions & 13 deletions framework/src/node/consensus/certificate_generation/commit_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ import { CommitList, COMMIT_SORT } from './commit_list';

export class CommitPool {
private readonly _nonGossipedCommits: CommitList;
private readonly _nonGossipedCommitsLocal: CommitList;
private readonly _gossipedCommits: CommitList;
private readonly _blockTime: number;
private readonly _bftAPI: BFTAPI;
private readonly _validatorsAPI: ValidatorAPI;
private readonly _chain: Chain;
private readonly _network: Network;
private readonly _db: KVStore;
private readonly _generatorAddress: Buffer;
private _jobIntervalID!: NodeJS.Timeout;

public constructor(config: CommitPoolConfig) {
Expand All @@ -54,8 +54,8 @@ export class CommitPool {
this._chain = config.chain;
this._network = config.network;
this._db = config.db;
this._generatorAddress = config.generatorAddress;
this._nonGossipedCommits = new CommitList();
this._nonGossipedCommitsLocal = new CommitList();
this._gossipedCommits = new CommitList();
}

Expand All @@ -72,9 +72,13 @@ export class CommitPool {
clearInterval(this._jobIntervalID);
}

public addCommit(commit: SingleCommit): void {
if (!this._nonGossipedCommits.exists(commit)) {
this._nonGossipedCommits.add(commit);
public addCommit(commit: SingleCommit, local = false): void {
if (!this._nonGossipedCommits.exists(commit) && !this._nonGossipedCommitsLocal.exists(commit)) {
if (local) {
this._nonGossipedCommitsLocal.add(commit);
} else {
this._nonGossipedCommits.add(commit);
}
}
}

Expand All @@ -90,9 +94,11 @@ export class CommitPool {
// 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;
const doesCommitExist = existsInGossiped || existsInNonGossiped || existsInNonGossipedLocal;

if (doesCommitExist) {
return false;
Expand Down Expand Up @@ -149,8 +155,9 @@ export class CommitPool {

public getCommitsByHeight(height: number): SingleCommit[] {
const nonGossipedCommits = this._nonGossipedCommits.getByHeight(height);
const nonGossipedCommitsLocal = this._nonGossipedCommitsLocal.getByHeight(height);
const gossipedCommits = this._gossipedCommits.getByHeight(height);
return [...nonGossipedCommits, ...gossipedCommits];
return [...nonGossipedCommits, ...nonGossipedCommitsLocal, ...gossipedCommits];
}

public createSingleCommit(
Expand Down Expand Up @@ -331,6 +338,7 @@ export class CommitPool {
while (nextHeight > maxHeightCertified) {
const singleCommits = [
...this._nonGossipedCommits.getByHeight(nextHeight),
...this._nonGossipedCommitsLocal.getByHeight(nextHeight),
...this._gossipedCommits.getByHeight(nextHeight),
];
const nextValidators = singleCommits.map(commit => commit.validatorAddress);
Expand Down Expand Up @@ -381,6 +389,16 @@ export class CommitPool {
for (const height of deletedNonGossipedHeights) {
this._nonGossipedCommits.deleteByHeight(height);
}
// Clean up nonGossipedCommitsLocal
const deletedNonGossipedHeightsLocal = await this._getDeleteHeights(
apiContext,
this._nonGossipedCommitsLocal,
removalHeight,
maxHeightPrecommitted,
);
for (const height of deletedNonGossipedHeightsLocal) {
this._nonGossipedCommits.deleteByHeight(height);
}
// Clean up gossipedCommits
const deletedGossipedHeights = await this._getDeleteHeights(
apiContext,
Expand Down Expand Up @@ -411,15 +429,15 @@ export class CommitPool {
// Non gossiped commits with descending order of height
const sortedNonGossipedCommits = this._nonGossipedCommits.getAll(COMMIT_SORT.DSC);

// 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 [index, commit] of sortedNonGossipedCommits.entries()) {
for (const commit of sortedNonGossipedCommitsLocal) {
if (selectedCommits.length >= maxSelectedCommitsLength) {
break;
}
if (commit.validatorAddress.equals(this._generatorAddress)) {
selectedCommits.push(commit);
sortedNonGossipedCommits.splice(index, 1);
}
selectedCommits.push(commit);
}
// 2.3 Select newly received commits by others
for (const commit of sortedNonGossipedCommits) {
Expand All @@ -437,10 +455,11 @@ export class CommitPool {
event: NETWORK_EVENT_COMMIT_MESSAGES,
data: codec.encode(singleCommitsNetworkPacketSchema, { commits: encodedCommitArray }),
});
// 4. Move any gossiped commit message included in nonGossipedCommits to gossipedCommits.
// 4. Move any gossiped commit message included in nonGossipedCommits, nonGossipedCommitsLocal to gossipedCommits.
for (const commit of selectedCommits) {
this._gossipedCommits.add(commit);
this._nonGossipedCommits.deleteSingle(commit);
this._nonGossipedCommitsLocal.deleteSingle(commit);
}
}

Expand Down Expand Up @@ -494,6 +513,7 @@ export class CommitPool {
// 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),
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,25 +242,36 @@ describe('CommitPool', () => {
});

it('should select non gossiped commits that are created by the generator of the node', async () => {
// Arrange
const generatorAddress = getRandomBytes(20);
commitPool['_nonGossipedCommits'].add({
commitPool.addCommit(
{
blockID: getRandomBytes(32),
certificateSignature: getRandomBytes(96),
height: 1070,
validatorAddress: generatorAddress,
},
true,
);
// Added to nonGossipedCommitsLocal
expect(commitPool['_nonGossipedCommitsLocal'].getAll()).toHaveLength(1);
commitPool.addCommit({
blockID: getRandomBytes(32),
certificateSignature: getRandomBytes(96),
height: 1070,
validatorAddress: generatorAddress,
validatorAddress: getRandomBytes(20),
});
// Assert
expect(commitPool['_gossipedCommits'].getAll()).toHaveLength(6);
// Arrange
commitPool['_bftAPI'].existBFTParameters = jest.fn().mockResolvedValue(true);
(commitPool['_generatorAddress'] as any) = generatorAddress;
const context = createNewAPIContext(new InMemoryKVStore());
// Act
await commitPool['_job'](context);
// Assert
// nonGossiped commits are moved to gossiped commits
expect(commitPool['_nonGossipedCommits'].getAll()).toHaveLength(0);
expect(commitPool['_gossipedCommits'].getAll()).toHaveLength(11);
expect(commitPool['_gossipedCommits'].getAll()).toHaveLength(12);
expect(commitPool['_nonGossipedCommits'].getByHeight(1070)).toBeArrayOfSize(0);
const commits = commitPool['_gossipedCommits'].getByHeight(1070);
expect(commits).toBeDefined();
Expand Down Expand Up @@ -289,15 +300,15 @@ describe('CommitPool', () => {
}

const sortedNonGossipedCommits = cp['_nonGossipedCommits'].getAll(COMMIT_SORT.DSC);
const sortedNonGossipedCommitsLocal = cp['_nonGossipedCommitsLocal'].getAll(
COMMIT_SORT.DSC,
);

for (const [index, commit] of sortedNonGossipedCommits.entries()) {
for (const commit of sortedNonGossipedCommitsLocal) {
if (selectedCommits.length >= maxSelectedCommitsLength) {
break;
}
if (commit.validatorAddress.equals(cp['_generatorAddress'])) {
selectedCommits.push(commit);
sortedNonGossipedCommits.splice(index, 1);
}
selectedCommits.push(commit);
}
// 2.3 Select newly received commits by others
for (const commit of sortedNonGossipedCommits) {
Expand Down