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

Commit

Permalink
Remove unnecessary exceptions (#9108)
Browse files Browse the repository at this point in the history 
💅 Remove unnecessary exceptions
  • Loading branch information
@shuse2
shuse2 authored Oct 20, 2023
1 parent 4968383 commit 7c6f3a6
Show file tree
Hide file tree
Showing 13 changed files with 14 additions and 130 deletions.
11 changes: 1 addition & 10 deletions framework/src/engine/bft/method.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,14 @@ export interface BlockHeaderAsset {
export class BFTMethod {
private _batchSize!: number;
private _blockTime!: number;
private _shuffleValidatorsFromHeight!: number;

public blockTime(): number {
return this._blockTime;
}

public init(batchSize: number, blockTime: number, shuffleValidatorsFromHeight: number) {
public init(batchSize: number, blockTime: number) {
this._batchSize = batchSize;
this._blockTime = blockTime;
this._shuffleValidatorsFromHeight = shuffleValidatorsFromHeight;
}

public areHeadersContradicting(bftHeader1: BlockHeader, bftHeader2: BlockHeader): boolean {
Expand Down Expand Up @@ -175,7 +173,6 @@ export class BFTMethod {
precommitThreshold: bigint,
certificateThreshold: bigint,
validators: Validator[],
height: number,
): Promise<void> {
if (validators.length > this._batchSize) {
throw new Error(
Expand Down Expand Up @@ -230,12 +227,6 @@ export class BFTMethod {

const validatorsHash = computeValidatorsHash(validatorsWithBFTWeight, certificateThreshold);

// Ensure that validator list is not shuffled before the configured block height,
// to be able to sync with the new version
if (height < this._shuffleValidatorsFromHeight) {
sortValidatorsByBLSKey(validators);
}

const bftParams: BFTParameters = {
prevoteThreshold: (BigInt(2) * aggregateBFTWeight) / BigInt(3) + BigInt(1),
precommitThreshold,
Expand Down
8 changes: 2 additions & 6 deletions framework/src/engine/bft/module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,9 @@ export class BFTModule {
private _maxLengthBlockBFTInfos!: number;

// eslint-disable-next-line @typescript-eslint/require-await
public async init(
batchSize: number,
blockTime: number,
shuffleValidatorsFromHeight: number,
): Promise<void> {
public async init(batchSize: number, blockTime: number): Promise<void> {
this._batchSize = batchSize;
this.method.init(this._batchSize, blockTime, shuffleValidatorsFromHeight);
this.method.init(this._batchSize, blockTime);
this._maxLengthBlockBFTInfos = 3 * this._batchSize;
}

Expand Down
8 changes: 1 addition & 7 deletions framework/src/engine/consensus/consensus.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,7 @@ export class Consensus {
blockExecutor,
mechanisms: [blockSyncMechanism, fastChainSwitchMechanism],
});
await this._bft.init(
this._genesisConfig.bftBatchSize,
this._genesisConfig.blockTime,
this._genesisConfig.exceptions.shuffleValidatorsFromHeight,
);
await this._bft.init(this._genesisConfig.bftBatchSize, this._genesisConfig.blockTime);

this._network.registerEndpoint(NETWORK_LEGACY_GET_BLOCKS_FROM_ID, async ({ data, peerId }) =>
this._legacyEndpoint.handleRPCGetLegacyBlocksFromID(data, peerId),
Expand Down Expand Up @@ -1032,7 +1028,6 @@ export class Consensus {
afterResult.preCommitThreshold,
afterResult.certificateThreshold,
afterResult.nextValidators,
block.header.height,
);
this.events.emit(CONSENSUS_EVENT_VALIDATORS_CHANGED, {
preCommitThreshold: afterResult.preCommitThreshold,
Expand Down Expand Up @@ -1086,7 +1081,6 @@ export class Consensus {
result.preCommitThreshold,
result.certificateThreshold,
result.nextValidators,
genesisBlock.header.height,
);
this.events.emit(CONSENSUS_EVENT_VALIDATORS_CHANGED, {
preCommitThreshold: result.preCommitThreshold,
Expand Down
1 change: 0 additions & 1 deletion framework/src/engine/generator/generator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,6 @@ export class Generator {
afterResult.preCommitThreshold,
afterResult.certificateThreshold,
afterResult.nextValidators,
height,
);
}

Expand Down
14 changes: 0 additions & 14 deletions framework/src/schema/application_config_schema.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,6 @@ export const applicationConfigSchema = {
minimum: 1,
description: 'Minimum block height which can be certified',
},
exceptions: {
type: 'object',
required: ['shuffleValidatorsFromHeight'],
properties: {
shuffleValidatorsFromHeight: {
type: 'integer',
minimum: 0,
description: 'Block height from which the validator list will be shuffled',
},
},
},
},
additionalProperties: false,
},
Expand Down Expand Up @@ -362,9 +351,6 @@ export const applicationConfigSchema = {
bftBatchSize: BFT_BATCH_SIZE,
maxTransactionsSize: MAX_TRANSACTIONS_SIZE,
minimumCertifyHeight: 1,
exceptions: {
shuffleValidatorsFromHeight: 0,
},
},
generator: {
keys: {},
Expand Down
3 changes: 0 additions & 3 deletions framework/src/testing/fixtures/config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ export const defaultConfig: ApplicationConfig = {
blockTime: 10,
chainID: '10000000',
maxTransactionsSize: 15 * 1024, // Kilo Bytes
exceptions: {
shuffleValidatorsFromHeight: 0,
},
},
network: {
version: '1.0',
Expand Down
1 change: 0 additions & 1 deletion framework/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ export interface GenesisConfig {
blockTime: number;
bftBatchSize: number;
minimumCertifyHeight: number;
exceptions: { shuffleValidatorsFromHeight: number };
}

export interface TransactionPoolConfig {
Expand Down
3 changes: 0 additions & 3 deletions framework/test/unit/__snapshots__/application.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ exports[`Application #constructor should set internal variables 1`] = `
},
"blockTime": 10,
"chainID": "10000000",
"exceptions": {
"shuffleValidatorsFromHeight": 0,
},
"maxTransactionsSize": 15360,
"minimumCertifyHeight": 1,
},
Expand Down
7 changes: 1 addition & 6 deletions framework/test/unit/engine/bft/bft_processing.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ describe('BFT processing', () => {
scenario11ValidatorsPartialSwitch,
];
const blockTime = 10;
const shuffleValidatorsFromHeight = 0;

for (const scenario of bftScenarios) {
// eslint-disable-next-line no-loop-func
Expand All @@ -50,11 +49,7 @@ describe('BFT processing', () => {

beforeAll(async () => {
bftModule = new BFTModule();
await bftModule.init(
scenario.config.activeValidators,
blockTime,
shuffleValidatorsFromHeight,
);
await bftModule.init(scenario.config.activeValidators, blockTime);
db = new InMemoryDatabase();
stateStore = new StateStore(db);

Expand Down
67 changes: 8 additions & 59 deletions framework/test/unit/engine/bft/method.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
} from '../../../../src/engine/bft/schemas';
import { createFakeBlockHeader } from '../../../../src/testing';
import { computeValidatorsHash } from '../../../../src/engine';
import { sortValidatorsByBLSKey } from '../../../../src/engine/bft/utils';

describe('BFT Method', () => {
let bftMethod: BFTMethod;
Expand All @@ -44,7 +43,7 @@ describe('BFT Method', () => {
beforeEach(() => {
bftMethod = new BFTMethod();
validatorsMethod = { getValidatorKeys: jest.fn() };
bftMethod.init(103, 10, 0);
bftMethod.init(103, 10);
});

describe('areHeadersContradicting', () => {
Expand Down Expand Up @@ -708,7 +707,6 @@ describe('BFT Method', () => {
const generatorAddress = utils.getRandomBytes(20);
const params20 = createParam();
const params30 = createParam();
const height = 8;

const validators = [
{
Expand Down Expand Up @@ -805,7 +803,6 @@ describe('BFT Method', () => {
blsKey: utils.getRandomBytes(48),
generatorKey: utils.getRandomBytes(32),
})),
height,
),
).rejects.toThrow('Invalid validators size.');
});
Expand All @@ -820,13 +817,7 @@ describe('BFT Method', () => {
validatorsAddressNotUnique[8].address = validatorsAddressNotUnique[12].address;

await expect(
bftMethod.setBFTParameters(
stateStore,
BigInt(68),
BigInt(68),
validatorsAddressNotUnique,
height,
),
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsAddressNotUnique),
).rejects.toThrow('Provided validator addresses are not unique.');
});

Expand All @@ -840,13 +831,7 @@ describe('BFT Method', () => {
validatorsBLSKeysNotUnique[13].blsKey = validatorsBLSKeysNotUnique[7].blsKey;

await expect(
bftMethod.setBFTParameters(
stateStore,
BigInt(68),
BigInt(68),
validatorsBLSKeysNotUnique,
height,
),
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsBLSKeysNotUnique),
).rejects.toThrow('Provided validator BLS keys are not unique.');
});

Expand All @@ -861,13 +846,7 @@ describe('BFT Method', () => {
validatorsInvalidBLSKeys[13].blsKey = Buffer.alloc(48, 0);

await expect(
bftMethod.setBFTParameters(
stateStore,
BigInt(68),
BigInt(68),
validatorsInvalidBLSKeys,
height,
),
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(68), validatorsInvalidBLSKeys),
).not.toReject();
});

Expand All @@ -883,32 +862,31 @@ describe('BFT Method', () => {
blsKey: utils.getRandomBytes(48),
generatorKey: utils.getRandomBytes(32),
})),
height,
),
).rejects.toThrow('BFT Weight must be 0 or greater.');
});

it('should throw when less than 1/3 of aggregateBFTWeight for precommitThreshold is given', async () => {
await expect(
bftMethod.setBFTParameters(stateStore, BigInt(34), BigInt(68), validators, height),
bftMethod.setBFTParameters(stateStore, BigInt(34), BigInt(68), validators),
).rejects.toThrow('Invalid precommitThreshold input.');
});

it('should throw when precommitThreshold is given is greater than aggregateBFTWeight', async () => {
await expect(
bftMethod.setBFTParameters(stateStore, BigInt(104), BigInt(68), validators, height),
bftMethod.setBFTParameters(stateStore, BigInt(104), BigInt(68), validators),
).rejects.toThrow('Invalid precommitThreshold input.');
});

it('should throw when less than 1/3 of aggregateBFTWeight for certificateThreshold is given', async () => {
await expect(
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(34), validators, height),
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(34), validators),
).rejects.toThrow('Invalid certificateThreshold input.');
});

it('should throw when certificateThreshold is given is greater than aggregateBFTWeight', async () => {
await expect(
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(104), validators, height),
bftMethod.setBFTParameters(stateStore, BigInt(68), BigInt(104), validators),
).rejects.toThrow('Invalid certificateThreshold input.');
});

Expand All @@ -932,7 +910,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

const bftParams = await bftParamsStore.getWithSchema<BFTParameters>(
Expand All @@ -944,29 +921,6 @@ describe('BFT Method', () => {
expect(bftParams.validators).toEqual(shuffledValidators);
});

it('should store validators in order of BLS keys if block height is earlier than the configured exception', async () => {
const sortedValidators = [...validators];
sortValidatorsByBLSKey(sortedValidators);

bftMethod.init(103, 10, 88);

await bftMethod.setBFTParameters(
stateStore,
precommitThreshold,
certificateThreshold,
validators,
height,
);

const bftParams = await bftParamsStore.getWithSchema<BFTParameters>(
utils.intToBuffer(104, 4),
bftParametersSchema,
);

expect(bftParams.validators).toHaveLength(3);
expect(bftParams.validators).toEqual(sortedValidators);
});

it('should store BFT parameters with height maxHeightPrevoted + 1 if blockBFTInfo does not exist', async () => {
await votesStore.setWithSchema(
EMPTY_KEY,
Expand All @@ -991,7 +945,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

await expect(
Expand All @@ -1008,7 +961,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

await expect(
Expand All @@ -1025,7 +977,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

const bftParams = await bftParamsStore.getWithSchema<BFTParameters>(
Expand All @@ -1041,7 +992,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

const voteState = await votesStore.getWithSchema<BFTVotes>(EMPTY_KEY, bftVotesSchema);
Expand All @@ -1060,7 +1010,6 @@ describe('BFT Method', () => {
precommitThreshold,
certificateThreshold,
validators,
height,
);

const voteState = await votesStore.getWithSchema<BFTVotes>(EMPTY_KEY, bftVotesSchema);
Expand Down
2 changes: 1 addition & 1 deletion framework/test/unit/engine/bft/module.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('bft module', () => {

describe('init', () => {
it('should initialize config with given value', async () => {
await expect(bftModule.init(20, 10, 0)).toResolve();
await expect(bftModule.init(20, 10)).toResolve();

expect(bftModule['_batchSize']).toBe(20);
});
Expand Down
3 changes: 0 additions & 3 deletions framework/test/unit/engine/consensus/consensus.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,6 @@ describe('consensus', () => {
bft,
genesisConfig: {
blockTime: 10,
exceptions: {
shuffleValidatorsFromHeight: 0,
},
} as any,
});
dbMock = {
Expand Down
Loading

0 comments on commit 7c6f3a6

Please sign in to comment.