Skip to content

Commit b623f0d

Browse files
crussokrpeacock
crusso
and
krpeacock
authored
fix: make IDL/Candid decoding of options spec compliant (#981)
* add backtracking to opt decoding * basic tests of variant defaulting under opt * implement remaining variant decoding edge cases * test opt parsing of null and any edge cases * add test for non-opt cases Co-authored-by: Kaia Peacock <[email protected]>
1 parent b8d70cc commit b623f0d

File tree

4 files changed

+303
-9
lines changed

4 files changed

+303
-9
lines changed

‎docs/CHANGELOG.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,22 @@
11
# Changelog
22

3+
34
## [Unreleased]
45

6+
- fix: Bring Candid decoding of `opt` types up to Candid spec:
7+
In particular, when decoding at an `opt` type:
8+
- If the wire type is an `opt` type, decode its payload at the expected content type
9+
(as before).
10+
- Allow decoding `null` wire type as IDL value `null` (i.e. JS `[]`).
11+
- Allow decoding of value of `reserved` wire type, defaulting to IDL value `null` (i.e. JS `[]`).
12+
- Allow decoding of wider variant type on the wire at narrower expected variant type,
13+
provided the decoded value is valid at the expected variant type. Otherwise, default to `null` (i.e. JS `[]`).
14+
- Otherwise:
15+
- If the expected content type is `null` or `reserved` or (nested) `opt`, return IDL value `null` (i.e. JS `[]`).
16+
- The expected content type is neither `null`, `reserved` or nested `opt`:
17+
allow decoding of the non-optioned value `v` as `opt v` (JS `[v*]`) if compatible with
18+
the expected content type; if incompatible, return IDL value `null` (JS `[]`).
19+
520
### Added
621

722
- test: added e2e test for CanisterStatus requesting a subnet path, as a reference for getting the subnet id of a given canister id

‎packages/candid/src/idl.test.ts‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,3 +698,214 @@ test('should decode matching optional fields if wire type contains additional fi
698698
b: ['123'],
699699
});
700700
});
701+
702+
703+
describe('IDL opt variant decoding', () => {
704+
it('should handle matching expected and wire type variants', () => {
705+
testDecode(
706+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null, z: IDL.Null })) }),
707+
{ a: [{ x: null }] },
708+
'4449444c036c0161016e026b03787f797f7a7f01000100', // Motoko: {a = ?#x} : {a : ?{#x;#y;#z}},
709+
'same variant under opt x',
710+
);
711+
});
712+
it('should handle matching expected and wire type variants', () => {
713+
testDecode(
714+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null, z: IDL.Null })) }),
715+
{ a: [{ z: null }] },
716+
'4449444c036c0161016e026b03787f797f7a7f01000102', // Motoko: {a = ?#z} : {a : ?{#x;#y;#z}}
717+
'same variant under opt z',
718+
);
719+
});
720+
it('should handle wider variant with expected tag', () => {
721+
testDecode(
722+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })) }),
723+
{ a: [{ x: null }] },
724+
'4449444c036c0161016e026b03787f797f7a7f01000100', // Motoko: {a = ?#x} : {a : ?{#x;#y;#z}}
725+
'extended variant under opt expected tag',
726+
);
727+
});
728+
it('should handle wider variant with unexpected tag', () => {
729+
testDecode(
730+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })) }),
731+
{ a: [] },
732+
'4449444c036c0161016e026b03787f797f7a7f01000102', // Motoko: {a = ?#z} : {a : ?{#x;#y;#z}}
733+
'extended variant under opt unexpected tag - defaulting',
734+
);
735+
});
736+
});
737+
738+
describe('IDL opt edge cases', () => {
739+
it('should handle the option when the wire type is null type', () => {
740+
testDecode(
741+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null }))}),
742+
{a: []},
743+
'4449444c016c01617f0100',
744+
// Motoko: {a = null} : {a : null}
745+
'opt expected type null on wire',
746+
)});
747+
it('should handle the option when the wire type is reserved', () => {
748+
testDecode(
749+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null }))}),
750+
{a: []},
751+
'4449444c016c0161700100',
752+
// Motoko: {a = (): Any} : {a : Any}
753+
'opt expected type reserved on wire',
754+
)});
755+
it('should handle the option when the wire typ is non-optioned', () => {
756+
testDecode(
757+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null }))}),
758+
{a: [{x: null}]},
759+
`4449444c026c0161016b02787f797f010000`,
760+
// Motoko: {a = #x } : {a : {#x;#y}}
761+
'opt expected type non-opt on wire',
762+
)});
763+
it('should handle the option when the wire typ is an non-optioned wider variant with expected tag', () => {
764+
testDecode(
765+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null }))}),
766+
{a: [{x: null}]},
767+
`4449444c026c0161016b03787f797f7a7f010000`,
768+
// Motoko: {a = #x } : {a : {#x;#y;#z}}
769+
'opt expected, wire type non-opt, extended, with expected tag',
770+
)});
771+
it('should handle the option when the wire typ is an non-optioned wider variant with unexpected tag, defaulting', () => {
772+
testDecode(
773+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null }))}),
774+
{a: []},
775+
`4449444c026c0161016b03787f797f7a7f010002`,
776+
// Motoko: {a = #z} : {a : {#x;#y;#z}}
777+
'opt expected, wire type non-opt, extended, with unexpected tag, defaulting',
778+
)});
779+
it('should handle the option when the expected type is opt null, wire type is non-opt', () => {
780+
testDecode(
781+
IDL.Record({a: IDL.Opt(IDL.Variant({ a: IDL.Opt(IDL.Null) }))}),
782+
{a: []},
783+
`4449444c016c01617d010001`,
784+
// Motoko: {a = 1} : {a : Nat }
785+
'opt expected, wire type non-opt, other type - defaulting',
786+
)});
787+
it('should handle the option when the expected type is opt opt Nat, wire type is non-opt', () => {
788+
testDecode(
789+
IDL.Record({a: IDL.Opt(IDL.Variant({ a: IDL.Opt(IDL.Opt(IDL.Nat)) }))}),
790+
{a: []},
791+
`4449444c016c01617d010001`,
792+
// Motoko: {a = 1} : {a : Nat }
793+
'opt expected, wire type non-opt, other type - defaulting',
794+
)});
795+
it('should handle the option when the expected type is opt reserved, wire type is non-opt', () => {
796+
testDecode(
797+
IDL.Record({a: IDL.Opt(IDL.Reserved)}),
798+
{a: []},
799+
`4449444c016c01617d010001`,
800+
// Motoko: {a = 1} : {a : Nat }
801+
'opt expected, wire type non-opt, other type - defaulting',
802+
)});
803+
});
804+
805+
806+
807+
/* The following suites are similar to the previous two but require more decoding of a Text value following the optional value.
808+
Tests decoding resumed correctly after any skip
809+
*/
810+
811+
describe('IDL opt variant decoding (embedded)', () => {
812+
it('should handle matching expected and wire type variants', () => {
813+
testDecode(
814+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null, z: IDL.Null })), b : IDL.Text }),
815+
{ a: [{ x: null }], b: "abc" },
816+
'4449444c036c02610162716e026b03787f797f7a7f0100010003616263', // Motoko: {a = ?#x; b = "abc"} : {a : ?{#x;#y;#z}; b :Text},
817+
'same variant under opt x',
818+
);
819+
});
820+
it('should handle matching expected and wire type variants', () => {
821+
testDecode(
822+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null, z: IDL.Null })), b : IDL.Text }),
823+
{ a: [{ z: null }], b: "abc" },
824+
'4449444c036c02610162716e026b03787f797f7a7f0100010203616263', // Motoko: {a = ?#z; b = "abc"} : {a : ?{#x;#y;#z}; b : Text}
825+
'same variant under opt z',
826+
);
827+
});
828+
it('should handle wider variant with expected tag', () => {
829+
testDecode(
830+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b: IDL.Text }),
831+
{ a: [{ x: null }], b: "abc" },
832+
'4449444c036c02610162716e026b03787f797f7a7f0100010003616263', // Motoko: {a = ?#x; b = "abc"} : {a : ?{#x;#y;#z}; b : Text}
833+
'extended variant under opt expected tag',
834+
);
835+
});
836+
it('should handle wider variant with unexpected tag', () => {
837+
testDecode(
838+
IDL.Record({ a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b: IDL.Text }),
839+
{ a: [], b: "abc" },
840+
'4449444c036c02610162716e026b03787f797f7a7f0100010203616263', // Motoko: {a = ?#z; b = "abc"} : {a : ?{#x;#y;#z}; b : Text}
841+
'extended variant under opt unexpected tag - defaulting',
842+
);
843+
});
844+
});
845+
846+
describe('IDL opt edge cases (embedded)', () => {
847+
it('should handle the option when the wire type is null type', () => {
848+
testDecode(
849+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b : IDL.Text}),
850+
{a: [], b: "abc"},
851+
'4449444c016c02617f6271010003616263',
852+
// Motoko: {a = null; b = "abc"} : {a : null; b: Text}
853+
'opt expected type null on wire',
854+
)});
855+
it('should handle the option when the wire type is reserved', () => {
856+
testDecode(
857+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b : IDL.Text}),
858+
{a: [], b: "abc"},
859+
'4449444c016c0261706271010003616263',
860+
// Motoko: {a = (): Any; b = "abc"} : {a : Any; b : Text}
861+
'opt expected type reserved on wire',
862+
)});
863+
it('should handle the option when the wire typ is non-optioned', () => {
864+
testDecode(
865+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b : IDL.Text}),
866+
{a: [{x: null}], b: "abc"},
867+
`4449444c026c02610162716b02787f797f01000003616263`,
868+
// Motoko: {a = #x; b = "abc" } : {a : {#x;#y}; b : Text}
869+
'opt expected type non-opt on wire',
870+
)});
871+
it('should handle the option when the wire typ is an non-optioned wider variant with expected tag', () => {
872+
testDecode(
873+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b : IDL.Text}),
874+
{a: [{x: null}], b: "abc"},
875+
`4449444c026c02610162716b03787f797f7a7f01000003616263`,
876+
// Motoko: {a = #x; b = "abc" } : {a : {#x;#y;#z}; b : Text}
877+
'opt expected, wire type non-opt, extended, with expected tag',
878+
)});
879+
it('should handle the option when the wire typ is an non-optioned wider variant with unexpected tag, defaulting', () => {
880+
testDecode(
881+
IDL.Record({a: IDL.Opt(IDL.Variant({ x: IDL.Null, y: IDL.Null })), b : IDL.Text}),
882+
{a: [], b: "abc"},
883+
`4449444c026c02610162716b03787f797f7a7f01000203616263`,
884+
// Motoko: {a = #z; b = "abc"} : {a : Nat; b : Text}
885+
'opt expected, wire type non-opt, extended, with unexpected tag - defaulting',
886+
)});
887+
it('should handle the option when the expected type is opt null, wire type is non-opt', () => {
888+
testDecode(
889+
IDL.Record({a: IDL.Opt(IDL.Variant({ a: IDL.Opt(IDL.Null) })), b : IDL.Text}),
890+
{a: [], b: "abc"},
891+
`4449444c036c02610162716e026b03787f797f7a7f0100010203616263`,
892+
// Motoko: {a = 1; b = "abc"} : {a : Nat; b : Text }
893+
'opt expected, wire type non-opt, other type - defaulting',
894+
)});
895+
it('should handle the option when the expected type is opt opt Nat, wire type is non-opt', () => {
896+
testDecode(
897+
IDL.Record({a: IDL.Opt(IDL.Variant({ a: IDL.Opt(IDL.Opt(IDL.Nat)) })), b : IDL.Text}),
898+
{a: [], b: "abc"},
899+
`4449444c016c02617d627101000103616263`,
900+
// Motoko: {a = 1; b = "abc"} : {a : Nat; b : Text }
901+
'opt expected, wire type non-opt, other type - defaulting',
902+
)});
903+
it('should handle the option when the expected type is opt reserved, wire type is non-opt', () => {
904+
testDecode(
905+
IDL.Record({a: IDL.Opt(IDL.Reserved), b: IDL.Text}),
906+
{a: [], b: "abc"},
907+
`4449444c016c02617d627101000103616263`,
908+
// Motoko: {a = 1; b = "abc"} : {a : Nat; b : Text }
909+
'opt expected, wire type non-opt, other type - defaulting',
910+
)});
911+
});

‎packages/candid/src/idl.ts‎

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ export abstract class Type<T = any> {
215215
public abstract encodeType(typeTable: TypeTable): ArrayBuffer;
216216

217217
public abstract checkType(t: Type): Type;
218+
218219
public abstract decodeValue(x: Pipe, t: Type): T;
219220

220221
protected abstract _buildTypeTableImpl(typeTable: TypeTable): void;
@@ -227,6 +228,7 @@ export abstract class PrimitiveType<T = any> extends Type<T> {
227228
}
228229
return t;
229230
}
231+
230232
// eslint-disable-next-line @typescript-eslint/no-unused-vars
231233
public _buildTypeTableImpl(typeTable: TypeTable): void {
232234
// No type table encoding for Primitive types.
@@ -914,17 +916,68 @@ export class OptClass<T> extends ConstructType<[T] | []> {
914916
}
915917

916918
public decodeValue(b: Pipe, t: Type): [T] | [] {
917-
const opt = this.checkType(t);
918-
if (!(opt instanceof OptClass)) {
919-
throw new Error('Not an option type');
919+
if (t instanceof NullClass) {
920+
return []
920921
}
921-
switch (safeReadUint8(b)) {
922-
case 0:
922+
923+
if (t instanceof ReservedClass) {
924+
return []
925+
}
926+
927+
let wireType = t;
928+
// unfold wireType, if needed
929+
if (t instanceof RecClass) {
930+
const ty = t.getType();
931+
if (typeof ty === 'undefined') {
932+
throw new Error('type mismatch with uninitialized type');
933+
} else wireType = ty;
934+
}
935+
936+
if (wireType instanceof OptClass) {
937+
switch (safeReadUint8(b)) {
938+
case 0:
939+
return [];
940+
case 1: {
941+
// Save the current state of the Pipe `b` to allow rollback in case of an error
942+
const checkpoint = b.save();
943+
try {
944+
// Attempt to decode a value using the `_type` of the current instance
945+
const v = this._type.decodeValue(b, wireType._type);
946+
return [v];
947+
} catch (e : any) {
948+
// If an error occurs during decoding, restore the Pipe `b` to its previous state
949+
b.restore(checkpoint);
950+
// Skip the value at the current wire type to advance the Pipe `b` position
951+
const skipped = wireType._type.decodeValue(b, wireType._type);
952+
// Return an empty array to indicate a `none` value
953+
return [];
954+
}
955+
}
956+
default:
957+
throw new Error('Not an option value');
958+
}
959+
} else if
960+
// this check corresponds to `not (null <: <t>)` in the spec
961+
(this._type instanceof NullClass || this._type instanceof OptClass || this._type instanceof ReservedClass) {
962+
// null <: <t> :
963+
// skip value at wire type (to advance b) and return "null", i.e. []
964+
const skipped = wireType.decodeValue(b, wireType);
965+
return [];
966+
} else {
967+
// not (null <: t) :
968+
// try constituent type
969+
const checkpoint = b.save();
970+
try {
971+
const v = this._type.decodeValue(b, t)
972+
return [v];
973+
} catch (e : any) {
974+
// decoding failed, but this is opt, so return "null", i.e. []
975+
b.restore(checkpoint);
976+
// skip value at wire type (to advance b)
977+
const skipped = wireType.decodeValue(b, t)
978+
// return "null"
923979
return [];
924-
case 1:
925-
return [this._type.decodeValue(b, opt._type)];
926-
default:
927-
throw new Error('Not an option value');
980+
}
928981
}
929982
}
930983

‎packages/candid/src/utils/buffer.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,21 @@ export class PipeArrayBuffer {
3838
*/
3939
private _view: Uint8Array;
4040

41+
/**
42+
* Save a checkpoint of the reading view (for backtracking)
43+
*/
44+
public save() : Uint8Array {
45+
return this._view
46+
}
47+
48+
/**
49+
* Restore a checkpoint of the reading view (for backtracking)
50+
* @param checkPoint a previously saved checkpoint
51+
*/
52+
public restore(checkPoint: Uint8Array) {
53+
this._view = checkPoint
54+
}
55+
4156
/**
4257
* The actual buffer containing the bytes.
4358
* @private

0 commit comments

Comments
 (0)