Check uniqueness of Amino type in fromAmino

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

packages/stargate/src/aminotypes.spec.ts

@@ -82,8 +82,7 @@ describe("AminoTypes", () => {
       });
     });
 
-    // This is a feature we have right now but we don't need
+    it("can override type with Amino type collision", () => {
-    it("can override type by Amino type", () => {
       const types = new AminoTypes({
         prefix: "cosmos",
         additions: {
@@ -109,12 +108,9 @@ describe("AminoTypes", () => {
           foo: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
         },
       });
-      expect(types.fromAmino(aminoMsg)).toEqual({
+      expect(() => types.fromAmino(aminoMsg)).toThrowError(
-        typeUrl: "/cosmos.staking.otherVersion456.MsgDelegate",
+        "Multiple types are registered with Amino type identifier 'cosmos-sdk/MsgDelegate': '/cosmos.staking.otherVersion456.MsgDelegate', '/cosmos.staking.v1beta1.MsgDelegate'. Thus fromAmino cannot be performed.",
-        value: {
+      );
-          bar: 123,
-        },
-      });
     });
   });
 
@@ -1052,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": {
+          "/cosmos.staking.v1beta1.MsgDelegate": {
-            aminoType: "cosmos-sdk/MsgDelegate",
+            aminoType: "cosmos-sdk/MsgDelegate2",
             toAmino: () => {},
             fromAmino: ({ foo }: { readonly foo: string }): MsgDelegate => ({
               delegatorAddress: foo,
@@ -1067,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 } = {
+      expect(msg).toEqual({
-        typeUrl: "/my.OverrideType",
+        typeUrl: "/cosmos.staking.v1beta1.MsgDelegate",
         value: {
           delegatorAddress: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
           validatorAddress: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
           amount: coin(1234, "ucosm"),
         },
-      };
+      });
-      expect(msg).toEqual(expected);
     });
 
     it("throws for unknown type url", () => {

packages/stargate/src/aminotypes.ts

@@ -519,31 +519,20 @@ export interface AminoTypesOptions {
   readonly additions?: Record<string, AminoConverter>;
 }
 
-function sameAminoType(a: AminoConverter, b: AminoConverter): boolean {
-  return a.aminoType === b.aminoType;
-}
-
 /**
  * A map from Stargate message types as used in the messages's `Any` type
  * 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<string, AminoConverter>;
 
   public constructor({ prefix, additions = {} }: AminoTypesOptions) {
     const defaultTypes = createDefaultTypes(prefix);
-    const additionalAminoTypes = Object.values(additions);
+    this.register = { ...defaultTypes, ...additions };
-    // `filteredDefaultTypes` contains all types of `defaultTypes`
-    // that are not included in the `additions`. Not included
-    // means not having the same Amino type identifier.
-    const filteredDefaultTypes = Object.entries(defaultTypes).reduce(
-      (acc, [key, value]) =>
-        additionalAminoTypes.find((addition) => sameAminoType(value, addition))
-          ? acc /* Skip this defaultTypes entry */
-          : { ...acc, [key]: value } /* Add this defaultTypes entry */,
-      {},
-    );
-    this.register = { ...filteredDefaultTypes, ...additions };
   }
 
   public toAmino({ typeUrl, value }: EncodeObject): AminoMsg {
@@ -562,18 +551,31 @@ export class AminoTypes {
   }
 
   public fromAmino({ type, value }: AminoMsg): EncodeObject {
-    const result = Object.entries(this.register).find(([_typeUrl, { aminoType }]) => aminoType === type);
+    const matches = Object.entries(this.register).filter(([_typeUrl, { aminoType }]) => aminoType === type);
-    if (!result) {
+    switch (matches.length) {
-      throw new Error(
+      case 0: {
-        `Amino type identifier '${type}' does not exist in the Amino message type register. ` +
+        throw new Error(
-          "If you need support for this message type, you can pass in additional entries to the AminoTypes constructor. " +
+          `Amino type identifier '${type}' does not exist in the Amino message type register. ` +
-          "If you think this message type should be included by default, please open an issue at https://github.com/cosmos/cosmjs/issues.",
+            "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),
-    };
   }
 }