Fix AminoMsgTransfer/AminoHeight encoding

CHANGELOG.md

@@ -6,6 +6,13 @@ and this project adheres to
 
 ## [Unreleased]
 
+## [0.25.6] - 2021-07-26
+
+### Fixed
+
+- @cosmjs/stargate: Fix types `AminoMsgTransfer` and `AminoHeight` as well as
+  the encoding of `MsgTransfer` for Amino signing.
+
 ## [0.25.5] - 2021-06-23
 
 ### Added
@@ -493,6 +500,7 @@ CHANGELOG entries missing. Please see [the diff][0.24.1].
 - @cosmjs/sdk38: Rename package to @cosmjs/launchpad.
 
 [unreleased]: https://github.com/cosmos/cosmjs/compare/v0.25.3...HEAD
+[0.25.6]: https://github.com/cosmos/cosmjs/compare/v0.25.5...v0.25.6
 [0.25.5]: https://github.com/cosmos/cosmjs/compare/v0.25.4...v0.25.5
 [0.25.4]: https://github.com/cosmos/cosmjs/compare/v0.25.3...v0.25.4
 [0.25.3]: https://github.com/cosmos/cosmjs/compare/v0.25.2...v0.25.3

packages/stargate/src/aminomsgs.ts

@@ -338,8 +338,10 @@ export function isAminoMsgUndelegate(msg: AminoMsg): msg is AminoMsgUndelegate {
 
 // https://github.com/cosmos/ibc-go/blob/07b6a97b67d17fd214a83764cbdb2c2c3daef445/modules/core/02-client/types/client.pb.go#L297-L312
 interface AminoHeight {
-  readonly revision_number: string;
+  /** 0 values must be omitted (https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/x/ibc/core/02-client/types/client.pb.go#L252). */
-  readonly revision_height: string;
+  readonly revision_number?: string;
+  /** 0 values must be omitted (https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/x/ibc/core/02-client/types/client.pb.go#L254). */
+  readonly revision_height?: string;
 }
 
 // https://github.com/cosmos/ibc-go/blob/07b6a97b67d17fd214a83764cbdb2c2c3daef445/modules/apps/transfer/types/tx.pb.go#L33-L53
@@ -354,10 +356,17 @@ export interface AminoMsgTransfer extends AminoMsg {
     readonly sender: string;
     /** Bech32 account address */
     readonly receiver: string;
+    /**
+     * It is unclear if this is really optional. The Amino encoding expects unset values to be
+     * encoded as {}.
+     */
     readonly timeout_height?: AminoHeight;
-    // Timeout timestamp (in nanoseconds) relative to the current block timestamp.
+    /**
-    // The timeout is disabled when set to 0.
+     * Timeout timestamp (in nanoseconds). The timeout is disabled when set to 0.
-    readonly timeout_timestamp: string;
+     *
+     * 0 values must be omitted (https://github.com/cosmos/cosmos-sdk/blob/v0.42.7/x/ibc/applications/transfer/types/tx.pb.go#L52).
+     */
+    readonly timeout_timestamp?: string;
   };
 }
 

packages/stargate/src/aminotypes.spec.ts

@@ -353,6 +353,70 @@ describe("AminoTypes", () => {
       expect(aminoMsg).toEqual(expected);
     });
 
+    it("works for MsgTransfer with empty values", () => {
+      const msg: MsgTransfer = {
+        sourcePort: "testport",
+        sourceChannel: "testchannel",
+        token: coin(1234, "utest"),
+        sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+        receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+        timeoutHeight: {
+          revisionHeight: Long.UZERO,
+          revisionNumber: Long.UZERO,
+        },
+        timeoutTimestamp: Long.UZERO,
+      };
+      const aminoMsg = new AminoTypes().toAmino({
+        typeUrl: "/ibc.applications.transfer.v1.MsgTransfer",
+        value: msg,
+      });
+      const expected: AminoMsgTransfer = {
+        type: "cosmos-sdk/MsgTransfer",
+        value: {
+          source_port: "testport",
+          source_channel: "testchannel",
+          token: coin(1234, "utest"),
+          sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+          receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+          timeout_height: {
+            revision_height: undefined,
+            revision_number: undefined,
+          },
+          timeout_timestamp: undefined,
+        },
+      };
+      expect(aminoMsg).toEqual(expected);
+    });
+
+    it("works for MsgTransfer with no height timeout", () => {
+      const msg: MsgTransfer = {
+        sourcePort: "testport",
+        sourceChannel: "testchannel",
+        token: coin(1234, "utest"),
+        sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+        receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+        timeoutHeight: undefined,
+        timeoutTimestamp: Long.UZERO,
+      };
+      const aminoMsg = new AminoTypes().toAmino({
+        typeUrl: "/ibc.applications.transfer.v1.MsgTransfer",
+        value: msg,
+      });
+      const expected: AminoMsgTransfer = {
+        type: "cosmos-sdk/MsgTransfer",
+        value: {
+          source_port: "testport",
+          source_channel: "testchannel",
+          token: coin(1234, "utest"),
+          sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+          receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+          timeout_height: {}, // 🤷‍♂️
+          timeout_timestamp: undefined,
+        },
+      };
+      expect(aminoMsg).toEqual(expected);
+    });
+
     it("works with custom type url", () => {
       const msg = {
         foo: "bar",
@@ -661,6 +725,41 @@ describe("AminoTypes", () => {
       });
     });
 
+    it("works for MsgTransfer with default values", () => {
+      const aminoMsg: AminoMsgTransfer = {
+        type: "cosmos-sdk/MsgTransfer",
+        value: {
+          source_port: "testport",
+          source_channel: "testchannel",
+          token: coin(1234, "utest"),
+          sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+          receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+          timeout_height: {
+            // revision_height omitted
+            // revision_number omitted
+          },
+          // timeout_timestamp omitted
+        },
+      };
+      const msg = new AminoTypes().fromAmino(aminoMsg);
+      const expectedValue: MsgTransfer = {
+        sourcePort: "testport",
+        sourceChannel: "testchannel",
+        token: coin(1234, "utest"),
+        sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
+        receiver: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
+        timeoutHeight: {
+          revisionHeight: Long.UZERO,
+          revisionNumber: Long.UZERO,
+        },
+        timeoutTimestamp: Long.UZERO,
+      };
+      expect(msg).toEqual({
+        typeUrl: "/ibc.applications.transfer.v1.MsgTransfer",
+        value: expectedValue,
+      });
+    });
+
     it("works for custom type url", () => {
       const aminoMsg = {
         type: "my-sdk/CustomType",

packages/stargate/src/aminotypes.ts

@@ -41,6 +41,22 @@ export interface AminoConverter {
   readonly fromAmino: (value: any) => any;
 }
 
+function omitDefault<T extends string | number | Long>(input: T): T | undefined {
+  if (typeof input === "string") {
+    return input === "" ? undefined : input;
+  }
+
+  if (typeof input === "number") {
+    return input === 0 ? undefined : input;
+  }
+
+  if (Long.isLong(input)) {
+    return input.isZero() ? undefined : input;
+  }
+
+  throw new Error(`Got unsupported type '${typeof input}'`);
+}
+
 function createDefaultTypes(prefix: string): Record<string, AminoConverter> {
   return {
     "/cosmos.bank.v1beta1.MsgSend": {
@@ -345,11 +361,11 @@ function createDefaultTypes(prefix: string): Record<string, AminoConverter> {
         receiver: receiver,
         timeout_height: timeoutHeight
           ? {
-              revision_height: timeoutHeight.revisionHeight.toString(),
+              revision_height: omitDefault(timeoutHeight.revisionHeight)?.toString(),
-              revision_number: timeoutHeight.revisionNumber.toString(),
+              revision_number: omitDefault(timeoutHeight.revisionNumber)?.toString(),
             }
-          : undefined,
+          : {},
-        timeout_timestamp: timeoutTimestamp.toString(),
+        timeout_timestamp: omitDefault(timeoutTimestamp)?.toString(),
       }),
       fromAmino: ({
         source_port,
@@ -367,11 +383,11 @@ function createDefaultTypes(prefix: string): Record<string, AminoConverter> {
         receiver: receiver,
         timeoutHeight: timeout_height
           ? {
-              revisionHeight: Long.fromString(timeout_height.revision_height, true),
+              revisionHeight: Long.fromString(timeout_height.revision_height || "0", true),
-              revisionNumber: Long.fromString(timeout_height.revision_number, true),
+              revisionNumber: Long.fromString(timeout_height.revision_number || "0", true),
             }
           : undefined,
-        timeoutTimestamp: Long.fromString(timeout_timestamp, true),
+        timeoutTimestamp: Long.fromString(timeout_timestamp || "0", true),
       }),
     },
   };

packages/stargate/src/signingstargateclient.spec.ts

@@ -2,6 +2,7 @@
 import { Secp256k1HdWallet } from "@cosmjs/amino";
 import { coin, coins, DirectSecp256k1HdWallet, Registry } from "@cosmjs/proto-signing";
 import { assert, sleep } from "@cosmjs/utils";
+import Long from "long";
 import protobuf from "protobufjs/minimal";
 
 import { decodeTxRaw } from "../../proto-signing/build";
@@ -14,7 +15,7 @@ import { AuthInfo, TxBody, TxRaw } from "./codec/cosmos/tx/v1beta1/tx";
 import { MsgDelegateEncodeObject, MsgSendEncodeObject } from "./encodeobjects";
 import { GasPrice } from "./fee";
 import { PrivateSigningStargateClient, SigningStargateClient } from "./signingstargateclient";
-import { assertIsBroadcastTxSuccess } from "./stargateclient";
+import { assertIsBroadcastTxSuccess, isBroadcastTxFailure } from "./stargateclient";
 import {
   faucet,
   makeRandomAddress,
@@ -321,6 +322,86 @@ describe("SigningStargateClient", () => {
     });
   });
 
+  describe("sendIbcTokens", () => {
+    it("works with direct signing", async () => {
+      pendingWithoutSimapp();
+      const wallet = await DirectSecp256k1HdWallet.fromMnemonic(faucet.mnemonic);
+      const client = await SigningStargateClient.connectWithSigner(simapp.tendermintUrl, wallet);
+      const memo = "Cross-chain fun";
+
+      // both timeouts set
+      {
+        const result = await client.sendIbcTokens(
+          faucet.address0,
+          faucet.address1,
+          coin(1234, "ucosm"),
+          "fooPort",
+          "fooChannel",
+          { revisionHeight: Long.fromNumber(123), revisionNumber: Long.fromNumber(456) },
+          Math.floor(Date.now() / 1000) + 60,
+          memo,
+        );
+        // CheckTx must pass but the execution must fail in DeliverTx due to invalid channel/port
+        expect(isBroadcastTxFailure(result)).toEqual(true);
+      }
+
+      // no height timeout
+      {
+        const result = await client.sendIbcTokens(
+          faucet.address0,
+          faucet.address1,
+          coin(1234, "ucosm"),
+          "fooPort",
+          "fooChannel",
+          undefined,
+          Math.floor(Date.now() / 1000) + 60,
+          memo,
+        );
+        // CheckTx must pass but the execution must fail in DeliverTx due to invalid channel/port
+        expect(isBroadcastTxFailure(result)).toEqual(true);
+      }
+    });
+
+    it("works with Amino signing", async () => {
+      pendingWithoutSimapp();
+      const wallet = await Secp256k1HdWallet.fromMnemonic(faucet.mnemonic);
+      const client = await SigningStargateClient.connectWithSigner(simapp.tendermintUrl, wallet);
+      const memo = "Cross-chain fun";
+
+      // both timeouts set
+      {
+        const result = await client.sendIbcTokens(
+          faucet.address0,
+          faucet.address1,
+          coin(1234, "ucosm"),
+          "fooPort",
+          "fooChannel",
+          { revisionHeight: Long.fromNumber(123), revisionNumber: Long.fromNumber(456) },
+          Math.floor(Date.now() / 1000) + 60,
+          memo,
+        );
+        // CheckTx must pass but the execution must fail in DeliverTx due to invalid channel/port
+        expect(isBroadcastTxFailure(result)).toEqual(true);
+      }
+
+      // no height timeout
+      {
+        const result = await client.sendIbcTokens(
+          faucet.address0,
+          faucet.address1,
+          coin(1234, "ucosm"),
+          "fooPort",
+          "fooChannel",
+          undefined,
+          Math.floor(Date.now() / 1000) + 60,
+          memo,
+        );
+        // CheckTx must pass but the execution must fail in DeliverTx due to invalid channel/port
+        expect(isBroadcastTxFailure(result)).toEqual(true);
+      }
+    });
+  });
+
   describe("signAndBroadcast", () => {
     describe("direct mode", () => {
       it("works", async () => {