fix(native-scripts): refuse a JSON slot above 2^53-1 rather than truncate it

.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.

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>("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<NativeSc
 // JSON Conversion Utilities
 // ============================================================================
 
+/**
+ * Largest magnitude a JSON number carries exactly. The cardano-cli JSON form uses JSON numbers for
+ * `slot` and `required`, so a `uint64` slot or an `int64` `required` outside this has no
+ * representation in it.
+ */
+const JSON_SAFE_MAX = BigInt(Number.MAX_SAFE_INTEGER)
+
+const jsonNumber = (value: bigint, field: string): number => {
+  // 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)
       }
   }

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)))
+  })
+})