diff --git a/packages/crypto-commit/source/factory.ts b/packages/crypto-commit/source/factory.ts index cfad268e2..398e323a0 100644 --- a/packages/crypto-commit/source/factory.ts +++ b/packages/crypto-commit/source/factory.ts @@ -29,7 +29,6 @@ export class CommitFactory implements Contracts.Crypto.CommitFactory { const proofBuffer = buffer.readBytes(this.proofSize()); const proof = await this.commitDeserializer.deserializeCommitProof(proofBuffer); - this.#verifySchema("commitProof", proof); const block = await this.blockFactory.fromBytes(buffer.getRemainder()); @@ -39,7 +38,7 @@ export class CommitFactory implements Contracts.Crypto.CommitFactory { serialized: buff.toString("hex"), }; - this.#verifySchema("commit", commit); + this.#verifySchema(commit); return commit; } @@ -75,15 +74,15 @@ export class CommitFactory implements Contracts.Crypto.CommitFactory { proof: json.proof, serialized: json.serialized, }; - this.#verifySchema("commit", commit); + this.#verifySchema(commit); return commit; } - #verifySchema(schema: string, data: T): void { - const result = this.validator.validate(schema, data); + #verifySchema(data: T): void { + const result = this.validator.validate("commit", data); if (result.error) { - throw new MessageSchemaError(schema, result.error); + throw new MessageSchemaError("commit", result.error); } } } diff --git a/packages/crypto-commit/source/schemas.test.ts b/packages/crypto-commit/source/schemas.test.ts index b9aaa55bc..9cf835107 100644 --- a/packages/crypto-commit/source/schemas.test.ts +++ b/packages/crypto-commit/source/schemas.test.ts @@ -19,19 +19,21 @@ import { schemas } from "./schemas"; describe<{ app: Application; validator: Validator; -}>("Schemas", ({ it, assert, beforeEach }) => { + configuration: Configuration; +}>("Schemas", ({ it, assert, beforeEach, spy }) => { beforeEach((context) => { context.app = new Application(); context.app.bind(Identifiers.Cryptography.Configuration).to(Configuration).inSingletonScope(); - context.app.get(Identifiers.Cryptography.Configuration).setConfig(cryptoJson); - context.app.get(Identifiers.Cryptography.Configuration).setHeight(1); + context.configuration = context.app.get(Identifiers.Cryptography.Configuration); + context.configuration.setConfig(cryptoJson); + context.configuration.setHeight(1); context.validator = context.app.resolve(Validator); for (const keyword of Object.values({ - ...makeBaseKeywords(context.app.get(Identifiers.Cryptography.Configuration)), - ...makeTransactionKeywords(context.app.get(Identifiers.Cryptography.Configuration)), + ...makeBaseKeywords(context.configuration), + ...makeTransactionKeywords(context.configuration), })) { context.validator.addKeyword(keyword); } @@ -60,6 +62,20 @@ describe<{ assert.undefined(result.error); }); + it("commit - should correctly parse block number", ({ validator, configuration }) => { + const spyGetMilestone = spy(configuration, "getMilestone"); + + const result = validator.validate("commit", { + block: blockData, + proof: commitProof1, + serialized: commitSerialized, + }); + assert.undefined(result.error); + + spyGetMilestone.calledTimes(7); // 6 x for block.number and 1 x for limitToRoundValidators + spyGetMilestone.calledWith(blockData.number); + }); + it("commit - should not allow additional fields", ({ validator }) => { const result = validator.validate("commit", { block: blockData, diff --git a/packages/crypto-commit/source/schemas.ts b/packages/crypto-commit/source/schemas.ts index 3d8c71f20..f2c451450 100644 --- a/packages/crypto-commit/source/schemas.ts +++ b/packages/crypto-commit/source/schemas.ts @@ -20,6 +20,9 @@ export const schemas: Record<"commit" | "commitProof", AnySchemaObject> = { signature: { $ref: "consensusSignature" }, validators: { items: { type: "boolean" }, + // NOTE: This is not an actual property of the commit proof, but we need it to validate the commit proof against the correct set of validators. + // We take value from block.number, which is available in the commit schema + limitToRoundValidators: { blockNumberPath: "block.number" }, type: "array", }, }, diff --git a/packages/crypto-messages/source/schemas.test.ts b/packages/crypto-messages/source/schemas.test.ts index b0f3456a3..b449a626c 100644 --- a/packages/crypto-messages/source/schemas.test.ts +++ b/packages/crypto-messages/source/schemas.test.ts @@ -15,18 +15,20 @@ import { schemas } from "./schemas"; describe<{ app: Application; validator: Validator; -}>("Schemas", ({ it, assert, beforeEach }) => { + configuration: Configuration; +}>("Schemas", ({ it, assert, beforeEach, spy }) => { beforeEach((context) => { context.app = new Application(); context.app.bind(Identifiers.Cryptography.Configuration).to(Configuration).inSingletonScope(); - context.app.get(Identifiers.Cryptography.Configuration).setConfig(cryptoJson); - context.app.get(Identifiers.Cryptography.Configuration).setHeight(1); // Required by schema to set number for validators + context.configuration = context.app.get(Identifiers.Cryptography.Configuration); + context.configuration.setConfig(cryptoJson); + context.configuration.setHeight(1); // Required by schema to set number for validators context.validator = context.app.resolve(Validator); for (const keyword of Object.values({ - ...makeBaseKeywords(context.app.get(Identifiers.Cryptography.Configuration)), + ...makeBaseKeywords(context.configuration), })) { context.validator.addKeyword(keyword); } @@ -144,4 +146,14 @@ describe<{ const result = validator.validate("message", { ...prevoteData, extraField: 123 }); assert.defined(result.error); }); + + it("message - should correctly parse block number", async ({ validator, configuration }) => { + const spyGetMilestone = spy(configuration, "getMilestone"); + + const result = validator.validate("message", { ...prevoteData, blockNumber: 3 }); + assert.undefined(result.error); + + spyGetMilestone.calledOnce(); + spyGetMilestone.calledWith(3); + }); }); diff --git a/packages/crypto-messages/source/schemas.ts b/packages/crypto-messages/source/schemas.ts index 9c42a70bf..48d33ad1c 100644 --- a/packages/crypto-messages/source/schemas.ts +++ b/packages/crypto-messages/source/schemas.ts @@ -11,7 +11,7 @@ export const schemas: Record<"message", AnySchemaObject> = { round: { minimum: 0, type: "integer" }, signature: { $ref: "consensusSignature" }, type: { enum: [Enums.Crypto.MessageType.Prevote, Enums.Crypto.MessageType.Precommit] }, - validatorIndex: { isValidatorIndex: {} }, + validatorIndex: { isValidatorIndex: { blockNumberPath: "blockNumber" } }, }, required: ["type", "blockNumber", "round", "validatorIndex", "signature"], type: "object", diff --git a/packages/crypto-proposal/source/factory.ts b/packages/crypto-proposal/source/factory.ts index f8335403d..2690fe767 100644 --- a/packages/crypto-proposal/source/factory.ts +++ b/packages/crypto-proposal/source/factory.ts @@ -63,6 +63,10 @@ export class Factory implements Contracts.Crypto.ProposalFactory { const lockProof = await this.#getLockProof(buffer); const block = await this.blockFactory.fromBytes(buffer.getRemainder()); + if (lockProof) { + this.#verifySchema("lockProof", { ...lockProof, number: block.number }); + } + return { block, lockProof, @@ -77,8 +81,6 @@ export class Factory implements Contracts.Crypto.ProposalFactory { if (lockProofLength > 0) { const lockProofBuffer = buffer.readBytes(lockProofLength); lockProof = await this.deserializer.deserializeLockProof(lockProofBuffer); - - this.#verifySchema("lockProof", lockProof); } return lockProof; @@ -92,6 +94,10 @@ export class Factory implements Contracts.Crypto.ProposalFactory { const lockProof = await this.#getLockProof(buffer); const blockHeader = await this.blockFactory.headerFromBytes(buffer.getRemainder()); + if (lockProof) { + this.#verifySchema("lockProof", { ...lockProof, number: blockHeader.number }); + } + return { blockHeader, lockProof, diff --git a/packages/crypto-proposal/source/schemas.test.ts b/packages/crypto-proposal/source/schemas.test.ts index 7debaa8ed..fb996a03e 100644 --- a/packages/crypto-proposal/source/schemas.test.ts +++ b/packages/crypto-proposal/source/schemas.test.ts @@ -18,24 +18,27 @@ import { } from "../test/fixtures/index.js"; import { schemas } from "./schemas"; import { signature } from "../test/fixtures/proposal.js"; +import { numberArray } from "@mainsail/utils"; describe<{ app: Application; validator: Validator; -}>("Schemas", ({ it, assert, beforeEach }) => { + configuration: Configuration; +}>("Schemas", ({ it, assert, beforeEach, spy }) => { const proposals = [Proposal, ProposalWithValidRound, ProposalWithLockProof, ProposalWithLockProofAndValidRound]; beforeEach((context) => { context.app = new Application(); context.app.bind(Identifiers.Cryptography.Configuration).to(Configuration).inSingletonScope(); - context.app.get(Identifiers.Cryptography.Configuration).setConfig(cryptoJson); - context.app.get(Identifiers.Cryptography.Configuration).setHeight(1); + context.configuration = context.app.get(Identifiers.Cryptography.Configuration); + context.configuration.setConfig(cryptoJson); + context.configuration.setHeight(1); context.validator = context.app.resolve(Validator); for (const keyword of Object.values({ - ...makeBaseKeywords(context.app.get(Identifiers.Cryptography.Configuration)), + ...makeBaseKeywords(context.configuration), })) { context.validator.addKeyword(keyword); } @@ -180,14 +183,15 @@ describe<{ }); it("lockProof - should be ok", ({ validator }) => { - const result = validator.validate("lockProof", lockProof); + const result = validator.validate("lockProof", { ...lockProof, number: 1 }); assert.undefined(result.error); }); - it("lockProof - all fields are required", ({ validator }) => { + it("lockProof - all fields are required (except number)", ({ validator }) => { const keys = Object.keys(schemas.lockProof.properties); for (let key of keys) { - const lockProofCopy = { ...lockProof, [key]: undefined }; + if (key === "number") continue; + const lockProofCopy = { ...lockProof, [key]: undefined, number: 1 }; const result = validator.validate("lockProof", lockProofCopy); assert.defined(result.error); assert.true(result.error?.includes(key)); @@ -195,7 +199,7 @@ describe<{ }); it("lockProof - should not allow additional fields", ({ validator }) => { - const lockProofCopy = { ...lockProof, extraField: "extraValue" }; + const lockProofCopy = { ...lockProof, extraField: "extraValue", number: 1 }; const result = validator.validate("lockProof", lockProofCopy); assert.defined(result.error); assert.true(result.error!.includes("additional properties")); @@ -204,6 +208,7 @@ describe<{ it("lockProof - signature should be ok", ({ validator }) => { for (let char of "0123456789abcdef") { const result = validator.validate("lockProof", { + number: 1, signature: char.repeat(192), validators: Array(53).fill(true), }); @@ -221,6 +226,7 @@ describe<{ const result = validator.validate("lockProof", { signature, validators: Array(53).fill(true), + number: 1, }); assert.defined(result.error); assert.true(result.error!.includes("signature")); @@ -238,6 +244,7 @@ describe<{ for (let validators of validValidators) { const result = validator.validate("lockProof", { + number: 1, signature, validators, }); @@ -258,9 +265,33 @@ describe<{ const result = validator.validate("lockProof", { signature, validators, + number: 1, }); assert.defined(result.error); assert.true(result.error!.includes("validators")); } }); + + it("proposalUnsigned - should correctly deserialize block number from payloadSerialized", ({ + validator, + configuration, + }) => { + const spyConfigurationGetMilestone = spy(configuration, "getMilestone"); + + const result = validator.validate("proposalUnsigned", Proposal.proposalDataSerializableUnsigned); + assert.undefined(result.error); + + spyConfigurationGetMilestone.calledOnce(); + spyConfigurationGetMilestone.calledWith(2); + }); + + it("lockProof - should correctly parse block number from number", ({ validator, configuration }) => { + const spyConfigurationGetMilestone = spy(configuration, "getMilestone"); + + const result = validator.validate("lockProof", { ...lockProof, number: 3 }); + assert.undefined(result.error); + + spyConfigurationGetMilestone.calledOnce(); + spyConfigurationGetMilestone.calledWith(3); + }); }); diff --git a/packages/crypto-proposal/source/schemas.ts b/packages/crypto-proposal/source/schemas.ts index 9e3200861..41c6490c6 100644 --- a/packages/crypto-proposal/source/schemas.ts +++ b/packages/crypto-proposal/source/schemas.ts @@ -7,7 +7,7 @@ const proposalUnsigned = { payloadSerialized: { $ref: "hex" }, round: { minimum: 0, type: "integer" }, validRound: { minimum: 0, type: "integer" }, - validatorIndex: { isValidatorIndex: {} }, + validatorIndex: { isValidatorIndex: { blockNumberPath: "payloadSerialized" } }, }, required: ["round", "payloadSerialized", "validatorIndex"], type: "object", @@ -18,14 +18,18 @@ export const schemas: Record<"lockProof" | "proposal" | "proposalUnsigned", AnyS $id: "lockProof", additionalProperties: false, properties: { + // NOTE: This is not an actual property of the lock proof, but we need it to validate the lock proof against the correct set of validators. + number: { minimum: 0, type: "integer" }, + signature: { $ref: "consensusSignature" }, + validators: { items: { type: "boolean" }, - limitToRoundValidators: {}, + limitToRoundValidators: { blockNumberPath: "number" }, type: "array", }, }, - required: ["signature", "validators"], + required: ["signature", "validators", "number"], type: "object", }, proposal: { diff --git a/packages/crypto-validation/source/keywords.ts b/packages/crypto-validation/source/keywords.ts index 00091f366..391e2f65c 100644 --- a/packages/crypto-validation/source/keywords.ts +++ b/packages/crypto-validation/source/keywords.ts @@ -2,6 +2,8 @@ import type { Contracts } from "@mainsail/contracts"; import { BigNumber } from "@mainsail/utils"; import type { FuncKeywordDefinition } from "ajv"; +import { parseBlockNumber } from "./parse-block-number.js"; + export const makeKeywords = ( configuration: Contracts.Crypto.Configuration, ): { @@ -62,13 +64,14 @@ export const makeKeywords = ( // Use by: crypto-proposal, p2p const limitToRoundValidators: FuncKeywordDefinition = { - compile(schema: { minimum?: number }) { - return (data) => { + compile(schema: { minimum?: number; blockNumberPath?: string }) { + return (data, parentSchema) => { if (!Array.isArray(data)) { return false; } - const { roundValidators } = configuration.getMilestone(); + const blockNumber = parseBlockNumber(schema.blockNumberPath, parentSchema); + const { roundValidators } = configuration.getMilestone(blockNumber); const minimum = schema.minimum !== undefined ? schema.minimum : roundValidators; if (data.length < minimum || data.length > roundValidators) { @@ -82,6 +85,7 @@ export const makeKeywords = ( keyword: "limitToRoundValidators", metaSchema: { properties: { + blockNumberPath: { type: "string" }, minimum: { type: "integer" }, }, type: "object", @@ -90,8 +94,8 @@ export const makeKeywords = ( // Used by: crypto-messages (prevotes / precommits) and crypto-proposal const isValidatorIndex: FuncKeywordDefinition = { - compile() { - return (data) => { + compile(schema: { blockNumberPath?: string }) { + return (data, parentSchema) => { if (!Number.isInteger(data)) { return false; } @@ -100,13 +104,20 @@ export const makeKeywords = ( return false; } - const { roundValidators } = configuration.getMilestone(); + const blockNumber = parseBlockNumber(schema.blockNumberPath, parentSchema); + const { roundValidators } = configuration.getMilestone(blockNumber); return data < roundValidators; }; }, errors: false, keyword: "isValidatorIndex", + metaSchema: { + properties: { + blockNumberPath: { type: "string" }, + }, + type: "object", + }, }; return { bignumber, buffer, isValidatorIndex, limitToRoundValidators, maxBytes }; diff --git a/packages/crypto-validation/source/parse-block-number.test.ts b/packages/crypto-validation/source/parse-block-number.test.ts new file mode 100644 index 000000000..8190ac9b4 --- /dev/null +++ b/packages/crypto-validation/source/parse-block-number.test.ts @@ -0,0 +1,100 @@ +import { Validator } from "@mainsail/validation/source/validator"; + +import { Application } from "@mainsail/kernel"; +import { describe } from "@mainsail/test-runner"; +import { parseBlockNumber } from "./parse-block-number"; +import { + Proposal, + ProposalWithLockProof, + ProposalWithLockProofAndValidRound, + ProposalWithValidRound, +} from "../../crypto-proposal/test/fixtures/index.js"; + +describe<{ + app: Application; + validator: Validator; +}>("Keywords", ({ it, beforeEach, assert }) => { + it("should return undefined if path is undefined", () => { + const result = parseBlockNumber(undefined, { + rootData: {}, + }); + assert.is(result, undefined); + }); + + it("should return undefined if path does not exist in rootData", () => { + const result = parseBlockNumber("nonexistent.path", { + rootData: {}, + }); + assert.undefined(result); + }); + + it("should return undefined if value at path is not a number", () => { + const result = parseBlockNumber("some.path", { + rootData: { + some: { path: "not a number" }, + }, + }); + assert.undefined(result); + }); + + it("should return undefined if value at path is string representation of a number", () => { + const result = parseBlockNumber("some.path", { + rootData: { + some: { path: "42" }, + }, + }); + assert.undefined(result); + }); + + it("should return the number at the specified path (root)", () => { + const result = parseBlockNumber("path", { + rootData: { + path: 42, + }, + }); + assert.equal(result, 42); + }); + + it("should return the number at the specified path (nested)", () => { + const result = parseBlockNumber("some.path", { + rootData: { + some: { path: 42 }, + }, + }); + assert.equal(result, 42); + }); + + it("should return undefined if payloadSerialized is missing", () => { + const result = parseBlockNumber("payloadSerialized", { + rootData: {}, + }); + assert.undefined(result); + }); + + it("should return undefined if payloadSerialized is too short", () => { + const result = parseBlockNumber("payloadSerialized", { + rootData: { + payloadSerialized: "00", + }, + }); + assert.undefined(result); + }); + + it("should return the block number from payloadSerialized", () => { + const payloadSerialized = Proposal.payloadSerialized; + + for (const payloadSerialized of [ + Proposal.payloadSerialized, + ProposalWithLockProof.payloadSerialized, + ProposalWithLockProofAndValidRound.payloadSerialized, + ProposalWithValidRound.payloadSerialized, + ]) { + const result = parseBlockNumber("payloadSerialized", { + rootData: { + payloadSerialized, + }, + }); + assert.equal(result, 2); + } + }); +}); diff --git a/packages/crypto-validation/source/parse-block-number.ts b/packages/crypto-validation/source/parse-block-number.ts new file mode 100644 index 000000000..0d1d26cf1 --- /dev/null +++ b/packages/crypto-validation/source/parse-block-number.ts @@ -0,0 +1,56 @@ +// Copied form ajv and modified +interface DataValidationCxt { + rootData: Record | unknown[]; +} + +const parseOnPath = (path: string, rootData: object): number | undefined => { + const parts = path.split("."); + let current = rootData; + + for (const part of parts) { + if (current[part] === undefined) { + return undefined; + } + current = current[part]; + } + + if (typeof current === "number") { + return current; + } + + return undefined; +}; + +// Proposals contain the block only in serialized form (hex). +// We can extract the block number at a fixed offset here, without needing to deserialize the whole block. +const parseSerializedPayload = (serialized): number | undefined => { + if (!serialized) { + return undefined; + } + + if (serialized.length < 30) { + return undefined; + } + + const lockProofSize = 2 + Number.parseInt(serialized.slice(0, 2), 16) * 2; + // version: 1 byte (2 hex) + // timestamp: 6 bytes (12 hex) + // blockNumber: 4 byte (8 hex) + const offset = lockProofSize + 2 + 12; + return Buffer.from(serialized.slice(offset, offset + 8), "hex").readUInt32LE(); +}; + +export const parseBlockNumber = ( + path: string | undefined, + parentSchema: DataValidationCxt | undefined, +): number | undefined => { + if (path === undefined || parentSchema === undefined) { + return undefined; + } + + if (path === "payloadSerialized") { + return parseSerializedPayload(parentSchema.rootData?.["payloadSerialized"]); + } + + return parseOnPath(path, parentSchema.rootData); +};