diff --git a/docs/src/content/docs/framework/query-caching.mdx b/docs/src/content/docs/framework/query-caching.mdx index 657d47835..6be2d7be5 100644 --- a/docs/src/content/docs/framework/query-caching.mdx +++ b/docs/src/content/docs/framework/query-caching.mdx @@ -37,7 +37,7 @@ const { data } = useActorQuery({ }) ``` -Each unique combination of canister ID, function name, and arguments creates a separate cache entry. The whole `args` array is serialized into a **single** string segment, `argsJson` — arguments are never spread into separate elements. It is JSON with BigInt values written as decimal strings and the keys of every plain object sorted, so `{ b, a }` and `{ a, b }` share an entry, and a bare `JSON.stringify(args)` does not match it once a record's fields are out of alphabetical order. A blob is written as a tag followed by its lowercase hex, so a `Uint8Array`, a `number[]` and a `DisplayReactor`'s hex text holding the same bytes share an entry too. A `DisplayReactor` writes each `opt` and variant in one form, whichever it was given in, so an `opt` given bare or as `[value]`, none given as `null`, `undefined` or `[]`, and a variant given with or without `_type` share an entry. A `vec record { text; T }` given to it as an object is written as its entries in order, since the order is part of what it sends. +Each unique combination of canister ID, function name, and arguments creates a separate cache entry. The whole `args` array is serialized into a **single** string segment, `argsJson` — arguments are never spread into separate elements. It is JSON with BigInt values written as decimal strings and the keys of every plain object sorted, so `{ b, a }` and `{ a, b }` share an entry, and a bare `JSON.stringify(args)` does not match it once a record's fields are out of alphabetical order. A blob is written as a tag followed by its lowercase hex, so a `Uint8Array`, a `number[]` and a `DisplayReactor`'s hex text holding the same bytes share an entry too. A `DisplayReactor` writes each `opt` and variant in one form, whichever it was given in, so an `opt` given bare or as `[value]`, none given as `null`, `undefined` or `[]`, and a variant given with or without `_type` share an entry. A `vec record { text; T }` given to it as an object is written as its entries in order, since the order is part of what it sends. Fields a record does not declare are left out and every value of `reserved` is written as `null`, since neither is sent, and a `DisplayReactor` writes a float or an integer of 32 bits or fewer given as text as its number, and a `Principal` as its text. An argument the reactor refuses, such as `undefined` where Candid `null` is required or a bigint where a `DisplayReactor` takes text, is written behind a tag of its own, so the call fails instead of being answered from the cache entry of an argument it takes. The keys above are a `Reactor`'s: `[canisterId, functionName, argsJson]`. A `DisplayReactor` adds a `{ transform: "display" }` segment after the function name — `['rrkah-fqaaa...', 'getUser', { transform: 'display' }, '["user-123"]']` — so a `Reactor` and a `DisplayReactor` over one canister never share a cache entry. Build keys with `generateQueryKey` (below) or a query object's `getQueryKey()` rather than by hand. diff --git a/docs/src/content/docs/reference/Reactor.mdx b/docs/src/content/docs/reference/Reactor.mdx index 724bd3332..3777a4224 100644 --- a/docs/src/content/docs/reference/Reactor.mdx +++ b/docs/src/content/docs/reference/Reactor.mdx @@ -192,7 +192,7 @@ The key is composed as: - **`resolvedCanisterId`** — `callConfig.canisterId` when supplied (normalized via `Principal.from(...).toString()`), otherwise the reactor's own canister ID. - **`{ transform }`** — present whenever the reactor's transform is not `"candid"`, so a `Reactor` key has none and a `DisplayReactor` key carries `{ transform: "display" }`. The two never share a cache entry. - **`{ effectiveTarget }`** — a wrapper object holding either `{ canisterId }` or `{ subnetId }`, built from `callConfig.effectiveTarget` or from `callConfig.effectiveCanisterId`. A `canisterId`-shaped target is **omitted** when it equals `resolvedCanisterId`, so the common case produces no such segment. -- **`argKey`** — args are **one single string segment**: JSON with BigInt values rendered as decimal strings and the keys of every plain object sorted, so `{ b, a }` and `{ a, b }` give the same key. A bare `JSON.stringify(args)` does not match it once a record's fields are out of alphabetical order. A blob is written as a tag followed by its lowercase hex, so a `Uint8Array`, a `number[]` and a `DisplayReactor`'s hex text holding the same bytes give the same key. A `DisplayReactor` writes each `opt` and variant in one form, whichever it was given in, so an `opt` given bare or as `[value]`, none given as `null`, `undefined` or `[]`, and a variant given with or without `_type` give the same key. A `vec record { text; T }` given to it as an object is written as its entries in order, since the order is part of what it sends. Args are not spread into separate elements, and `args: []` still produces the literal `"[]"`. +- **`argKey`** — args are **one single string segment**: JSON with BigInt values rendered as decimal strings and the keys of every plain object sorted, so `{ b, a }` and `{ a, b }` give the same key. A bare `JSON.stringify(args)` does not match it once a record's fields are out of alphabetical order. A blob is written as a tag followed by its lowercase hex, so a `Uint8Array`, a `number[]` and a `DisplayReactor`'s hex text holding the same bytes give the same key. A `DisplayReactor` writes each `opt` and variant in one form, whichever it was given in, so an `opt` given bare or as `[value]`, none given as `null`, `undefined` or `[]`, and a variant given with or without `_type` give the same key. A `vec record { text; T }` given to it as an object is written as its entries in order, since the order is part of what it sends. Fields a record does not declare are left out and every value of `reserved` is written as `null`, since neither is sent, and a `DisplayReactor` writes a float or an integer of 32 bits or fewer given as text as its number, and a `Principal` as its text. An argument the reactor refuses, such as `undefined` where Candid `null` is required or a bigint where a `DisplayReactor` takes text, is written behind a tag of its own, so the call fails instead of being answered from the cache entry of an argument it takes. Args are not spread into separate elements, and `args: []` still produces the literal `"[]"`. - **`...queryKey`** — a custom `queryKey` is spread onto the end; it never replaces the identity prefix. Use this for: diff --git a/packages/core/README.md b/packages/core/README.md index 202e661f5..cdd8d1e29 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -526,7 +526,14 @@ bytes give the same key. A `DisplayReactor` writes each `opt` and variant in one form, whichever it was given in, so an `opt` given bare or as `[value]`, none given as `null`, `undefined` or `[]`, and a variant given with or without `_type` give the same key. A `vec record { text; T }` given to it as an object is written as -its entries in order, since the order is part of what it sends. The +its entries in order, since the order is part of what it sends. Fields a record +does not declare are left out and every value of `reserved` is written as +`null`, since neither is sent, and a `DisplayReactor` writes a float or an +integer of 32 bits or fewer given as text as its number, and a `Principal` as +its text. An argument the reactor refuses, such as `undefined` where Candid +`null` is required or a bigint where a `DisplayReactor` takes text, is written +behind a tag of its own, so the call fails instead of being answered from the +cache entry of an argument it takes. The `{ effectiveTarget }` segment is dropped when it names the same canister the key is already rooted at, and any custom `queryKey` is appended element-wise. Build keys with `generateQueryKey` (or a query object's diff --git a/packages/core/src/display-reactor.ts b/packages/core/src/display-reactor.ts index 098fdfdea..a264481c1 100644 --- a/packages/core/src/display-reactor.ts +++ b/packages/core/src/display-reactor.ts @@ -19,7 +19,12 @@ import { } from "./types/reactor.js" import { extractOkResult } from "./utils/helper.js" import { ArgsKeyVisitor } from "./utils/args-key.js" -import { isOptionalWrapper, isTextKeyedPair } from "./display/visitor.js" +import { + isDisplayPrincipal, + isOptionalWrapper, + isTextKeyedPair, + numberOfText, +} from "./display/visitor.js" import { CanisterError, ValidationError } from "./errors/index.js" import { DisplayReactorParameters, @@ -54,6 +59,8 @@ function methodDisplayCodecs(methodType: IDL.Type): { const displayArgsKey = new ArgsKeyVisitor({ isOptionalWrapper, isTextKeyedPair, + numberOfText, + isPrincipal: isDisplayPrincipal, }) // ============================================================================ @@ -413,8 +420,11 @@ export class DisplayReactor< * bytes, as a Reactor keys it. An opt given bare, wrapped or as any form of * none, and a variant with or without its `_type`, are keyed in one form, * and a `vec record { text; T }` given as an object by its entries in the - * order they are sent. A method without a codec sends its args to IDL.encode - * unchanged, so they are read as a Reactor's are. + * order they are sent. A float or an integer of 32 bits or fewer given as + * text is keyed as its number, and a Principal as its text. A value the + * codecs refuse is keyed behind a tag, apart from every value they take. A + * method without a codec sends its args to IDL.encode unchanged, so they are + * read as a Reactor's are. */ protected argsForQueryKey>( functionName: M, diff --git a/packages/core/src/display/visitor.ts b/packages/core/src/display/visitor.ts index 4c3c95a93..02d2fcb84 100644 --- a/packages/core/src/display/visitor.ts +++ b/packages/core/src/display/visitor.ts @@ -159,42 +159,100 @@ export function isOptionalWrapper( return !elemIsArrayValued || couldBeDisplayOf(elemType, inner) } -function createFixedNumberCodec(bits: number, signed: boolean): z.ZodTypeAny { +const NAT_TEXT = /^\d+$/ +const INT_TEXT = /^-?\d+$/ + +const invalidFixed = (bits: number, signed: boolean, expected: string) => + `[ic-reactor] Invalid ${signed ? "int" : "nat"}${bits} display value: expected ${expected}` + +/** + * The number a display value of a fixed-width integer of 32 bits or fewer + * sends: the number itself, or the number integer text spells. Throws for a + * value the codec refuses. + */ +function fixedNumberOf( + bits: number, + signed: boolean, + val: string | number +): number { const min = signed ? -(2 ** (bits - 1)) : 0 const max = signed ? 2 ** (bits - 1) - 1 : 2 ** bits - 1 - const integerPattern = signed ? /^-?\d+$/ : /^\d+$/ - const typeName = `${signed ? "int" : "nat"}${bits}` + const num = typeof val === "string" ? Number(val) : val - const parseDisplayNumber = (val: string | number): number => { - const num = typeof val === "string" ? Number(val) : val + if (typeof val === "string" && !(signed ? INT_TEXT : NAT_TEXT).test(val)) { + throw new TypeError( + `${invalidFixed(bits, signed, "an integer string")}, got "${val}"` + ) + } - if (typeof val === "string" && !integerPattern.test(val)) { - throw new TypeError( - `[ic-reactor] Invalid ${typeName} display value: expected an integer string, got "${val}"` - ) - } + if (!Number.isInteger(num)) { + throw new TypeError( + `${invalidFixed(bits, signed, "an integer")}, got ${String(val)}` + ) + } - if (!Number.isInteger(num)) { - throw new TypeError( - `[ic-reactor] Invalid ${typeName} display value: expected an integer, got ${String(val)}` - ) - } + if (num < min || num > max) { + throw new RangeError( + `${invalidFixed(bits, signed, `${min}..${max}`)}, got ${String(val)}` + ) + } - if (num < min || num > max) { - throw new RangeError( - `[ic-reactor] Invalid ${typeName} display value: expected ${min}..${max}, got ${String(val)}` - ) - } + return num +} - return num +/** + * The number a display value of a float sends: the number itself, or the + * number text spells. Throws for a value the codec refuses. + */ +function floatNumberOf(bits: number, val: string | number): number { + const trimmed = typeof val === "string" ? val.trim() : undefined + if (trimmed === "") { + throw new TypeError( + `[ic-reactor] Invalid float${bits} display value: expected a number, got ""` + ) + } + const num = trimmed === undefined ? (val as number) : Number(trimmed) + // A finite double can still overflow float32: IDL.encode narrows + // 3.4028236e38 to Infinity and sends that, so check the narrowed + // value for float32, not just the double. + const narrowed = bits === 32 ? Math.fround(num) : num + if (!Number.isFinite(narrowed)) { + throw new TypeError( + `[ic-reactor] Invalid float${bits} display value: expected a finite float${bits}, got ${String(val)}` + ) } + return num +} + +/** + * The number the codec of a float, or of an integer of 32 bits or fewer, + * sends for display `text`. Throws for text the codec refuses. The query key + * reads numeric text with this, so it cannot drift from what the codec sends. + */ +export function numberOfText( + type: IDL.FixedNatClass | IDL.FixedIntClass | IDL.FloatClass, + text: string +): number { + return type instanceof IDL.FloatClass + ? floatNumberOf(type._bits, text) + : fixedNumberOf(type._bits, type instanceof IDL.FixedIntClass, text) +} + +/** Is `value` a Principal the principal codec takes as it is? */ +export function isDisplayPrincipal(value: unknown): value is Principal { + return value instanceof Principal +} + +function createFixedNumberCodec(bits: number, signed: boolean): z.ZodTypeAny { + const min = signed ? -(2 ** (bits - 1)) : 0 + const max = signed ? 2 ** (bits - 1) - 1 : 2 ** bits - 1 return z.codec( z.number().int().min(min).max(max), // Candid format z.union([z.number(), z.string()]), // Display format { decode: (val) => val, - encode: parseDisplayNumber, + encode: (val) => fixedNumberOf(bits, signed, val), } ) } @@ -325,7 +383,7 @@ export class DisplayCodecVisitor extends IDL.Visitor { // as a string (the visitors in @ic-reactor/candid emit "" and a string // schema for float32/float64), so a value that passed the form's own // validation must encode here too. Same contract as the ≤32-bit integers. - const typeName = `float${t._bits}` + // // NaN, Infinity and -Infinity are valid float32/float64 values, and // IDL.decode returns them as numbers. Zod 4's z.number() rejects all three, // so one of them in a result failed the decode of the whole response and @@ -337,24 +395,7 @@ export class DisplayCodecVisitor extends IDL.Visitor { z.union([anyNumber, z.string()]), // Display format { decode: (val) => val, - encode: (val) => { - const num = typeof val === "string" ? Number(val.trim()) : val - if (typeof val === "string" && val.trim() === "") { - throw new TypeError( - `[ic-reactor] Invalid ${typeName} display value: expected a number, got ""` - ) - } - // A finite double can still overflow float32: IDL.encode narrows - // 3.4028236e38 to Infinity and sends that, so check the narrowed - // value for float32, not just the double. - const narrowed = t._bits === 32 ? Math.fround(num) : num - if (!Number.isFinite(narrowed)) { - throw new TypeError( - `[ic-reactor] Invalid ${typeName} display value: expected a finite ${typeName}, got ${String(val)}` - ) - } - return num - }, + encode: (val) => floatNumberOf(t._bits, val), } ) } diff --git a/packages/core/src/reactor.ts b/packages/core/src/reactor.ts index 68213856f..afc3ccef4 100644 --- a/packages/core/src/reactor.ts +++ b/packages/core/src/reactor.ts @@ -326,8 +326,11 @@ export class Reactor { /** * The args as the query key records them: each blob the method's Candid * type declares is keyed by its bytes, so a `Uint8Array` and a `number[]` - * holding the same bytes get one key. Every other value is unchanged. A - * subclass whose `transformArgs` takes other shapes reads them here too. + * holding the same bytes get one key. A record's undeclared fields are left + * out and a `reserved` value is keyed as `null`, since neither is sent, and + * a value IDL.encode refuses is keyed behind a tag, apart from every value + * it takes. Every other value is unchanged. A subclass whose + * `transformArgs` takes other shapes reads them here too. */ protected argsForQueryKey>( functionName: M, diff --git a/packages/core/src/utils/args-key.ts b/packages/core/src/utils/args-key.ts index 779493ca0..c1ca716fb 100644 --- a/packages/core/src/utils/args-key.ts +++ b/packages/core/src/utils/args-key.ts @@ -25,6 +25,24 @@ import { IDL } from "@icp-sdk/core/candid" * object's keys, so two orders of one map, which send different vectors, * shared a key. The key lists the entries in order, as the pairs they are * sent as. + * - In a DisplayReactor, a float or an integer of 32 bits or fewer given as + * numeric text, and a `Principal` given as the object. The key writes the + * number the text spells and the principal's text, the forms the codecs + * return. And in both reactors, any value of `reserved`, which sends + * nothing: the key writes `null`, the value `reserved` decodes to. + * + * The JSON of a value the reactor refuses can also be the JSON of one it + * takes, so the refused call was answered from the other's cache entry + * instead of failing. `undefined` where Candid `null` is required is written + * as `null` in an array; a bigint is written as its decimal text (#515), the + * key of that text for a `text` or for a DisplayReactor's number; text given + * to a Reactor's integer is the key of the bigint it spells; a plain + * `{ __principal__ }` object, which is what JSON.parse returns for a + * Principal, is the key of the Principal; and a Reactor's variant with an + * `undefined` beside its arm is the key of the arm alone. The key checks each + * value of a primitive type, each record, and each variant a Reactor sends, as + * the codec or IDL.encode does, and writes one they refuse as a + * {@link RefusedKey}, behind a tag no value they take can produce. * * Only a position the method's Candid type names is rewritten: a `number[]` is * a blob as a `vec nat8`, and the same array passed as a `vec nat16` keeps the @@ -39,6 +57,11 @@ export class BlobKey { constructor(readonly hex: string) {} } +/** An argument value the reactor refuses, as the query key records it. */ +export class RefusedKey { + constructor(readonly value: unknown) {} +} + /** * The shapes a DisplayReactor's codecs take that IDL.encode does not. Passed * in by the DisplayReactor rather than imported here, so a Reactor does not @@ -49,6 +72,16 @@ export interface DisplayArgShapes { isOptionalWrapper(elemType: IDL.Type, inner: unknown): boolean /** Is `type` a `record { text; T }`, whose vector is also taken as an object? */ isTextKeyedPair(type: IDL.Type): type is IDL.TupleClass + /** + * The number the codec of a float, or of an integer of 32 bits or fewer, + * sends for `text`. Throws for text the codec refuses. + */ + numberOfText( + type: IDL.FixedNatClass | IDL.FixedIntClass | IDL.FloatClass, + text: string + ): number + /** Is `value` a Principal the principal codec takes as it is? */ + isPrincipal(value: unknown): value is { toText(): string } } const isPlainObject = (value: unknown): value is Record => { @@ -71,6 +104,22 @@ const isOpt = (type: IDL.Type | undefined): boolean => type instanceof IDL.OptClass || (type instanceof IDL.RecClass && isOpt(type.getType())) +/** Is `type` `reserved`, behind any recursive alias? */ +const isReserved = (type: IDL.Type | undefined): boolean => + type instanceof IDL.ReservedClass || + (type instanceof IDL.RecClass && isReserved(type.getType())) + +/** An integer type a DisplayReactor takes as a number or as text. */ +const isSmallInteger = ( + type: IDL.Type +): type is IDL.FixedNatClass | IDL.FixedIntClass => + (type instanceof IDL.FixedNatClass || type instanceof IDL.FixedIntClass) && + type._bits <= 32 + +/** Does IDL.encode take `value` as a principal? It asks nothing more. */ +const isPrincipalLike = (value: unknown): boolean => + Boolean(value && (value as { _isPrincipal?: unknown })._isPrincipal) + /** * Can a value of `type` itself be null: an `opt`'s none, `null`, or * `reserved`, which takes any value? Behind any recursive alias. @@ -139,6 +188,25 @@ function reaches( return false } +/** Sets `label` on `record`, a field named `__proto__` included. */ +function setField( + record: Record, + label: string, + value: unknown +): void { + if (label !== "__proto__") { + record[label] = value + return + } + // Defined, not assigned, so that a field named `__proto__` stays a field. + Object.defineProperty(record, label, { + value, + enumerable: true, + writable: true, + configurable: true, + }) +} + /** `items` with `map` applied, or `items` itself when nothing changed. */ function mapItems( items: unknown[], @@ -154,8 +222,10 @@ function mapItems( /** * Walks an argument alongside its Candid type and writes each value that has - * more than one form in one form: a blob as a {@link BlobKey}, and in a - * DisplayReactor also an opt, a variant and a vector given as an object. It + * more than one form in one form: a blob as a {@link BlobKey}, a record + * without its undeclared fields, `reserved` as `null`, and in a DisplayReactor + * also an opt, a variant, a vector given as an object, numeric text and a + * Principal. A value the reactor refuses it writes as a {@link RefusedKey}. It * reads the value the way the reactor encodes it: as IDL.encode does, or, * given the display shapes, as a DisplayReactor's codecs do. Everything that * holds none of these, and every value already in that form, it returns as the @@ -194,14 +264,36 @@ export class ArgsKeyVisitor extends IDL.Visitor { return rewrites } - /** Does the key write a value of `type` itself in a form of its own? */ + /** + * Does the key check a value of `type` itself, or write it in a form of its + * own? A float is checked only in a DisplayReactor: a Reactor takes it only + * as a number, and no value it refuses there has a number's JSON. + */ private isRewritten(type: IDL.Type): boolean { - if (isBlob(type)) return true - if (!this.display) return false - return ( - type instanceof IDL.OptClass || + if ( + isBlob(type) || + type instanceof IDL.NullClass || + type instanceof IDL.BoolClass || + type instanceof IDL.TextClass || + type instanceof IDL.NatClass || + type instanceof IDL.IntClass || + type instanceof IDL.FixedNatClass || + type instanceof IDL.FixedIntClass || + type instanceof IDL.PrincipalClass || + type instanceof IDL.ReservedClass || type instanceof IDL.VariantClass || - (type instanceof IDL.VecClass && this.display.isTextKeyedPair(type._type)) + // For the fields it does not declare. A tuple is a RecordClass too, but + // it is an array. + (type instanceof IDL.RecordClass && !(type instanceof IDL.TupleClass)) + ) { + return true + } + return ( + !!this.display && + (type instanceof IDL.FloatClass || + type instanceof IDL.OptClass || + (type instanceof IDL.VecClass && + this.display.isTextKeyedPair(type._type))) ) } @@ -225,40 +317,144 @@ export class ArgsKeyVisitor extends IDL.Visitor { : undefined } + /** + * `value` with each of `fields` it has rewritten. With `onlyDeclared`, the + * fields a record does not declare are left out as well: neither IDL.encode + * nor the record codec sends them. + */ private mapFields( value: Record, - fields: ReadonlyArray<[string, IDL.Type]> + fields: ReadonlyArray<[string, IDL.Type]>, + onlyDeclared = false ): Record { let copy: Record | undefined + let declared = 0 for (const [label, type] of fields) { - if (!hasOwn(value, label) || !this.rewrites(type)) continue - const field = type.accept(this, value[label]) - if (Object.is(field, value[label])) continue + let field: unknown + if (hasOwn(value, label)) { + declared++ + if (!this.rewrites(type)) continue + field = type.accept(this, value[label]) + if (Object.is(field, value[label])) continue + } else if (this.display && isReserved(type)) { + // The record codec reads an absent field as undefined, which + // `reserved` takes. IDL.encode refuses the absent field itself. + field = null + } else { + continue + } copy ??= { ...value } - // Defined, not assigned, so that a field named `__proto__` stays a field. - Object.defineProperty(copy, label, { - value: field, - enumerable: true, - writable: true, - configurable: true, - }) + setField(copy, label, field) + } + if (onlyDeclared && Object.keys(value).length > declared) { + const source = copy ?? value + const record: Record = {} + for (const [label] of fields) { + if (hasOwn(source, label)) setField(record, label, source[label]) + } + return record } return copy ?? value } - private mapArm( - value: Record, - fields: Array<[string, IDL.Type]>, - tag: string - ): Record { - const arm = fields.find(([label]) => label === tag) - return arm ? this.mapFields(value, [arm]) : value - } - visitType(_t: IDL.Type, value: unknown): unknown { return value } + // Each primitive is checked by its JavaScript type, as the codec or + // IDL.encode checks it first. A value of another type is refused whatever + // else is true of it. A value of the right type that is still refused, such + // as -1 for a `nat` or "abc" for a DisplayReactor's `nat`, keeps its key: no + // value taken there has the same JSON. + + visitNull(_t: IDL.NullClass, value: unknown): unknown { + return value === null ? value : new RefusedKey(value) + } + + visitBool(_t: IDL.BoolClass, value: unknown): unknown { + return typeof value === "boolean" ? value : new RefusedKey(value) + } + + visitText(_t: IDL.TextClass, value: unknown): unknown { + return typeof value === "string" ? value : new RefusedKey(value) + } + + visitNat(t: IDL.NatClass, value: unknown): unknown { + return this.keyInteger(t, value) + } + + visitInt(t: IDL.IntClass, value: unknown): unknown { + return this.keyInteger(t, value) + } + + visitFixedNat(t: IDL.FixedNatClass, value: unknown): unknown { + return this.keyInteger(t, value) + } + + visitFixedInt(t: IDL.FixedIntClass, value: unknown): unknown { + return this.keyInteger(t, value) + } + + visitFloat(t: IDL.FloatClass, value: unknown): unknown { + return this.display ? this.keyDisplayNumber(t, value) : value + } + + visitPrincipal(_t: IDL.PrincipalClass, value: unknown): unknown { + if (!this.display) { + return isPrincipalLike(value) ? value : new RefusedKey(value) + } + // The principal codec takes text, which it returns, and a Principal. + if (typeof value === "string") return value + return this.display.isPrincipal(value) + ? value.toText() + : new RefusedKey(value) + } + + visitReserved(_t: IDL.ReservedClass, _value: unknown): unknown { + // `reserved` takes any value and sends none. + return null + } + + private keyInteger( + t: IDL.NatClass | IDL.IntClass | IDL.FixedNatClass | IDL.FixedIntClass, + value: unknown + ): unknown { + if (!this.display) { + // IDL.encode takes a bigint or a number. + return typeof value === "bigint" || typeof value === "number" + ? value + : new RefusedKey(value) + } + if (isSmallInteger(t)) return this.keyDisplayNumber(t, value) + // The codec of `nat`, `int` and the 64-bit integers takes only text. + return typeof value === "string" ? value : new RefusedKey(value) + } + + /** + * A float or an integer of 32 bits or fewer in a DisplayReactor, whose + * codec takes a number or text and returns a number. Text is keyed as the + * number it sends, and an integer's -0 as the 0 it sends. + */ + private keyDisplayNumber( + t: IDL.FixedNatClass | IDL.FixedIntClass | IDL.FloatClass, + value: unknown + ): unknown { + let number: number + if (typeof value === "number") { + number = value + } else if (typeof value !== "string") { + return new RefusedKey(value) + } else { + try { + number = this.display!.numberOfText(t, value) + } catch { + // Refused text keeps its key, which no number has. + return value + } + } + return number === 0 && !(t instanceof IDL.FloatClass) ? 0 : number + } + visitRec( _t: IDL.RecClass, ty: IDL.ConstructType, @@ -279,8 +475,15 @@ export class ArgsKeyVisitor extends IDL.Visitor { // A DisplayReactor also takes a `vec record { text; T }` as an object keyed // by the text, and sends `Object.entries` of it: the pairs in the object's // order. The key lists the same pairs in the same order, so it matches the - // vector sent, and the array of pairs, which sends the same one. - if (this.display?.isTextKeyedPair(elemType) && isPlainObject(value)) { + // vector sent, and the array of pairs, which sends the same one. The codec + // takes every object that is not an array so, a boxed `true` included, + // whose JSON is that of the `true` it refuses. + if ( + this.display?.isTextKeyedPair(elemType) && + typeof value === "object" && + value !== null && + !Array.isArray(value) + ) { const pairs: unknown[] = Object.entries(value) return this.rewrites(elemType) ? pairs.map((pair) => elemType.accept(this, pair)) @@ -334,7 +537,24 @@ export class ArgsKeyVisitor extends IDL.Visitor { fields: Array<[string, IDL.Type]>, value: unknown ): unknown { - return isPlainObject(value) ? this.mapFields(value, fields) : value + // IDL.encode takes only an object, and `null` only for a record without + // fields, where it takes any object: a boxed `true` or a Date, whose JSON + // is that of the `true` or the text it refuses. The record codec passes + // any other value to it as it is. + if (typeof value !== "object") return new RefusedKey(value) + if (value === null) return fields.length > 0 ? new RefusedKey(value) : value + if (!isPlainObject(value)) return value + // IDL.encode calls the record's own `hasOwnProperty` for each field, which + // an object without Object.prototype does not have. The record codec + // reads its fields another way. + if ( + !this.display && + fields.length > 0 && + Object.getPrototypeOf(value) === null + ) { + return new RefusedKey(value) + } + return this.mapFields(value, fields, true) } visitTuple( @@ -357,8 +577,16 @@ export class ArgsKeyVisitor extends IDL.Visitor { ): unknown { if (!isPlainObject(value)) return value if (!this.display) { + // IDL.encode takes one own key, `undefined` or not, which names an arm, + // and calls the value's own `hasOwnProperty`. JSON leaves out a key whose + // value is undefined, so `{ A: 1, B: undefined }`, which it refuses, had + // the key of `{ A: 1 }`. const tags = Object.keys(value) - return tags.length === 1 ? this.mapArm(value, fields, tags[0]) : value + const arm = + tags.length === 1 && Object.getPrototypeOf(value) !== null + ? fields.find(([label]) => label === tags[0]) + : undefined + return arm ? this.mapFields(value, [arm]) : new RefusedKey(value) } // The variant codec names the arm in `_type`, or else by the one key the // value has. @@ -375,11 +603,12 @@ export class ArgsKeyVisitor extends IDL.Visitor { if (!arm) return value const [label, type] = arm const payload = hasOwn(value, label) ? value[label] : undefined - // What the codec sends: nothing for a null arm, whatever the payload, and - // nothing for a missing payload unless the arm is an opt, whose none that - // is. + // What the codec sends: nothing for a null or a reserved arm, whatever the + // payload, and nothing for a missing payload unless the arm is an opt, + // whose none that is. const sent = !(type instanceof IDL.NullClass) && + !isReserved(type) && ((payload !== null && payload !== undefined) || isOpt(type)) const key = !sent ? undefined diff --git a/packages/core/src/utils/helper.ts b/packages/core/src/utils/helper.ts index ed98abcf3..77fa71d6c 100644 --- a/packages/core/src/utils/helper.ts +++ b/packages/core/src/utils/helper.ts @@ -1,5 +1,5 @@ import { LOCAL_HOSTS, REMOTE_HOSTS } from "./constants.js" -import { BlobKey } from "./args-key.js" +import { BlobKey, RefusedKey } from "./args-key.js" import { CanisterError } from "../errors/index.js" import { OkResult } from "../types/index.js" @@ -10,12 +10,13 @@ const isPlainObject = (value: unknown): value is Record => { } /** - * Leads the key of a float that JSON has no number for, and of a blob. A - * string that already starts with it gets one more in front, so the count of - * leading U+0000s tells the cases apart: none for any other string (or a - * BigInt), exactly one for a tagged float or blob, two or more for a string - * that began with U+0000. No string argument can therefore serialise to a - * tag, and no other JSON type serialises to a string at all. + * Leads the key of a float that JSON has no number for, of a blob, and of a + * value the reactor refuses. A string that already starts with it gets one + * more in front, so the count of leading U+0000s tells the cases apart: none + * for any other string (or a BigInt), exactly one for a tagged float, blob or + * refused value, two or more for a string that began with U+0000. No string + * argument can therefore serialise to a tag, and no other JSON type + * serialises to a string at all. */ const SPECIAL_NUMBER_TAG = "\u0000" @@ -44,14 +45,21 @@ const SPECIAL_NUMBER_TAG = "\u0000" * `"\u0000blob:"` and its lowercase hex. It carries the same tag, so no * argument can produce it: not hex text given to a Reactor, which refuses it, * and not a BigInt, whose digits are also hex. + * - A value the reactor refuses can have the JSON of one it takes: `undefined` + * is written as `null` in an array, and a BigInt as its digits. The key hands + * such a value over as a `RefusedKey`, written here as `"\u0000refused:"` and + * the key of the value alone, behind the same tag. * * BigInts are written as decimal strings. Everything else — including every * string that does not start with U+0000, and an object whose keys are already * in sorted order — serialises exactly as before. */ -export const generateKey = (args: any[]) => { +export const generateKey = (args: any[]): string => { return JSON.stringify(args, (_, v: unknown) => { if (v instanceof BlobKey) return `${SPECIAL_NUMBER_TAG}blob:${v.hex}` + if (v instanceof RefusedKey) { + return `${SPECIAL_NUMBER_TAG}refused:${generateKey([v.value])}` + } if (typeof v === "string") { return v.startsWith(SPECIAL_NUMBER_TAG) ? SPECIAL_NUMBER_TAG + v : v } diff --git a/packages/core/tests/reactor-refused-args-query-key.test.ts b/packages/core/tests/reactor-refused-args-query-key.test.ts new file mode 100644 index 000000000..89e9eca6c --- /dev/null +++ b/packages/core/tests/reactor-refused-args-query-key.test.ts @@ -0,0 +1,415 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import { QueryClient } from "@tanstack/query-core" +import { QueryResponseStatus } from "@icp-sdk/core/agent" +import { IDL } from "@icp-sdk/core/candid" +import { Principal } from "@icp-sdk/core/principal" +import { ClientManager } from "../src/client.js" +import { Reactor } from "../src/reactor.js" +import { DisplayReactor } from "../src/display-reactor.js" +import { generateKey } from "../src/utils/helper.js" + +/** + * The args segment of a query key is the JSON of the arguments, and JSON + * writes some values a reactor refuses exactly as it writes values the + * reactor takes (#765): + * + * - `undefined` where Candid `null` is required: JSON writes it as `null`. + * - A bigint, which the key writes as its decimal text (#515): the key of + * that text for a `text`, and for a DisplayReactor's numbers, whose codecs + * take text and refuse a bigint. + * - Text for a Reactor's integer: the key of the bigint it spells. + * - `{ __principal__: "..." }`, which is what JSON.parse returns for a + * Principal: the key of the Principal. + * - A Reactor's variant with an `undefined` beside its arm: JSON leaves the + * `undefined` out, and IDL.encode refuses a variant with two keys. + * + * With the valid call cached, the refused one was answered from the cache by + * `fetchQuery` or a query hook instead of failing. + * + * Some forms that send the same bytes also got keys of their own: text for a + * DisplayReactor's small integer or float, a Principal for its text, fields a + * record does not declare, and values of `reserved`. + */ + +const CANISTER_ID = "ryjl3-tyaaa-aaaaa-aaaba-cai" +const OWNER_TEXT = "aaaaa-aa" +const OWNER = Principal.fromText(OWNER_TEXT) + +const Account = IDL.Record({ + owner: IDL.Principal, + subaccount: IDL.Opt(IDL.Vec(IDL.Nat8)), +}) +const Filter = IDL.Variant({ + All: IDL.Null, + ByOwner: IDL.Principal, + ByMemo: IDL.Opt(IDL.Text), +}) + +const idlFactory: IDL.InterfaceFactory = ({ IDL }) => + IDL.Service({ + ping: IDL.Func([IDL.Null], [IDL.Nat], ["query"]), + stats: IDL.Func([IDL.Record({})], [IDL.Nat], ["query"]), + greet: IDL.Func([IDL.Text], [IDL.Nat], ["query"]), + balance: IDL.Func([IDL.Nat], [IDL.Nat], ["query"]), + page: IDL.Func([IDL.Nat32], [IDL.Nat], ["query"]), + offset: IDL.Func([IDL.Int32], [IDL.Nat], ["query"]), + ratio: IDL.Func([IDL.Float64], [IDL.Nat], ["query"]), + owner_of: IDL.Func([IDL.Principal], [IDL.Nat], ["query"]), + balance_of: IDL.Func([Account], [IDL.Nat], ["query"]), + search: IDL.Func([Filter], [IDL.Nat], ["query"]), + scores: IDL.Func( + [IDL.Vec(IDL.Tuple(IDL.Text, IDL.Nat))], + [IDL.Nat], + ["query"] + ), + // `reserved` stands for a field or an arm an interface has retired. + legacy: IDL.Func( + [IDL.Record({ id: IDL.Nat, old: IDL.Reserved })], + [IDL.Nat], + ["query"] + ), + pick: IDL.Func( + [IDL.Variant({ A: IDL.Reserved, B: IDL.Reserved })], + [IDL.Nat], + ["query"] + ), + }) + +let queryClient: QueryClient +let clientManager: ClientManager +/** The Candid argument bytes of each query the agent was asked to send. */ +let sent: string[] + +beforeEach(() => { + // No retries: a call that fails should fail at once. + queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }) + clientManager = new ClientManager({ + queryClient, + agentOptions: { host: "https://icp-api.io" }, + }) + sent = [] + // Each reply is the number of calls so far. + vi.spyOn(clientManager.agent, "query").mockImplementation((async ( + _canisterId: Principal, + { arg }: { arg: Uint8Array } + ) => { + sent.push(Array.from(arg).join(",")) + return { + status: QueryResponseStatus.Replied, + reply: { arg: IDL.encode([IDL.Nat], [BigInt(sent.length)]) }, + } + }) as never) +}) + +type Kind = "Reactor" | "DisplayReactor" + +const makeReactor = (kind: Kind) => + new (kind === "Reactor" ? Reactor : DisplayReactor)({ + clientManager, + name: kind, + canisterId: CANISTER_ID, + idlFactory, + }) + +type AnyReactor = ReturnType + +const keyOf = (reactor: AnyReactor, functionName: string, args: unknown[]) => + reactor.generateQueryKey({ + functionName: functionName as never, + args: args as never, + }) + +/** The args segment: the last element of a key built with args. */ +const argsSegment = (reactor: AnyReactor, fn: string, args: unknown[]) => { + const key = keyOf(reactor, fn, args) + return key[key.length - 1] +} + +/** The distinct keys among `forms`, each passed as the only argument. */ +const distinctKeys = (reactor: AnyReactor, fn: string, forms: unknown[]) => + new Set(forms.map((form) => JSON.stringify(keyOf(reactor, fn, [form])))) + +/** The bytes each form sends, one call each. */ +const sentBy = async (reactor: AnyReactor, fn: string, forms: unknown[]) => { + const before = sent.length + for (const form of forms) { + await reactor.callMethod({ + functionName: fn as never, + args: [form] as never, + }) + } + return sent.slice(before) +} + +/** What `fetchQuery` returns for each form, in turn. */ +const fetchEach = async (reactor: AnyReactor, fn: string, forms: unknown[]) => { + const results: unknown[] = [] + for (const form of forms) { + results.push( + await reactor.fetchQuery({ + functionName: fn as never, + args: [form] as never, + }) + ) + } + return results +} + +/** A record without Object.prototype, as `Object.create(null)` makes. */ +const bare = (fields: object) => Object.assign(Object.create(null), fields) + +// [what is refused, reactor, method, a valid argument, the refused one] +const REFUSED: Array<[string, Kind, string, unknown, unknown]> = [ + ["undefined for null", "Reactor", "ping", null, undefined], + ["undefined for null", "DisplayReactor", "ping", null, undefined], + // IDL.encode takes `null` for a record without fields, but not `undefined`. + ["undefined for an empty record", "Reactor", "stats", null, undefined], + ["undefined for an empty record", "DisplayReactor", "stats", null, undefined], + ["a bigint for text", "Reactor", "greet", "10", 10n], + ["a bigint for text", "DisplayReactor", "greet", "10", 10n], + ["text for a nat", "Reactor", "balance", 10n, "10"], + ["a bigint for a nat", "DisplayReactor", "balance", "10", 10n], + ["a bigint for a nat32", "DisplayReactor", "page", "10", 10n], + ["a bigint for a float64", "DisplayReactor", "ratio", "10", 10n], + [ + "a parsed Principal for a principal", + "Reactor", + "owner_of", + OWNER, + JSON.parse(JSON.stringify(OWNER)), + ], + [ + "a parsed Principal for a principal", + "DisplayReactor", + "owner_of", + OWNER, + JSON.parse(JSON.stringify(OWNER)), + ], + [ + "a variant with an undefined beside its arm", + "Reactor", + "search", + { All: null }, + { All: null, ByMemo: undefined }, + ], + [ + "a record without Object.prototype", + "Reactor", + "balance_of", + { owner: OWNER, subaccount: [] }, + bare({ owner: OWNER, subaccount: [] }), + ], + [ + "a record without its reserved field", + "Reactor", + "legacy", + { id: 1n, old: undefined }, + { id: 1n }, + ], + // The map codec takes any object that is not an array as its entries: a + // Date has none. It refuses the Date's text, whose JSON is the Date's. + [ + "the text of a Date for a map", + "DisplayReactor", + "scores", + new Date(0), + new Date(0).toJSON(), + ], +] + +describe("a refused argument (#765)", () => { + it.each(REFUSED)( + "%s in a %s is not answered from the valid call's cache entry", + async (_what, kind, fn, valid, refused) => { + const reactor = makeReactor(kind) + // The premise: the reactor takes one and refuses the other. + await expect( + reactor.callMethod({ + functionName: fn as never, + args: [refused] as never, + }) + ).rejects.toThrow() + expect(sent).toHaveLength(0) + + await reactor.fetchQuery({ + functionName: fn as never, + args: [valid] as never, + }) + expect(sent).toHaveLength(1) + + await expect( + reactor.fetchQuery({ + functionName: fn as never, + args: [refused] as never, + }) + ).rejects.toThrow() + expect(distinctKeys(reactor, fn, [valid, refused]).size).toBe(2) + } + ) + + it("keeps apart two arms of a Reactor's variant whose payload is reserved", async () => { + // `{ A: undefined }` and `{ B: undefined }` send different arms. JSON + // leaves out the one key each has, so both were `{}`, and the second + // call was answered with the first one's result. + const reactor = makeReactor("Reactor") + expect( + await fetchEach(reactor, "pick", [{ A: undefined }, { B: undefined }]) + ).toEqual([1n, 2n]) + expect(new Set(sent).size).toBe(2) + }) + + it("writes a refused value behind a tag no argument can spell", () => { + for (const kind of ["Reactor", "DisplayReactor"] as const) { + const reactor = makeReactor(kind) + const [tag] = JSON.parse( + argsSegment(reactor, "greet", [10n]) as string + ) as [string] + expect(tag.startsWith("\u0000refused:")).toBe(true) + // The same text, given where text is taken, keeps a key of its own. + expect(distinctKeys(reactor, "greet", [10n, tag]).size).toBe(2) + } + }) +}) + +describe("forms of one value, which send the same bytes (#765)", () => { + it("gets one key for a DisplayReactor's small integer as a number or as text", async () => { + const reactor = makeReactor("DisplayReactor") + for (const [fn, forms] of [ + ["page", [10, "10", "010"]], + // The codec sends 0 for -0 and for "-0". + ["offset", [0, -0, "0", "-0", "00"]], + ] as const) { + expect(new Set(await sentBy(reactor, fn, [...forms])).size).toBe(1) + expect(distinctKeys(reactor, fn, [...forms]).size).toBe(1) + } + }) + + it("gets one key for a DisplayReactor's float as a number or as text", async () => { + const reactor = makeReactor("DisplayReactor") + const forms = [1.5, "1.5", " 1.5 ", "15e-1"] + expect(new Set(await sentBy(reactor, "ratio", forms)).size).toBe(1) + expect(distinctKeys(reactor, "ratio", forms).size).toBe(1) + // A float sends -0 as -0, so it stays apart from 0. + expect(new Set(await sentBy(reactor, "ratio", [-0, "-0"])).size).toBe(1) + expect(distinctKeys(reactor, "ratio", [-0, "-0"]).size).toBe(1) + expect(distinctKeys(reactor, "ratio", [-0, 0]).size).toBe(2) + }) + + it("gets one key for a principal as a Principal or as its text", async () => { + const reactor = makeReactor("DisplayReactor") + expect(await fetchEach(reactor, "owner_of", [OWNER, OWNER_TEXT])).toEqual([ + "1", + "1", + ]) + expect(sent).toHaveLength(1) + + const accounts = [{ owner: OWNER }, { owner: OWNER_TEXT }] + expect(new Set(await sentBy(reactor, "balance_of", accounts)).size).toBe(1) + expect(distinctKeys(reactor, "balance_of", accounts).size).toBe(1) + }) + + it("leaves out the fields a record does not declare", async () => { + for (const [kind, account] of [ + ["Reactor", { owner: OWNER, subaccount: [] }], + ["DisplayReactor", { owner: OWNER_TEXT }], + ] as const) { + const reactor = makeReactor(kind) + const forms = [account, { ...account, label: "savings" }] + expect(new Set(await sentBy(reactor, "balance_of", forms)).size).toBe(1) + expect(distinctKeys(reactor, "balance_of", forms).size).toBe(1) + } + }) + + it("gets one key for every value of reserved", async () => { + const reactor = makeReactor("Reactor") + const records = [ + { id: 1n, old: null }, + { id: 1n, old: 5 }, + { id: 1n, old: undefined }, + ] + expect(new Set(await sentBy(reactor, "legacy", records)).size).toBe(1) + expect(distinctKeys(reactor, "legacy", records).size).toBe(1) + expect( + distinctKeys(reactor, "pick", [{ A: null }, { A: 5 }, { A: undefined }]) + .size + ).toBe(1) + + // The record codec also takes the field absent. + const display = makeReactor("DisplayReactor") + const forms = [{ id: "1", old: null }, { id: "1", old: "x" }, { id: "1" }] + expect(new Set(await sentBy(display, "legacy", forms)).size).toBe(1) + expect(distinctKeys(display, "legacy", forms).size).toBe(1) + expect( + distinctKeys(display, "pick", [{ _type: "A" }, { A: 5 }, { A: null }]) + .size + ).toBe(1) + }) + + it("keeps distinct values distinct", () => { + const display = makeReactor("DisplayReactor") + expect(distinctKeys(display, "page", [10, "11", "100"]).size).toBe(3) + expect(distinctKeys(display, "ratio", [1.5, "1.25", "-1.5"]).size).toBe(3) + expect( + distinctKeys(display, "owner_of", [OWNER, CANISTER_ID, "2vxsx-fae"]).size + ).toBe(3) + expect( + distinctKeys(display, "legacy", [{ id: "1" }, { id: "2", old: null }]) + .size + ).toBe(2) + + const reactor = makeReactor("Reactor") + for (const label of [undefined, "savings"]) { + expect( + distinctKeys( + reactor, + "balance_of", + [ + { owner: OWNER, subaccount: [] }, + { owner: Principal.fromText(CANISTER_ID), subaccount: [] }, + { owner: OWNER, subaccount: [[1]] }, + ].map((account) => (label ? { ...account, label } : account)) + ).size + ).toBe(3) + } + expect(distinctKeys(reactor, "pick", [{ A: null }, { B: null }]).size).toBe( + 2 + ) + }) +}) + +describe("the key of an argument in the forms the codecs return", () => { + it("is byte-identical to the key before", () => { + const calls: Array<[Kind, string, unknown[]]> = [ + ["Reactor", "ping", [null]], + ["Reactor", "greet", ["10"]], + ["Reactor", "balance", [10n]], + ["Reactor", "page", [10]], + ["Reactor", "offset", [-5]], + ["Reactor", "ratio", [-0]], + ["Reactor", "owner_of", [OWNER]], + ["Reactor", "balance_of", [{ owner: OWNER, subaccount: [] }]], + ["Reactor", "search", [{ ByMemo: ["x"] }]], + ["Reactor", "search", [{ ByOwner: OWNER }]], + ["Reactor", "scores", [[["a", 1n]]]], + ["Reactor", "legacy", [{ id: 1n, old: null }]], + ["Reactor", "pick", [{ B: null }]], + ["DisplayReactor", "ping", [null]], + ["DisplayReactor", "greet", ["10"]], + ["DisplayReactor", "balance", ["10"]], + ["DisplayReactor", "page", [10]], + ["DisplayReactor", "offset", [-5]], + ["DisplayReactor", "ratio", [1.5]], + ["DisplayReactor", "owner_of", [OWNER_TEXT]], + ["DisplayReactor", "balance_of", [{ owner: OWNER_TEXT }]], + ["DisplayReactor", "search", [{ _type: "ByOwner", ByOwner: OWNER_TEXT }]], + ["DisplayReactor", "scores", [[["a", "1"]]]], + ["DisplayReactor", "legacy", [{ id: "1", old: null }]], + ["DisplayReactor", "pick", [{ _type: "B" }]], + ] + for (const [kind, fn, args] of calls) { + expect(argsSegment(makeReactor(kind), fn, args)).toBe(generateKey(args)) + } + }) +})