diff --git a/framework/src/modules/interoperability/base_interoperability_internal_methods.ts b/framework/src/modules/interoperability/base_interoperability_internal_methods.ts index a5cdb9fca63..76224c78dd2 100644 --- a/framework/src/modules/interoperability/base_interoperability_internal_methods.ts +++ b/framework/src/modules/interoperability/base_interoperability_internal_methods.ts @@ -26,6 +26,7 @@ import { MODULE_NAME_INTEROPERABILITY, EMPTY_HASH, MAX_NUM_VALIDATORS, + MAX_UINT64, } from './constants'; import { ccmSchema } from './schemas'; import { CCMsg, CrossChainUpdateTransactionParams, ChainAccount, ChainValidators } from './types'; @@ -369,9 +370,28 @@ export abstract class BaseInteroperabilityInternalMethod extends BaseInternalMet ); } + let totalWeight = BigInt(0); + for (const currentValidator of newActiveValidators) { + if (currentValidator.bftWeight === BigInt(0)) { + throw new Error('Validator bft weight must be positive integer.'); + } + totalWeight += currentValidator.bftWeight; + if (totalWeight > MAX_UINT64) { + throw new Error('Total BFT weight exceeds maximum value.'); + } + } const certificate = codec.decode(certificateSchema, ccu.certificate); validator.validate(certificateSchema, certificate); + const { certificateThreshold } = ccu; + + if (certificateThreshold < totalWeight / BigInt(3) + BigInt(1)) { + throw new Error('Certificate threshold is too small.'); + } + if (certificateThreshold > totalWeight) { + throw new Error('Certificate threshold is too large.'); + } + const newValidatorsHash = computeValidatorsHash(newActiveValidators, ccu.certificateThreshold); if (!certificate.validatorsHash.equals(newValidatorsHash)) { throw new Error('ValidatorsHash in certificate and the computed values do not match.'); diff --git a/framework/src/modules/interoperability/certificates.ts b/framework/src/modules/interoperability/certificates.ts index 8bab338cb17..784970b96e9 100644 --- a/framework/src/modules/interoperability/certificates.ts +++ b/framework/src/modules/interoperability/certificates.ts @@ -1,6 +1,6 @@ /** * TODO: Now that we have `certificates.ts`, these methods could be moved there - * (checkCertificateTimestamp, checkCertificateValidity, checkValidatorsHashWithCertificate, + * (checkCertificateTimestamp, * isCertificateEmpty, verifyCertificateSignature) */ import { LastCertificate, LastCertificateJSON } from './types'; diff --git a/framework/src/modules/interoperability/sidechain/commands/register_mainchain.ts b/framework/src/modules/interoperability/sidechain/commands/register_mainchain.ts index 2e69ad3cd61..b924bcf9150 100644 --- a/framework/src/modules/interoperability/sidechain/commands/register_mainchain.ts +++ b/framework/src/modules/interoperability/sidechain/commands/register_mainchain.ts @@ -112,7 +112,7 @@ export class RegisterMainchainCommand extends BaseInteroperabilityCommand export const isOutboxRootWitnessEmpty = (outboxRootWitness: OutboxRootWitness) => outboxRootWitness.siblingHashes.length === 0 || outboxRootWitness.bitmap.length === 0; -export const checkLivenessRequirementFirstCCU = ( +export const checkLivenessRequirement = ( partnerChainAccount: ChainAccount, txParams: CrossChainUpdateTransactionParams, ): VerificationResult => { @@ -142,39 +141,6 @@ export const checkLivenessRequirementFirstCCU = ( }; }; -export const checkCertificateValidity = ( - partnerChainAccount: ChainAccount, - encodedCertificate: Buffer, -): VerificationResult => { - if (encodedCertificate.equals(EMPTY_BYTES)) { - return { - status: VerifyStatus.OK, - }; - } - - const certificate = codec.decode(certificateSchema, encodedCertificate); - try { - validator.validate(certificateSchema, certificate); - } catch (err) { - return { - status: VerifyStatus.FAIL, - error: new Error('Certificate is missing required values.'), - }; - } - - // Last certificate height should be less than new certificate height - if (partnerChainAccount.lastCertificate.height >= certificate.height) { - return { - status: VerifyStatus.FAIL, - error: new Error('Certificate height should be greater than last certificate height.'), - }; - } - - return { - status: VerifyStatus.OK, - }; -}; - export const checkCertificateTimestamp = ( txParams: CrossChainUpdateTransactionParams, certificate: Certificate, @@ -190,60 +156,6 @@ export const checkCertificateTimestamp = ( } }; -export const checkValidatorsHashWithCertificate = ( - txParams: CrossChainUpdateTransactionParams, - partnerValidators: ChainValidators, -): VerificationResult => { - if ( - !emptyActiveValidatorsUpdate(txParams.activeValidatorsUpdate) || - txParams.certificateThreshold > BigInt(0) - ) { - if (txParams.certificate.equals(EMPTY_BYTES)) { - return { - status: VerifyStatus.FAIL, - error: new Error( - 'Certificate cannot be empty when activeValidatorsUpdate or certificateThreshold has a non-empty value.', - ), - }; - } - let certificate: Certificate; - try { - certificate = codec.decode(certificateSchema, txParams.certificate); - validator.validate(certificateSchema, certificate); - } catch (error) { - return { - status: VerifyStatus.FAIL, - error: new Error( - 'Certificate should have all required values when activeValidatorsUpdate or certificateThreshold has a non-empty value.', - ), - }; - } - - const newActiveValidators = calculateNewActiveValidators( - partnerValidators.activeValidators, - txParams.activeValidatorsUpdate.blsKeysUpdate, - txParams.activeValidatorsUpdate.bftWeightsUpdate, - txParams.activeValidatorsUpdate.bftWeightsUpdateBitmap, - ); - - const validatorsHash = computeValidatorsHash( - newActiveValidators, - txParams.certificateThreshold || partnerValidators.certificateThreshold, - ); - - if (!certificate.validatorsHash.equals(validatorsHash)) { - return { - status: VerifyStatus.FAIL, - error: new Error('Validators hash given in the certificate is incorrect.'), - }; - } - } - - return { - status: VerifyStatus.OK, - }; -}; - export const chainAccountToJSON = (chainAccount: ChainAccount) => { const { lastCertificate, name, status } = chainAccount; diff --git a/framework/test/unit/modules/interoperability/internal_method.spec.ts b/framework/test/unit/modules/interoperability/internal_method.spec.ts index 6899c5b0f6b..cf673986898 100644 --- a/framework/test/unit/modules/interoperability/internal_method.spec.ts +++ b/framework/test/unit/modules/interoperability/internal_method.spec.ts @@ -24,6 +24,7 @@ import { EMPTY_BYTES, EMPTY_HASH, HASH_LENGTH, + MAX_UINT64, MESSAGE_TAG_CERTIFICATE, MIN_RETURN_FEE_PER_BYTE_BEDDOWS, } from '../../../../src/modules/interoperability/constants'; @@ -149,7 +150,7 @@ describe('Base interoperability internal method', () => { siblingHashes: [], }, }, - certificateThreshold: BigInt(99), + certificateThreshold: BigInt(9), sendingChainID: cryptoUtils.getRandomBytes(4), }; let mainchainInteroperabilityInternalMethod: MainchainInteroperabilityInternalMethod; @@ -926,6 +927,134 @@ describe('Base interoperability internal method', () => { ).rejects.toThrow('New validators must have a positive BFT weight.'); }); + it('should reject if new active validator bft weight equals 0', async () => { + const ccu = { + ...ccuParams, + certificate: codec.encode(certificateSchema, defaultCertificate), + activeValidatorsUpdate: { + blsKeysUpdate: [ + Buffer.from([0, 0, 0, 0]), + Buffer.from([0, 0, 0, 1]), + Buffer.from([0, 0, 3, 0]), + ], + bftWeightsUpdate: [BigInt(1), BigInt(3), BigInt(4)], + // 7 corresponds to 0111 + bftWeightsUpdateBitmap: Buffer.from([7]), + }, + }; + const existingKey = Buffer.from([0, 2, 3, 0]); + await chainValidatorsSubstore.set(methodContext, ccu.sendingChainID, { + activeValidators: [{ blsKey: existingKey, bftWeight: BigInt(2) }], + certificateThreshold: BigInt(1), + }); + const newValidators = [ + { blsKey: Buffer.from([0, 0, 0, 0]), bftWeight: BigInt(1) }, + { blsKey: Buffer.from([0, 0, 0, 1]), bftWeight: BigInt(3) }, + { blsKey: Buffer.from([0, 0, 2, 0]), bftWeight: BigInt(0) }, + ]; + jest.spyOn(utils, 'calculateNewActiveValidators').mockReturnValue(newValidators); + + await expect( + mainchainInteroperabilityInternalMethod.verifyValidatorsUpdate(methodContext, ccu), + ).rejects.toThrow('Validator bft weight must be positive integer.'); + }); + + it(`should reject if total bft weight > ${MAX_UINT64}`, async () => { + const ccu = { + ...ccuParams, + certificate: codec.encode(certificateSchema, defaultCertificate), + activeValidatorsUpdate: { + blsKeysUpdate: [ + Buffer.from([0, 0, 0, 0]), + Buffer.from([0, 0, 0, 1]), + Buffer.from([0, 0, 3, 0]), + ], + bftWeightsUpdate: [BigInt(1), BigInt(3), BigInt(4)], + // 7 corresponds to 0111 + bftWeightsUpdateBitmap: Buffer.from([7]), + }, + }; + const existingKey = Buffer.from([0, 2, 3, 0]); + await chainValidatorsSubstore.set(methodContext, ccu.sendingChainID, { + activeValidators: [{ blsKey: existingKey, bftWeight: BigInt(2) }], + certificateThreshold: BigInt(1), + }); + const newValidators = [ + { blsKey: Buffer.from([0, 0, 0, 0]), bftWeight: BigInt(1) }, + { blsKey: Buffer.from([0, 0, 0, 1]), bftWeight: BigInt(3) }, + { blsKey: Buffer.from([0, 0, 2, 0]), bftWeight: MAX_UINT64 }, + ]; + jest.spyOn(utils, 'calculateNewActiveValidators').mockReturnValue(newValidators); + + await expect( + mainchainInteroperabilityInternalMethod.verifyValidatorsUpdate(methodContext, ccu), + ).rejects.toThrow('Total BFT weight exceeds maximum value.'); + }); + + it('should reject if certificate threshold is too small', async () => { + const ccu = { + ...ccuParams, + certificate: codec.encode(certificateSchema, defaultCertificate), + activeValidatorsUpdate: { + blsKeysUpdate: [ + Buffer.from([0, 0, 0, 0]), + Buffer.from([0, 0, 0, 1]), + Buffer.from([0, 0, 3, 0]), + ], + bftWeightsUpdate: [BigInt(1), BigInt(3), BigInt(4)], + // 7 corresponds to 0111 + bftWeightsUpdateBitmap: Buffer.from([7]), + }, + }; + const existingKey = Buffer.from([0, 2, 3, 0]); + await chainValidatorsSubstore.set(methodContext, ccu.sendingChainID, { + activeValidators: [{ blsKey: existingKey, bftWeight: BigInt(2) }], + certificateThreshold: BigInt(1), + }); + const newValidators = [ + { blsKey: Buffer.from([0, 0, 0, 0]), bftWeight: BigInt(1000000000000) }, + { blsKey: Buffer.from([0, 0, 0, 1]), bftWeight: BigInt(1000000000000) }, + { blsKey: Buffer.from([0, 0, 2, 0]), bftWeight: BigInt(1000000000000) }, + ]; + jest.spyOn(utils, 'calculateNewActiveValidators').mockReturnValue(newValidators); + + await expect( + mainchainInteroperabilityInternalMethod.verifyValidatorsUpdate(methodContext, ccu), + ).rejects.toThrow('Certificate threshold is too small.'); + }); + + it('should reject if certificate threshold is too large', async () => { + const ccu = { + ...ccuParams, + certificate: codec.encode(certificateSchema, defaultCertificate), + activeValidatorsUpdate: { + blsKeysUpdate: [ + Buffer.from([0, 0, 0, 0]), + Buffer.from([0, 0, 0, 1]), + Buffer.from([0, 0, 3, 0]), + ], + bftWeightsUpdate: [BigInt(1), BigInt(3), BigInt(4)], + // 7 corresponds to 0111 + bftWeightsUpdateBitmap: Buffer.from([7]), + }, + }; + const existingKey = Buffer.from([0, 2, 3, 0]); + await chainValidatorsSubstore.set(methodContext, ccu.sendingChainID, { + activeValidators: [{ blsKey: existingKey, bftWeight: BigInt(2) }], + certificateThreshold: BigInt(1), + }); + const newValidators = [ + { blsKey: Buffer.from([0, 0, 0, 0]), bftWeight: BigInt(1) }, + { blsKey: Buffer.from([0, 0, 0, 1]), bftWeight: BigInt(1) }, + { blsKey: Buffer.from([0, 0, 2, 0]), bftWeight: BigInt(1) }, + ]; + jest.spyOn(utils, 'calculateNewActiveValidators').mockReturnValue(newValidators); + + await expect( + mainchainInteroperabilityInternalMethod.verifyValidatorsUpdate(methodContext, ccu), + ).rejects.toThrow('Certificate threshold is too large.'); + }); + it('should reject if new validatorsHash does not match with certificate', async () => { const ccu = { ...ccuParams, diff --git a/framework/test/unit/modules/interoperability/utils.spec.ts b/framework/test/unit/modules/interoperability/utils.spec.ts index 84576d455a0..b7be77d65fb 100644 --- a/framework/test/unit/modules/interoperability/utils.spec.ts +++ b/framework/test/unit/modules/interoperability/utils.spec.ts @@ -33,10 +33,7 @@ import { } from '../../../../src/modules/interoperability/types'; import { checkCertificateTimestamp, - checkCertificateValidity, - checkLivenessRequirementFirstCCU, - checkValidatorsHashWithCertificate, - computeValidatorsHash, + checkLivenessRequirement, validateFormat, verifyLivenessConditionForRegisteredChains, isValidName, @@ -61,22 +58,11 @@ describe('Utils', () => { signature: cryptography.utils.getRandomBytes(BLS_SIGNATURE_LENGTH), }; - const defaultActiveValidatorsUpdate = { - blsKeysUpdate: [ - utils.getRandomBytes(48), - utils.getRandomBytes(48), - utils.getRandomBytes(48), - utils.getRandomBytes(48), - ].sort((v1, v2) => v1.compare(v2)), - bftWeightsUpdate: [BigInt(1), BigInt(3), BigInt(4), BigInt(3)], - bftWeightsUpdateBitmap: Buffer.from([1, 0, 2]), - }; - beforeEach(() => { jest.spyOn(validator, 'validate'); }); - describe('checkLivenessRequirementFirstCCU', () => { + describe('checkLivenessRequirement', () => { const partnerChainAccount = { status: ChainStatus.REGISTERED, }; @@ -91,7 +77,7 @@ describe('Utils', () => { }; it(`should return VerifyStatus.FAIL status when chain status ${ChainStatus.REGISTERED} && certificate is empty`, () => { - const result = checkLivenessRequirementFirstCCU( + const result = checkLivenessRequirement( partnerChainAccount as ChainAccount, txParamsEmptyCertificate as CrossChainUpdateTransactionParams, ); @@ -99,7 +85,7 @@ describe('Utils', () => { }); it(`should return status VerifyStatus.OK status when chain status ${ChainStatus.REGISTERED} && certificate is non-empty`, () => { - const result = checkLivenessRequirementFirstCCU( + const result = checkLivenessRequirement( partnerChainAccount as ChainAccount, txParamsNonEmptyCertificate as CrossChainUpdateTransactionParams, ); @@ -107,74 +93,6 @@ describe('Utils', () => { }); }); - describe('checkCertificateValidity', () => { - const partnerChainAccount = { - lastCertificate: { - height: 20, - }, - }; - - const partnerChainAccountWithHigherHeight = { - lastCertificate: { - height: 40, - }, - }; - - const certificate = { - ...defaultCertificate, - }; - - const certificateWithEmptyValues = { - ...defaultCertificate, - stateRoot: EMPTY_BYTES, - validatorsHash: EMPTY_BYTES, - aggregationBits: EMPTY_BYTES, - signature: EMPTY_BYTES, - }; - - const encodedCertificate = codec.encode(certificateSchema, certificate); - const encodedWithEmptyValuesCertificate = codec.encode( - certificateSchema, - certificateWithEmptyValues, - ); - - it('should return VerifyStatus.FAIL when certificate required properties are missing', () => { - const { status, error } = checkCertificateValidity( - partnerChainAccount as ChainAccount, - encodedWithEmptyValuesCertificate, - ); - - expect(status).toEqual(VerifyStatus.FAIL); - expect(error?.message).toBe('Certificate is missing required values.'); - }); - - it('should return VerifyStatus.FAIL when certificate height is less than or equal to last certificate height', () => { - const { status, error } = checkCertificateValidity( - partnerChainAccountWithHigherHeight as ChainAccount, - encodedCertificate, - ); - - expect(status).toEqual(VerifyStatus.FAIL); - expect(error?.message).toBe( - 'Certificate height should be greater than last certificate height.', - ); - }); - - it('should return VerifyStatus.OK when certificate has all values and height greater than last certificate height', () => { - const { status, error } = checkCertificateValidity( - partnerChainAccount as ChainAccount, - encodedCertificate, - ); - - expect(status).toEqual(VerifyStatus.OK); - expect(error).toBeUndefined(); - expect(validator.validate).toHaveBeenCalledWith( - certificateSchema, - expect.toBeObject() as Certificate, - ); - }); - }); - describe('checkCertificateTimestamp', () => { const timestamp = Date.now(); const txParams: any = { @@ -207,157 +125,18 @@ describe('Utils', () => { ).toThrow('Certificate is invalid due to invalid timestamp.'); }); - it('should return undefined certificate.timestamp is less than header.timestamp', () => { - expect(checkCertificateTimestamp(txParams, certificate, header)).toBeUndefined(); - }); - }); - - describe('checkValidatorsHashWithCertificate', () => { - const activeValidatorsUpdate = { ...defaultActiveValidatorsUpdate }; - const partnerValidators: any = { - certificateThreshold: BigInt(10), - activeValidators: activeValidatorsUpdate.blsKeysUpdate.map((v, i) => ({ - blsKey: v, - bftWeight: activeValidatorsUpdate.bftWeightsUpdate[i] + BigInt(1), - })), - }; - const validatorsHash = computeValidatorsHash( - partnerValidators.activeValidators, - partnerValidators.certificateThreshold, - ); - - const certificate: Certificate = { - ...defaultCertificate, - validatorsHash, - }; - - const encodedCertificate = codec.encode(certificateSchema, certificate); - - const txParams: any = { - certificate: encodedCertificate, - activeValidatorsUpdate, - certificateThreshold: BigInt(10), - }; - - beforeEach(() => { - jest - .spyOn(interopUtils, 'calculateNewActiveValidators') - .mockReturnValue(partnerValidators.activeValidators); - }); - - it('should return VerifyStatus.FAIL when certificate is empty', () => { - const txParamsWithIncorrectHash = { ...txParams, certificate: EMPTY_BYTES }; - const { status, error } = checkValidatorsHashWithCertificate( - txParamsWithIncorrectHash, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.FAIL); - expect(error?.message).toBe( - 'Certificate cannot be empty when activeValidatorsUpdate or certificateThreshold has a non-empty value.', - ); - }); - - it('should return VerifyStatus.FAIL when certificate has missing fields', () => { - const txParamsWithIncorrectHash = { ...txParams, certificate: Buffer.alloc(2) }; - const { status, error } = checkValidatorsHashWithCertificate( - txParamsWithIncorrectHash, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.FAIL); - expect(error?.message).toBe( - 'Certificate should have all required values when activeValidatorsUpdate or certificateThreshold has a non-empty value.', - ); - }); - - it('should return VerifyStatus.FAIL when validators hash is incorrect', () => { - const certificateInvalidValidatorHash: Certificate = { - ...certificate, - validatorsHash: cryptography.utils.getRandomBytes(HASH_LENGTH), - }; - const invalidEncodedCertificate = codec.encode( - certificateSchema, - certificateInvalidValidatorHash, - ); - - const txParamsWithIncorrectHash = { ...txParams, certificate: invalidEncodedCertificate }; - const { status, error } = checkValidatorsHashWithCertificate( - txParamsWithIncorrectHash, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.FAIL); - expect(error?.message).toBe('Validators hash given in the certificate is incorrect.'); - }); - - it('should return VerifyStatus.OK when validators hash is correct', () => { - const txParamsWithCorrectHash = { ...txParams }; - const { status, error } = checkValidatorsHashWithCertificate( - txParamsWithCorrectHash, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.OK); - expect(error).toBeUndefined(); - expect(validator.validate).toHaveBeenCalledWith( - certificateSchema, - expect.toBeObject() as Certificate, - ); - }); - - it('should return VerifyStatus.OK when activeValidatorsUpdate is empty and certificateThreshold === 0', () => { - const ineligibleTxParams = { - ...txParams, - activeValidatorsUpdate: { - bftWeightsUpdate: [], - bftWeightsUpdateBitmap: Buffer.from([]), - blsKeysUpdate: [], - }, - certificateThreshold: BigInt(0), - }; - const { status, error } = checkValidatorsHashWithCertificate( - ineligibleTxParams, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.OK); - expect(error).toBeUndefined(); - }); - - it('should return VerifyStatus.OK when certificateThreshold === 0 but activeValidatorsUpdate.length > 0', () => { - const { status, error } = checkValidatorsHashWithCertificate( - { ...txParams, certificateThreshold: BigInt(0) }, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.OK); - expect(error).toBeUndefined(); - expect(validator.validate).toHaveBeenCalledWith( - certificateSchema, - expect.toBeObject() as Certificate, - ); + it('should throw error when certificate.timestamp is equal to header.timestamp', () => { + expect(() => + checkCertificateTimestamp( + txParams, + { ...certificate, timestamp: header.timestamp }, + header, + ), + ).toThrow('Certificate is invalid due to invalid timestamp.'); }); - it('should return VerifyStatus.OK when certificateThreshold > 0 but activeValidatorsUpdate is empty', () => { - const { status, error } = checkValidatorsHashWithCertificate( - { - ...txParams, - activeValidatorsUpdate: { - bftWeightsUpdate: [], - bftWeightsUpdateBitmap: Buffer.from([]), - blsKeysUpdate: [], - }, - }, - partnerValidators, - ); - - expect(status).toEqual(VerifyStatus.OK); - expect(error).toBeUndefined(); - expect(validator.validate).toHaveBeenCalledWith( - certificateSchema, - expect.toBeObject() as Certificate, - ); + it('should return undefined certificate.timestamp is less than header.timestamp', () => { + expect(checkCertificateTimestamp(txParams, certificate, header)).toBeUndefined(); }); });