From a10ca13ce32191f3e7c1e63165095a678685bdc2 Mon Sep 17 00:00:00 2001 From: jorbuedo Date: Wed, 21 Oct 2026 13:01:19 +0200 Subject: [PATCH] fix(native-scripts): refuse a JSON slot above 2^53-1 rather than truncate it The cardano-cli JSON form carries `slot` and `required` as JSON numbers, so a CDDL uint64 above `Number.MAX_SAFE_INTEGER` has no exact representation in it. `Number(script.slot)` returned a different slot and reported nothing. `required` is an int64 and the ledger accepts a negative one, so the lower bound is reachable the same way the upper one is, and is refused with it. A script read off the chain may carry any uint64, so this is reachable on decode and not only when authoring one. The refusal is the module's `NativeScriptError`, so callers can catch it by tag. `Hash.symbol` hashes the variant structurally instead of going through `toJSON`, which would now throw for exactly these scripts: a `HashSet` holding one would fail, and before the refusal two scripts differing only above the bound hashed equal. Structurally means a key hash hashes from its bytes rather than from its container, so a `Buffer` and a `Uint8Array` over the same bytes still hash alike and `Equal.equals`, which compares hashes first, still holds for them. `fromJSON`, the inverse that found this, is left for its own change. --- .changeset/native-scripts-json-slot-range.md | 7 ++ packages/evolution/src/NativeScripts.ts | 53 ++++++++++- .../evolution/test/NativeScripts.JSON.test.ts | 94 +++++++++++++++++++ 3 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 .changeset/native-scripts-json-slot-range.md create mode 100644 packages/evolution/test/NativeScripts.JSON.test.ts diff --git a/.changeset/native-scripts-json-slot-range.md b/.changeset/native-scripts-json-slot-range.md new file mode 100644 index 000000000..e04dd9354 --- /dev/null +++ b/.changeset/native-scripts-json-slot-range.md @@ -0,0 +1,7 @@ +--- +"@evolution-sdk/evolution": patch +--- + +`NativeScripts.toJSON` refuses a `slot` or `required` outside ±`Number.MAX_SAFE_INTEGER` instead of truncating it. The cardano-cli JSON form carries both as JSON numbers, so a `uint64` slot above 2^53-1 has no exact representation in it, and `Number(slot)` returned a different slot with no error; `required` is an `int64` the ledger allows to be negative, so the lower bound is reachable the same way. A script decoded from the chain may carry either, so this was reachable on read, not only when authoring a script. The refusal is the module's `NativeScriptError`, so callers can catch it by tag. + +`NativeScript`'s `Hash.symbol` now hashes the variant structurally instead of routing through `toJSON`, so such a script still hashes and compares. The structural hash also hashes a key hash from its bytes rather than its container, which keeps `Equal.equals` true for a `Buffer` and a `Uint8Array` over the same bytes. diff --git a/packages/evolution/src/NativeScripts.ts b/packages/evolution/src/NativeScripts.ts index 26037b31e..ddd818e81 100644 --- a/packages/evolution/src/NativeScripts.ts +++ b/packages/evolution/src/NativeScripts.ts @@ -102,6 +102,27 @@ const equalsVariant = (a: NativeScriptVariants, b: NativeScriptVariants): boolea } } +/** + * Helper function to recursively hash NativeScriptVariants, agreeing with `equalsVariant` + */ +const hashVariant = (script: NativeScriptVariants): number => { + const tag = Hash.string(script._tag) + switch (script._tag) { + case "ScriptPubKey": + // Hashed from the bytes, not their container: `equalsVariant` compares with `Bytes.equals`, so + // a `Buffer` and a `Uint8Array` over the same bytes are equal and must hash alike. + return Hash.combine(Hash.array(Array.from(script.keyHash)))(tag) + case "InvalidBefore": + case "InvalidHereafter": + return Hash.combine(Hash.hash(script.slot))(tag) + case "ScriptAll": + case "ScriptAny": + return Hash.combine(Hash.array(script.scripts.map(hashVariant)))(tag) + case "ScriptNOfK": + return Hash.combine(Hash.array(script.scripts.map(hashVariant)))(Hash.combine(Hash.hash(script.required))(tag)) + } +} + /** * Internal Union schema for the actual native script variants * @@ -164,7 +185,9 @@ export class NativeScript extends Schema.TaggedClass("NativeScript } [Hash.symbol](): number { - return Hash.hash(JSON.stringify(toJSON(this.script))) + // Hashed structurally, not through `toJSON`: that form refuses a `slot` or `required` outside the + // JSON-safe range, so routing through it would throw for a script a `HashSet` must still hold. + return hashVariant(this.script) } } @@ -261,9 +284,31 @@ export const makeScriptNOfK = (required: bigint, scripts: ReadonlyArray { + // Refused rather than truncated: `Number(2n ** 53n + 1n)` silently returns a different slot, and a + // script decoded from the chain may carry any `uint64` slot, or an `int64` `required` the ledger + // allows to be negative. + if (value > JSON_SAFE_MAX || value < -JSON_SAFE_MAX) { + throw new NativeScriptError({ + message: `${field} ${value} exceeds what the cardano-cli JSON form can represent exactly` + }) + } + return Number(value) +} + /** * Convert a NativeScript to JSON representation matching cardano-cli format * + * Throws `NativeScriptError` if `slot` or `required` falls outside ±`Number.MAX_SAFE_INTEGER`, which + * the JSON form cannot carry exactly. + * * @since 2.0.0 * @category conversion */ @@ -277,12 +322,12 @@ export const toJSON = (script: NativeScriptVariants): any => { case "InvalidBefore": return { type: "after" as const, - slot: Number(script.slot) + slot: jsonNumber(script.slot, "slot") } case "InvalidHereafter": return { type: "before" as const, - slot: Number(script.slot) + slot: jsonNumber(script.slot, "slot") } case "ScriptAll": return { @@ -297,7 +342,7 @@ export const toJSON = (script: NativeScriptVariants): any => { case "ScriptNOfK": return { type: "atLeast" as const, - required: Number(script.required), + required: jsonNumber(script.required, "required"), scripts: script.scripts.map(toJSON) } } diff --git a/packages/evolution/test/NativeScripts.JSON.test.ts b/packages/evolution/test/NativeScripts.JSON.test.ts new file mode 100644 index 000000000..fa6732f33 --- /dev/null +++ b/packages/evolution/test/NativeScripts.JSON.test.ts @@ -0,0 +1,94 @@ +import { Equal, Hash } from "effect" +import { describe, expect, it } from "vitest" + +import * as NativeScripts from "../src/NativeScripts.js" + +describe("a slot or `required` the cardano-cli JSON form cannot carry", () => { + // ⚠️ `toJSON` used to do `Number(slot)`, so a slot above 2^53-1 came back as a DIFFERENT slot + // with no error. A script decoded from the chain may legitimately carry any uint64, so this is + // reachable on read, not only when authoring a script. + const tooBig = BigInt(Number.MAX_SAFE_INTEGER) + 1n + + it("refuses an out-of-range slot instead of truncating it", () => { + expect(() => NativeScripts.toJSON({ _tag: "InvalidHereafter", slot: tooBig })).toThrow(/exceeds/) + expect(() => NativeScripts.toJSON({ _tag: "InvalidBefore", slot: tooBig })).toThrow(/exceeds/) + }) + + it("refuses an out-of-range `required` instead of truncating it", () => { + expect(() => NativeScripts.toJSON({ _tag: "ScriptNOfK", required: tooBig, scripts: [] })).toThrow(/exceeds/) + }) + + // `required` is an int64, and the ledger accepts a negative one, so the lower bound is reachable + // the same way the upper one is. + it("refuses a `required` below the negative bound, and carries one inside it", () => { + expect(() => NativeScripts.toJSON({ _tag: "ScriptNOfK", required: -tooBig, scripts: [] })).toThrow(/exceeds/) + expect(NativeScripts.toJSON({ _tag: "ScriptNOfK", required: -1n, scripts: [] })).toEqual({ + type: "atLeast", + required: -1, + scripts: [] + }) + }) + + it("accepts the largest value the form does carry, at either end", () => { + const max = BigInt(Number.MAX_SAFE_INTEGER) + expect(NativeScripts.toJSON({ _tag: "InvalidHereafter", slot: max })).toEqual({ + type: "before", + slot: Number.MAX_SAFE_INTEGER + }) + expect(NativeScripts.toJSON({ _tag: "ScriptNOfK", required: -max, scripts: [] })).toEqual({ + type: "atLeast", + required: -Number.MAX_SAFE_INTEGER, + scripts: [] + }) + }) + + it("refuses with the module's error, so callers can catch it by tag", () => { + expect(() => NativeScripts.toJSON({ _tag: "InvalidHereafter", slot: tooBig })).toThrow( + NativeScripts.NativeScriptError + ) + }) + + it("still hashes and compares such a script, rather than throwing", () => { + // `Hash.symbol` must not route through `toJSON`, or putting one of these in a HashSet would + // throw. Two scripts differing only above the bound must also not hash equal. + const a = new NativeScripts.NativeScript({ script: { _tag: "InvalidHereafter", slot: tooBig } }) + const b = new NativeScripts.NativeScript({ script: { _tag: "InvalidHereafter", slot: tooBig + 1n } }) + expect(() => Hash.hash(a)).not.toThrow() + expect(Hash.hash(a)).not.toBe(Hash.hash(b)) + expect(Equal.equals(a, b)).toBe(false) + }) +}) + +describe("the hash agrees with equality on the bytes, not on their container", () => { + // `Equal.equals` compares hashes before structure (effect `Equal.ts:47`), so a hash that + // distinguishes a `Buffer` from a `Uint8Array` over the same bytes makes two equal scripts + // unequal — which a `JSON.stringify` of the variant did, since it writes a `Buffer` as + // `{"type":"Buffer","data":[…]}` and a `Uint8Array` as `{"0":…}`. + const bytes = new Uint8Array(28).fill(7) + + it("hashes a Buffer key hash and a Uint8Array key hash alike", () => { + const fromBytes = NativeScripts.makeScriptPubKey(bytes) + const fromBuffer = NativeScripts.makeScriptPubKey(Buffer.from(bytes)) + expect(Hash.hash(fromBytes)).toBe(Hash.hash(fromBuffer)) + expect(Equal.equals(fromBytes, fromBuffer)).toBe(true) + }) + + it("keeps that true for a nested script", () => { + const nest = (keyHash: Uint8Array) => + NativeScripts.makeScriptNOfK(1n, [ + { _tag: "ScriptPubKey", keyHash }, + { _tag: "InvalidHereafter", slot: 42n } + ]) + expect(Hash.hash(nest(bytes))).toBe(Hash.hash(nest(Buffer.from(bytes)))) + expect(Equal.equals(nest(bytes), nest(Buffer.from(bytes)))).toBe(true) + }) + + it("still separates scripts that differ", () => { + const other = new Uint8Array(28).fill(8) + expect(Hash.hash(NativeScripts.makeScriptPubKey(bytes))).not.toBe( + Hash.hash(NativeScripts.makeScriptPubKey(other)) + ) + // A tag in the hash keeps the two time variants apart, which carry the same payload. + expect(Hash.hash(NativeScripts.makeInvalidBefore(5n))).not.toBe(Hash.hash(NativeScripts.makeInvalidHereafter(5n))) + }) +})