diff --git a/framework/src/modules/validators/method.ts b/framework/src/modules/validators/method.ts index cf27cad65ea..0fa0cc242c0 100644 --- a/framework/src/modules/validators/method.ts +++ b/framework/src/modules/validators/method.ts @@ -17,6 +17,7 @@ import { BaseMethod } from '../base_method'; import { MethodContext, ImmutableMethodContext } from '../../state_machine'; import { ADDRESS_LENGTH, + BLS_POP_LENGTH, BLS_PUBLIC_KEY_LENGTH, ED25519_PUBLIC_KEY_LENGTH, EMPTY_KEY, @@ -31,6 +32,13 @@ import { BlsKeyRegistrationEvent } from './events/bls_key_registration'; import { ValidatorsParams, ValidatorsParamsStore } from './stores/validators_params'; import { NextValidatorsSetter, Validator } from '../../state_machine/types'; +export type ValidatorArgs = { + validatorAddress?: Buffer; + blsKey?: Buffer; + proofOfPossession?: Buffer; + generatorKey?: Buffer; +}; + export class ValidatorsMethod extends BaseMethod { private _blockTime!: number; @@ -45,9 +53,8 @@ export class ValidatorsMethod extends BaseMethod { generatorKey: Buffer, proofOfPossession: Buffer, ): Promise { - if (validatorAddress.length !== ADDRESS_LENGTH) { - throw new Error(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); - } + this._validateLengths({ validatorAddress, blsKey, proofOfPossession }); + const validatorAccount = { generatorKey, blsKey, @@ -103,9 +110,8 @@ export class ValidatorsMethod extends BaseMethod { validatorAddress: Buffer, generatorKey: Buffer, ): Promise { - if (validatorAddress.length !== ADDRESS_LENGTH) { - throw new Error(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); - } + this._validateLengths({ validatorAddress }); + const validatorAccount = { generatorKey, blsKey: INVALID_BLS_KEY, @@ -137,9 +143,7 @@ export class ValidatorsMethod extends BaseMethod { methodContext: MethodContext, blsKey: Buffer, ): Promise { - if (blsKey.length !== BLS_PUBLIC_KEY_LENGTH) { - throw new Error(`BLS public key must be ${BLS_PUBLIC_KEY_LENGTH} bytes long.`); - } + this._validateLengths({ blsKey }); const blsKeysSubStore = this.stores.get(BLSKeyStore); const blsKeyExists = await blsKeysSubStore.has(methodContext, blsKey); @@ -157,9 +161,7 @@ export class ValidatorsMethod extends BaseMethod { methodContext: ImmutableMethodContext, address: Buffer, ): Promise { - if (address.length !== ADDRESS_LENGTH) { - throw new Error(`Validator address length must be ${ADDRESS_LENGTH}.`); - } + this._validateLengths({ validatorAddress: address }); const validatorsSubStore = this.stores.get(ValidatorKeysStore); const addressExists = await validatorsSubStore.has(methodContext, address); @@ -176,12 +178,7 @@ export class ValidatorsMethod extends BaseMethod { blsKey: Buffer, proofOfPossession: Buffer, ): Promise { - if (validatorAddress.length !== ADDRESS_LENGTH) { - throw new Error(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); - } - if (blsKey.length !== BLS_PUBLIC_KEY_LENGTH) { - throw new Error(`BLS public key must be ${BLS_PUBLIC_KEY_LENGTH} bytes long.`); - } + this._validateLengths({ validatorAddress, blsKey, proofOfPossession }); const validatorsSubStore = this.stores.get(ValidatorKeysStore); const addressExists = await validatorsSubStore.has(methodContext, validatorAddress); @@ -244,12 +241,7 @@ export class ValidatorsMethod extends BaseMethod { validatorAddress: Buffer, generatorKey: Buffer, ): Promise { - if (validatorAddress.length !== ADDRESS_LENGTH) { - throw new Error(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); - } - if (generatorKey.length !== ED25519_PUBLIC_KEY_LENGTH) { - throw new Error(`Generator key must be ${ED25519_PUBLIC_KEY_LENGTH} bytes long.`); - } + this._validateLengths({ validatorAddress, generatorKey }); const validatorsSubStore = this.stores.get(ValidatorKeysStore); @@ -362,4 +354,19 @@ export class ValidatorsMethod extends BaseMethod { }); validatorSetter.setNextValidators(preCommitThreshold, certificateThreshold, validatorsWithKey); } + + private _validateLengths(args: ValidatorArgs) { + if (args.validatorAddress && args.validatorAddress.length !== ADDRESS_LENGTH) { + throw new Error(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); + } + if (args.blsKey && args.blsKey.length !== BLS_PUBLIC_KEY_LENGTH) { + throw new Error(`BLS public key must be ${BLS_PUBLIC_KEY_LENGTH} bytes long.`); + } + if (args.proofOfPossession && args.proofOfPossession.length !== BLS_POP_LENGTH) { + throw new Error(`Proof of possesion must be ${BLS_POP_LENGTH} bytes long.`); + } + if (args.generatorKey && args.generatorKey.length !== ED25519_PUBLIC_KEY_LENGTH) { + throw new Error(`Generator key must be ${ED25519_PUBLIC_KEY_LENGTH} bytes long.`); + } + } } diff --git a/framework/src/modules/validators/schemas.ts b/framework/src/modules/validators/schemas.ts index d8f1fde53db..4060b60a19d 100644 --- a/framework/src/modules/validators/schemas.ts +++ b/framework/src/modules/validators/schemas.ts @@ -12,6 +12,8 @@ * Removal or modification of this copyright notice is prohibited. */ +import { BLS_POP_LENGTH, BLS_PUBLIC_KEY_LENGTH } from './constants'; + export interface ValidateBLSKeyRequest { proofOfPossession: string; blsKey: string; @@ -25,10 +27,14 @@ export const validateBLSKeyRequestSchema = { proofOfPossession: { type: 'string', format: 'hex', + minLength: BLS_POP_LENGTH * 2, + maxLength: BLS_POP_LENGTH * 2, }, blsKey: { type: 'string', format: 'hex', + minLength: BLS_PUBLIC_KEY_LENGTH * 2, + maxLength: BLS_PUBLIC_KEY_LENGTH * 2, }, }, required: ['proofOfPossession', 'blsKey'], diff --git a/framework/test/unit/modules/validators/endpoint.spec.ts b/framework/test/unit/modules/validators/endpoint.spec.ts index 3fbb6221efd..8b933bb67d9 100644 --- a/framework/test/unit/modules/validators/endpoint.spec.ts +++ b/framework/test/unit/modules/validators/endpoint.spec.ts @@ -21,16 +21,20 @@ import { PrefixedStateReadWriter } from '../../../../src/state_machine/prefixed_ import { createTransientModuleEndpointContext } from '../../../../src/testing'; import { InMemoryPrefixedStateDB } from '../../../../src/testing/in_memory_prefixed_state'; import { createStoreGetter } from '../../../../src/testing/utils'; +import { + ADDRESS_LENGTH, + BLS_POP_LENGTH, + BLS_PUBLIC_KEY_LENGTH, + ED25519_PUBLIC_KEY_LENGTH, +} from '../../../../src/modules/validators/constants'; describe('ValidatorsModuleEndpoint', () => { let validatorsModule: ValidatorsModule; let stateStore: PrefixedStateReadWriter; - const pk = utils.getRandomBytes(48); - const address = utils.getRandomBytes(48); - const proof = utils.getRandomBytes(96); - const validatorAddress = utils.getRandomBytes(20); - const blsKey = utils.getRandomBytes(48); - const generatorKey = utils.getRandomBytes(32); + const proof = utils.getRandomBytes(BLS_POP_LENGTH); + const validatorAddress = utils.getRandomBytes(ADDRESS_LENGTH); + const blsKey = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH); + const generatorKey = utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH); const validProof = '88bb31b27eae23038e14f9d9d1b628a39f5881b5278c3c6f0249f81ba0deb1f68aa5f8847854d6554051aa810fdf1cdb02df4af7a5647b1aa4afb60ec6d446ee17af24a8a50876ffdaf9bf475038ec5f8ebeda1c1c6a3220293e23b13a9a5d26'; @@ -46,13 +50,13 @@ describe('ValidatorsModuleEndpoint', () => { stateStore, params: { proofOfPossession: proof.toString('hex'), - blsKey: pk.toString('hex'), + blsKey: blsKey.toString('hex'), }, }); - await validatorsModule.stores - .get(BLSKeyStore) - .set(createStoreGetter(stateStore), pk, { address }); + await validatorsModule.stores.get(BLSKeyStore).set(createStoreGetter(stateStore), blsKey, { + address: utils.getRandomBytes(ADDRESS_LENGTH), + }); await expect(validatorsModule.endpoint.validateBLSKey(context)).resolves.toStrictEqual({ valid: false, @@ -64,7 +68,7 @@ describe('ValidatorsModuleEndpoint', () => { stateStore, params: { proofOfPossession: proof.toString('hex'), - blsKey: pk.toString('hex'), + blsKey: blsKey.toString('hex'), }, }); await expect(validatorsModule.endpoint.validateBLSKey(context)).resolves.toStrictEqual({ @@ -87,7 +91,7 @@ describe('ValidatorsModuleEndpoint', () => { }); it('should resolve with false when proof of possession is invalid but bls key has a valid length', async () => { - const validPk = + const validBLSKey = 'a491d1b0ecd9bb917989f0e74f0dea0422eac4a873e5e2644f368dffb9a6e20fd6e10c1b77654d067c0618f6e5a7f79a'; const invalidProof = 'b803eb0ed93ea10224a73b6b9c725796be9f5fefd215ef7a5b97234cc956cf6870db6127b7e4d824ec62276078e787db05584ce1adbf076bc0808ca0f15b73d59060254b25393d95dfc7abe3cda566842aaedf50bbb062aae1bbb6ef3b1fffff'; @@ -95,7 +99,7 @@ describe('ValidatorsModuleEndpoint', () => { stateStore, params: { proofOfPossession: invalidProof, - blsKey: validPk, + blsKey: validBLSKey, }, }); await expect(validatorsModule.endpoint.validateBLSKey(context)).resolves.toStrictEqual({ @@ -103,32 +107,32 @@ describe('ValidatorsModuleEndpoint', () => { }); }); - it('should resolve with false when bls key length is less than 48 bytes and proof of possession has valid length', async () => { - const invalidPk = utils.getRandomBytes(47).toString('hex'); + it('should throw when BLS key length is too short and proof of possession has valid length', async () => { + const shortBLSKey = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH - 1).toString('hex'); const context = createTransientModuleEndpointContext({ stateStore, params: { proofOfPossession: validProof, - blsKey: invalidPk, + blsKey: shortBLSKey, }, }); - await expect(validatorsModule.endpoint.validateBLSKey(context)).resolves.toStrictEqual({ - valid: false, - }); + await expect(validatorsModule.endpoint.validateBLSKey(context)).rejects.toThrow( + `Property '.blsKey' must NOT have fewer than ${BLS_PUBLIC_KEY_LENGTH * 2} characters`, + ); }); - it('should resolve with false when bls key length is greater than 48 bytes and proof of possession has valid length', async () => { - const invalidPk = utils.getRandomBytes(49).toString('hex'); + it('should throw when BLS key length is too long and proof of possession has valid length', async () => { + const longBLSKey = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH + 1).toString('hex'); const context = createTransientModuleEndpointContext({ stateStore, params: { proofOfPossession: validProof, - blsKey: invalidPk, + blsKey: longBLSKey, }, }); - await expect(validatorsModule.endpoint.validateBLSKey(context)).resolves.toStrictEqual({ - valid: false, - }); + await expect(validatorsModule.endpoint.validateBLSKey(context)).rejects.toThrow( + `Property '.blsKey' must NOT have more than ${BLS_PUBLIC_KEY_LENGTH * 2} characters`, + ); }); }); diff --git a/framework/test/unit/modules/validators/method.spec.ts b/framework/test/unit/modules/validators/method.spec.ts index fac6118abb1..0c34961381f 100644 --- a/framework/test/unit/modules/validators/method.spec.ts +++ b/framework/test/unit/modules/validators/method.spec.ts @@ -23,6 +23,7 @@ import { ADDRESS_LENGTH, BLS_PUBLIC_KEY_LENGTH, ED25519_PUBLIC_KEY_LENGTH, + BLS_POP_LENGTH, } from '../../../../src/modules/validators/constants'; import * as generatorList from '../../../fixtures/config/devnet/validators_for_first_round.json'; import { @@ -46,6 +47,7 @@ import { BlsKeyRegistrationEvent, } from '../../../../src/modules/validators/events/bls_key_registration'; import { ValidatorsParamsStore } from '../../../../src/modules/validators/stores/validators_params'; +import { ValidatorArgs } from '../../../../src/modules/validators/method'; describe('ValidatorsModuleMethod', () => { let validatorsMethod: ValidatorsMethod; @@ -58,8 +60,8 @@ describe('ValidatorsModuleMethod', () => { const blockTime = 10; const genesisConfig: any = { blockTime }; const moduleConfig: any = {}; - const address = utils.getRandomBytes(20); - const generatorKey = utils.getRandomBytes(32); + const address = utils.getRandomBytes(ADDRESS_LENGTH); + const generatorKey = utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH); const proofOfPossession = Buffer.from( '88bb31b27eae23038e14f9d9d1b628a39f5881b5278c3c6f0249f81ba0deb1f68aa5f8847854d6554051aa810fdf1cdb02df4af7a5647b1aa4afb60ec6d446ee17af24a8a50876ffdaf9bf475038ec5f8ebeda1c1c6a3220293e23b13a9a5d26', 'hex', @@ -69,15 +71,15 @@ describe('ValidatorsModuleMethod', () => { 'hex', ); const invalidProofOfPossession = Buffer.from( - '0x88bb31b27eae23038e14f9d9d1b628a39f5881b5278c3c6f0249f81ba0deb1f68aa5f8847854d6554051aa810fdf1cdb02df4af7a5647b1aa4afb60ec6d446ee17af24a8a50876ffdaf9bf475038ec5f8ebeda1c1c6a3220293e23b13a9a5d26', + '88bb31b27eae23038e14f9d9d1b628a39f5881b5278c3c6f0249f81ba0deb1f68aa5f8847854d6554051aa810fdf1cdb02df4af7a5647b1aa4afb60ec6d446ee17af24a8a50876ffdaf9bf475038ec5f8ebeda1c1c6a3220293e23b13a9a5d27', 'hex', ); - const invalidAddressShort = utils.getRandomBytes(19); - const invalidAddressLong = utils.getRandomBytes(21); - const invalidGeneratorKeyShort = utils.getRandomBytes(31); - const invalidGeneratorKeyLong = utils.getRandomBytes(33); - const invalidBlsKeyShort = utils.getRandomBytes(47); - const invalidBlsKeyLong = utils.getRandomBytes(49); + const invalidAddressShort = utils.getRandomBytes(ADDRESS_LENGTH - 1); + const invalidAddressLong = utils.getRandomBytes(ADDRESS_LENGTH + 1); + const invalidGeneratorKeyShort = utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH - 1); + const invalidGeneratorKeyLong = utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH + 1); + const invalidBlsKeyShort = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH - 1); + const invalidBlsKeyLong = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH + 1); beforeAll(async () => { validatorsModule = new ValidatorsModule(); @@ -607,8 +609,8 @@ describe('ValidatorsModuleMethod', () => { validators: generatorList.map(addr => ({ address: Buffer.from(addr, 'hex'), bftWeight: BigInt(1), - blsKey: Buffer.alloc(98), - generatorKey: Buffer.alloc(32), + blsKey: Buffer.alloc(BLS_PUBLIC_KEY_LENGTH), + generatorKey: Buffer.alloc(ED25519_PUBLIC_KEY_LENGTH), })), }); }); @@ -620,8 +622,8 @@ describe('ValidatorsModuleMethod', () => { validators: generatorList.map(addr => ({ address: Buffer.from(addr, 'hex'), bftWeight: BigInt(1), - blsKey: Buffer.alloc(98), - generatorKey: Buffer.alloc(32), + blsKey: Buffer.alloc(BLS_PUBLIC_KEY_LENGTH), + generatorKey: Buffer.alloc(ED25519_PUBLIC_KEY_LENGTH), })), }); @@ -733,8 +735,8 @@ describe('ValidatorsModuleMethod', () => { methodContext = createNewMethodContext(new InMemoryPrefixedStateDB()); validatorAccount = { - generatorKey: utils.getRandomBytes(48), - blsKey: utils.getRandomBytes(32), + generatorKey: utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH), + blsKey: utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH), }; const validatorsStore = validatorsModule.stores.get(ValidatorKeysStore); @@ -750,14 +752,13 @@ describe('ValidatorsModuleMethod', () => { }); it(`should throw error when address length is not ${ADDRESS_LENGTH}`, async () => { - const invalidAddress = utils.getRandomBytes(19); await expect( - validatorsMethod.getValidatorKeys(methodContext, invalidAddress), - ).rejects.toThrow(`Validator address length must be ${ADDRESS_LENGTH}.`); + validatorsMethod.getValidatorKeys(methodContext, invalidAddressShort), + ).rejects.toThrow(`Validator address must be ${ADDRESS_LENGTH} bytes long.`); }); it('should throw if address does not exist', async () => { - const nonExistingAddress = utils.getRandomBytes(20); + const nonExistingAddress = utils.getRandomBytes(ADDRESS_LENGTH); await expect( validatorsMethod.getValidatorKeys(methodContext, nonExistingAddress), ).rejects.toThrow('No validator account found for the input address.'); @@ -891,4 +892,45 @@ describe('ValidatorsModuleMethod', () => { await expect(validatorsSubStore.get(methodContext, address)).rejects.toThrow(); }); }); + + describe('_validateLengths', () => { + let validatorArgs: ValidatorArgs; + + beforeEach(() => { + validatorArgs = { + validatorAddress: utils.getRandomBytes(ADDRESS_LENGTH), + blsKey: utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH), + proofOfPossession: utils.getRandomBytes(BLS_POP_LENGTH), + generatorKey: utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH), + }; + }); + + it('should NOT throw when all properties are present and have correct length', () => { + expect(() => validatorsModule.method['_validateLengths'](validatorArgs)).not.toThrow(); + }); + + it('should NOT throw when some properties are missing, but all have correct length', () => { + delete validatorArgs.blsKey; + delete validatorArgs.generatorKey; + + expect(() => validatorsModule.method['_validateLengths'](validatorArgs)).not.toThrow(); + }); + + it('should throw when all properties are present, but 1 of them have incorrect length', () => { + validatorArgs.blsKey = utils.getRandomBytes(BLS_PUBLIC_KEY_LENGTH + 1); + + expect(() => validatorsModule.method['_validateLengths'](validatorArgs)).toThrow( + `BLS public key must be ${BLS_PUBLIC_KEY_LENGTH} bytes long.`, + ); + }); + + it('should throw when 1 property are missing, and 1 of the existing ones have incorrect length', () => { + delete validatorArgs.blsKey; + validatorArgs.generatorKey = utils.getRandomBytes(ED25519_PUBLIC_KEY_LENGTH - 1); + + expect(() => validatorsModule.method['_validateLengths'](validatorArgs)).toThrow( + `Generator key must be ${ED25519_PUBLIC_KEY_LENGTH} bytes long.`, + ); + }); + }); });