From ce4202c3c3b23d4bcf3b885f36fb2727530a90ea Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 26 Jul 2021 10:23:48 +0200 Subject: [PATCH] Fix AminoMsgTransfer/AminoHeight encoding --- CHANGELOG.md | 8 ++ packages/stargate/src/aminomsgs.ts | 19 +++- packages/stargate/src/aminotypes.spec.ts | 99 +++++++++++++++++++ packages/stargate/src/aminotypes.ts | 30 ++++-- .../src/signingstargateclient.spec.ts | 83 +++++++++++++++- 5 files changed, 226 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da81bf31d9..747aed666d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/packages/stargate/src/aminomsgs.ts b/packages/stargate/src/aminomsgs.ts index 5040c11f45..42f288ee5f 100644 --- a/packages/stargate/src/aminomsgs.ts +++ b/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; - readonly revision_height: 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_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. - readonly timeout_timestamp: string; + /** + * Timeout timestamp (in nanoseconds). The timeout is disabled when set to 0. + * + * 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; }; } diff --git a/packages/stargate/src/aminotypes.spec.ts b/packages/stargate/src/aminotypes.spec.ts index b249175150..27aca60083 100644 --- a/packages/stargate/src/aminotypes.spec.ts +++ b/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", diff --git a/packages/stargate/src/aminotypes.ts b/packages/stargate/src/aminotypes.ts index 41a4827736..390ccac310 100644 --- a/packages/stargate/src/aminotypes.ts +++ b/packages/stargate/src/aminotypes.ts @@ -41,6 +41,22 @@ export interface AminoConverter { readonly fromAmino: (value: any) => any; } +function omitDefault(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 { return { "/cosmos.bank.v1beta1.MsgSend": { @@ -345,11 +361,11 @@ function createDefaultTypes(prefix: string): Record { receiver: receiver, timeout_height: timeoutHeight ? { - revision_height: timeoutHeight.revisionHeight.toString(), - revision_number: timeoutHeight.revisionNumber.toString(), + revision_height: omitDefault(timeoutHeight.revisionHeight)?.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 { receiver: receiver, timeoutHeight: timeout_height ? { - revisionHeight: Long.fromString(timeout_height.revision_height, true), - revisionNumber: Long.fromString(timeout_height.revision_number, true), + revisionHeight: Long.fromString(timeout_height.revision_height || "0", true), + revisionNumber: Long.fromString(timeout_height.revision_number || "0", true), } : undefined, - timeoutTimestamp: Long.fromString(timeout_timestamp, true), + timeoutTimestamp: Long.fromString(timeout_timestamp || "0", true), }), }, }; diff --git a/packages/stargate/src/signingstargateclient.spec.ts b/packages/stargate/src/signingstargateclient.spec.ts index c547f26b88..0c872e93d9 100644 --- a/packages/stargate/src/signingstargateclient.spec.ts +++ b/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 () => {