diff --git a/CHANGELOG.md b/CHANGELOG.md index 15b6551eaf..8a54b32875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,10 @@ and this project adheres to - cosmos.authz.v1beta1.MsgRevoke - cosmos.feegrant.v1beta1.MsgGrantAllowance - cosmos.feegrant.v1beta1.MsgRevokeAllowance +- @cosmjs/stargate: In `AminoTypes` the uniqueness of the Amino type identifier + is checked in `fromAmino` now instead of the constructor. This only affects + you if multiple different protobuf type URLs map to the same Amino type + identifier which should not be the case anyways. [#989]: https://github.com/cosmos/cosmjs/issues/989 [#994]: https://github.com/cosmos/cosmjs/issues/994 diff --git a/packages/stargate/src/aminotypes.spec.ts b/packages/stargate/src/aminotypes.spec.ts index 158f31e43d..d2fd70e31c 100644 --- a/packages/stargate/src/aminotypes.spec.ts +++ b/packages/stargate/src/aminotypes.spec.ts @@ -41,6 +41,79 @@ import { import { AminoTypes } from "./aminotypes"; describe("AminoTypes", () => { + describe("constructor", () => { + const msg: MsgDelegate = { + delegatorAddress: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6", + validatorAddress: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5", + amount: coin(1234, "ucosm"), + }; + + it("can override type by type URL", () => { + const types = new AminoTypes({ + prefix: "cosmos", + additions: { + "/cosmos.staking.v1beta1.MsgDelegate": { + aminoType: "my-override/MsgDelegate", + toAmino: (m: MsgDelegate): { readonly foo: string } => ({ + foo: m.delegatorAddress ?? "", + }), + fromAmino: () => ({ + bar: 123, + }), + }, + }, + }); + + const aminoMsg = types.toAmino({ + typeUrl: "/cosmos.staking.v1beta1.MsgDelegate", + value: msg, + }); + expect(aminoMsg).toEqual({ + type: "my-override/MsgDelegate", + value: { + foo: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6", + }, + }); + expect(types.fromAmino(aminoMsg)).toEqual({ + typeUrl: "/cosmos.staking.v1beta1.MsgDelegate", + value: { + bar: 123, + }, + }); + }); + + it("can override type with Amino type collision", () => { + const types = new AminoTypes({ + prefix: "cosmos", + additions: { + "/cosmos.staking.otherVersion456.MsgDelegate": { + aminoType: "cosmos-sdk/MsgDelegate", + toAmino: (m: MsgDelegate): { readonly foo: string } => ({ + foo: m.delegatorAddress ?? "", + }), + fromAmino: () => ({ + bar: 123, + }), + }, + }, + }); + + const aminoMsg = types.toAmino({ + typeUrl: "/cosmos.staking.otherVersion456.MsgDelegate", + value: msg, + }); + expect(aminoMsg).toEqual({ + type: "cosmos-sdk/MsgDelegate", + value: { + foo: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6", + }, + }); + expect(() => types.fromAmino(aminoMsg)).toThrowError( + "Multiple types are registered with Amino type identifier 'cosmos-sdk/MsgDelegate': '/cosmos.staking.otherVersion456.MsgDelegate', '/cosmos.staking.v1beta1.MsgDelegate'. Thus fromAmino cannot be performed.", + ); + }); + }); + describe("toAmino", () => { // bank @@ -975,12 +1048,12 @@ describe("AminoTypes", () => { }); }); - it("works with overridden type url", () => { + it("works with overridden type URL", () => { const msg = new AminoTypes({ prefix: "cosmos", additions: { - "/my.OverrideType": { - aminoType: "cosmos-sdk/MsgDelegate", + "/cosmos.staking.v1beta1.MsgDelegate": { + aminoType: "cosmos-sdk/MsgDelegate2", toAmino: () => {}, fromAmino: ({ foo }: { readonly foo: string }): MsgDelegate => ({ delegatorAddress: foo, @@ -990,20 +1063,19 @@ describe("AminoTypes", () => { }, }, }).fromAmino({ - type: "cosmos-sdk/MsgDelegate", + type: "cosmos-sdk/MsgDelegate2", value: { foo: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6", }, }); - const expected: { readonly typeUrl: "/my.OverrideType"; readonly value: MsgDelegate } = { - typeUrl: "/my.OverrideType", + expect(msg).toEqual({ + typeUrl: "/cosmos.staking.v1beta1.MsgDelegate", value: { delegatorAddress: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6", validatorAddress: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5", amount: coin(1234, "ucosm"), }, - }; - expect(msg).toEqual(expected); + }); }); it("throws for unknown type url", () => { diff --git a/packages/stargate/src/aminotypes.ts b/packages/stargate/src/aminotypes.ts index cf7a545147..efc8e1cde2 100644 --- a/packages/stargate/src/aminotypes.ts +++ b/packages/stargate/src/aminotypes.ts @@ -524,18 +524,15 @@ export interface AminoTypesOptions { * to Amino types. */ export class AminoTypes { + // The map type here ensures uniqueness of the protobuf type URL in the key. + // There is no uniqueness guarantee of the Amino type identifier in the type + // system or constructor. Instead it's the user's responsibility to ensure + // there is no overlap when fromAmino is called. private readonly register: Record; public constructor({ prefix, additions = {} }: AminoTypesOptions) { - const additionalAminoTypes = Object.values(additions); - const filteredDefaultTypes = Object.entries(createDefaultTypes(prefix)).reduce( - (acc, [key, value]) => - additionalAminoTypes.find(({ aminoType }) => value.aminoType === aminoType) - ? acc - : { ...acc, [key]: value }, - {}, - ); - this.register = { ...filteredDefaultTypes, ...additions }; + const defaultTypes = createDefaultTypes(prefix); + this.register = { ...defaultTypes, ...additions }; } public toAmino({ typeUrl, value }: EncodeObject): AminoMsg { @@ -554,18 +551,31 @@ export class AminoTypes { } public fromAmino({ type, value }: AminoMsg): EncodeObject { - const result = Object.entries(this.register).find(([_typeUrl, { aminoType }]) => aminoType === type); - if (!result) { - throw new Error( - `Amino type identifier '${type}' does not exist in the Amino message type register. ` + - "If you need support for this message type, you can pass in additional entries to the AminoTypes constructor. " + - "If you think this message type should be included by default, please open an issue at https://github.com/cosmos/cosmjs/issues.", - ); + const matches = Object.entries(this.register).filter(([_typeUrl, { aminoType }]) => aminoType === type); + switch (matches.length) { + case 0: { + throw new Error( + `Amino type identifier '${type}' does not exist in the Amino message type register. ` + + "If you need support for this message type, you can pass in additional entries to the AminoTypes constructor. " + + "If you think this message type should be included by default, please open an issue at https://github.com/cosmos/cosmjs/issues.", + ); + } + case 1: { + const [typeUrl, converter] = matches[0]; + return { + typeUrl: typeUrl, + value: converter.fromAmino(value), + }; + } + default: + throw new Error( + `Multiple types are registered with Amino type identifier '${type}': '` + + matches + .map(([key, _value]) => key) + .sort() + .join("', '") + + "'. Thus fromAmino cannot be performed.", + ); } - const [typeUrl, converter] = result; - return { - typeUrl: typeUrl, - value: converter.fromAmino(value), - }; } }