Merge pull request #1509 from cosmos/omitDefault-instantiate2

Use omitDefault to fix Amino JSON converter for Instantiate2
cosmos · Nov 15, 2023 · 8f8449d · 8f8449d
2 parents bb34b58 + cf1fd0b
commit 8f8449d
Show file tree

Hide file tree

Showing 8 changed files with 132 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,15 +12,20 @@ and this project adheres to
   `sign`/`signAndBroadcast`/`signAndBroadcastSync` and related functions now
   have an additional parameter to specify the timeout height. After this height,
   a signed transaction will be considered invalid by the chain. ([#1489])
+- @cosmjs/amino: Export `omitDefault` to help build Amino JSON converters.
 
 [#1489]: https://github.com/cosmos/cosmjs/pull/1489
 
 ### Fixed
 
 - @cosmjs/stargate: Handle key value pairs in tx search correctly if the value
   is a numeric value. ([#1462])
+- @cosmjs/cosmwasm-stargate: Make `fix_msg` optional in
+  `AminoMsgInstantiateContract2` and omit default in the Amino JSON converter to
+  fix Amino JSON signing for MsgInstantiateContract2. ([#1509])
 
 [#1462]: https://github.com/cosmos/cosmjs/issues/1462
+[#1509]: https://github.com/cosmos/cosmjs/pull/1509
 
 ### Changed
 

diff --git a/packages/amino/src/index.ts b/packages/amino/src/index.ts
@@ -14,6 +14,7 @@ export {
   encodeSecp256k1Pubkey,
 } from "./encoding";
 export { createMultisigThresholdPubkey } from "./multisig";
+export { omitDefault } from "./omitdefault";
 export { makeCosmoshubPath } from "./paths";
 export {
   Ed25519Pubkey,

diff --git a/packages/amino/src/omitdefault.spec.ts b/packages/amino/src/omitdefault.spec.ts
@@ -0,0 +1,23 @@
+import { omitDefault } from "./omitdefault";
+
+describe("omitDefault", () => {
+  it("works for numbers", () => {
+    expect(omitDefault(17)).toEqual(17);
+    expect(omitDefault(0)).toEqual(undefined);
+  });
+
+  it("works for bigint", () => {
+    expect(omitDefault(17n)).toEqual(17n);
+    expect(omitDefault(0n)).toEqual(undefined);
+  });
+
+  it("works for boolean", () => {
+    expect(omitDefault(true)).toEqual(true);
+    expect(omitDefault(false)).toEqual(undefined);
+  });
+
+  it("works for strings", () => {
+    expect(omitDefault("hi")).toEqual("hi");
+    expect(omitDefault("")).toEqual(undefined);
+  });
+});
diff --git a/packages/amino/src/omitdefault.ts b/packages/amino/src/omitdefault.ts
@@ -0,0 +1,18 @@
+/**
+ * Returns the given input. If the input is the default value
+ * of protobuf, undefined is retunred. Use this when creating Amino JSON converters.
+ */
+export function omitDefault<T extends string | number | bigint | boolean>(input: T): T | undefined {
+  switch (typeof input) {
+    case "string":
+      return input === "" ? undefined : input;
+    case "number":
+      return input === 0 ? undefined : input;
+    case "bigint":
+      return input === BigInt(0) ? undefined : input;
+    case "boolean":
+      return !input ? undefined : input;
+    default:
+      throw new Error(`Got unsupported type '${typeof input}'`);
+  }
+}
diff --git a/packages/cosmwasm-stargate/src/modules/wasm/aminomessages.spec.ts b/packages/cosmwasm-stargate/src/modules/wasm/aminomessages.spec.ts
@@ -159,7 +159,7 @@ describe("AminoTypes", () => {
             funds: coins(1234, "ucosm"),
             admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
             salt: toBase64(toUtf8("salt")),
-            fix_msg: false,
+            fix_msg: undefined,
           },
         };
         expect(aminoMsg).toEqual(expected);
@@ -191,7 +191,39 @@ describe("AminoTypes", () => {
             funds: coins(1234, "ucosm"),
             admin: undefined,
             salt: toBase64(toUtf8("salt")),
-            fix_msg: false,
+            fix_msg: undefined,
+          },
+        };
+        expect(aminoMsg).toEqual(expected);
+      }
+
+      // With fixMsg=true (typically not needed)
+      {
+        const msg: MsgInstantiateContract2 = {
+          sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+          codeId: BigInt("12345"),
+          label: "sticky",
+          msg: toUtf8(`{"foo":"bar"}`),
+          funds: coins(1234, "ucosm"),
+          admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+          salt: toUtf8("salt"),
+          fixMsg: true,
+        };
+        const aminoMsg = new AminoTypes(createWasmAminoConverters()).toAmino({
+          typeUrl: "/cosmwasm.wasm.v1.MsgInstantiateContract2",
+          value: msg,
+        });
+        const expected: AminoMsgInstantiateContract2 = {
+          type: "wasm/MsgInstantiateContract2",
+          value: {
+            sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+            code_id: "12345",
+            label: "sticky",
+            msg: { foo: "bar" },
+            funds: coins(1234, "ucosm"),
+            admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+            salt: toBase64(toUtf8("salt")),
+            fix_msg: true,
           },
         };
         expect(aminoMsg).toEqual(expected);

diff --git a/packages/cosmwasm-stargate/src/modules/wasm/aminomessages.ts b/packages/cosmwasm-stargate/src/modules/wasm/aminomessages.ts
@@ -1,4 +1,5 @@
 /* eslint-disable @typescript-eslint/naming-convention */
+import { omitDefault } from "@cosmjs/amino";
 import { fromBase64, fromUtf8, toBase64, toUtf8 } from "@cosmjs/encoding";
 import { AminoConverters, Coin } from "@cosmjs/stargate";
 import {
@@ -141,8 +142,11 @@ export interface AminoMsgInstantiateContract2 {
     readonly admin?: string;
     /** Arbitrary Base64-encoded value provided by the sender */
     readonly salt: string;
-    /** Whether or not to include the msg value into the hash for the address */
-    readonly fix_msg: boolean;
+    /**
+     * Whether or not to include the msg value into the hash for the address.
+     * Unset means false. This should always be unset/false (https://medium.com/cosmwasm/dev-note-3-limitations-of-instantiate2-and-how-to-deal-with-them-a3f946874230).
+     */
+    readonly fix_msg?: boolean;
   };
 }
 
@@ -248,7 +252,7 @@ export function createWasmAminoConverters(): AminoConverters {
         label: label,
         msg: JSON.parse(fromUtf8(msg)),
         funds: funds,
-        admin: admin || undefined,
+        admin: omitDefault(admin),
       }),
       fromAmino: ({
         sender,
@@ -283,9 +287,9 @@ export function createWasmAminoConverters(): AminoConverters {
         label: label,
         msg: JSON.parse(fromUtf8(msg)),
         funds: funds,
-        admin: admin || undefined,
+        admin: omitDefault(admin),
         salt: toBase64(salt),
-        fix_msg: fixMsg,
+        fix_msg: omitDefault(fixMsg),
       }),
       fromAmino: ({
         sender,
@@ -304,7 +308,7 @@ export function createWasmAminoConverters(): AminoConverters {
         funds: [...funds],
         admin: admin ?? "",
         salt: fromBase64(salt),
-        fixMsg: fix_msg,
+        fixMsg: fix_msg ?? false,
       }),
     },
     "/cosmwasm.wasm.v1.MsgUpdateAdmin": {

diff --git a/packages/cosmwasm-stargate/src/signingcosmwasmclient.spec.ts b/packages/cosmwasm-stargate/src/signingcosmwasmclient.spec.ts
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/naming-convention */
 import { Secp256k1HdWallet } from "@cosmjs/amino";
-import { sha256 } from "@cosmjs/crypto";
+import { Random, sha256 } from "@cosmjs/crypto";
 import { toHex, toUtf8 } from "@cosmjs/encoding";
 import { decodeTxRaw, DirectSecp256k1HdWallet, Registry } from "@cosmjs/proto-signing";
 import {
@@ -343,6 +343,45 @@ describe("SigningCosmWasmClient", () => {
       expect(contractAddress).toEqual(expectedAddress);
       client.disconnect();
     });
+
+    it("works with Amino JSON signing", async () => {
+      pendingWithoutWasmd();
+      const aminoJsonWallet = await Secp256k1HdWallet.fromMnemonic(alice.mnemonic, {
+        prefix: wasmd.prefix,
+      });
+      const client = await SigningCosmWasmClient.connectWithSigner(
+        wasmd.endpoint,
+        aminoJsonWallet,
+        defaultSigningClientOptions,
+      );
+      const { codeId } = await client.upload(alice.address0, getHackatom().data, defaultUploadFee);
+      const funds = [coin(1234, "ucosm"), coin(321, "ustake")];
+      const salt = Random.getBytes(64);
+      const msg = {
+        verifier: alice.address0,
+        beneficiary: makeRandomAddress(),
+      };
+
+      const { contractAddress } = await client.instantiate2(
+        alice.address0,
+        codeId,
+        salt,
+        msg,
+        "My cool label--",
+        defaultInstantiateFee,
+        {
+          memo: "Let's see if the memo is used",
+          funds: funds,
+        },
+      );
+
+      const ucosmBalance = await client.getBalance(contractAddress, "ucosm");
+      const ustakeBalance = await client.getBalance(contractAddress, "ustake");
+      expect(ucosmBalance).toEqual(funds[0]);
+      expect(ustakeBalance).toEqual(funds[1]);
+
+      client.disconnect();
+    });
   });
 
   describe("updateAdmin", () => {

diff --git a/packages/stargate/src/modules/ibc/aminomessages.ts b/packages/stargate/src/modules/ibc/aminomessages.ts
@@ -1,5 +1,5 @@
 /* eslint-disable @typescript-eslint/naming-convention */
-import { AminoMsg, Coin } from "@cosmjs/amino";
+import { AminoMsg, Coin, omitDefault } from "@cosmjs/amino";
 import { MsgTransfer } from "cosmjs-types/ibc/applications/transfer/v1/tx";
 
 import { AminoConverters } from "../../aminotypes";
@@ -45,22 +45,6 @@ export function isAminoMsgTransfer(msg: AminoMsg): msg is AminoMsgTransfer {
   return msg.type === "cosmos-sdk/MsgTransfer";
 }
 
-function omitDefault<T extends string | number | bigint>(input: T): T | undefined {
-  if (typeof input === "string") {
-    return input === "" ? undefined : input;
-  }
-
-  if (typeof input === "number") {
-    return input === 0 ? undefined : input;
-  }
-
-  if (typeof input === "bigint") {
-    return input === BigInt(0) ? undefined : input;
-  }
-
-  throw new Error(`Got unsupported type '${typeof input}'`);
-}
-
 export function createIbcAminoConverters(): AminoConverters {
   return {
     "/ibc.applications.transfer.v1.MsgTransfer": {