From efdfd8a73366a1f455f902320abb28c793fcc0d0 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 17 May 2021 10:58:30 +0200 Subject: [PATCH 1/6] Write fee amount more compact --- packages/stargate/src/stargateclient.spec.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/stargate/src/stargateclient.spec.ts b/packages/stargate/src/stargateclient.spec.ts index c12f1199..3130ab32 100644 --- a/packages/stargate/src/stargateclient.spec.ts +++ b/packages/stargate/src/stargateclient.spec.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { fromBase64, toBase64 } from "@cosmjs/encoding"; import { + coins, DirectSecp256k1HdWallet, encodePubkey, makeAuthInfoBytes, @@ -312,12 +313,7 @@ describe("StargateClient", () => { }; const txBodyBytes = registry.encode(txBodyFields); const { accountNumber, sequence } = (await client.getSequence(address))!; - const feeAmount = [ - { - amount: "2000", - denom: "ucosm", - }, - ]; + const feeAmount = coins(2000, "ucosm"); const gasLimit = 200000; const authInfoBytes = makeAuthInfoBytes([pubkey], feeAmount, gasLimit, sequence); @@ -373,12 +369,7 @@ describe("StargateClient", () => { }; const txBodyBytes = registry.encode(txBodyFields); const chainId = await client.getChainId(); - const feeAmount = [ - { - amount: "2000", - denom: "ucosm", - }, - ]; + const feeAmount = coins(2000, "ucosm"); const gasLimit = 200000; const { accountNumber: accountNumber1, sequence: sequence1 } = (await client.getSequence(address))!; From 2d6bbb079f4fec630726164b487b0db41bc494ec Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 17 May 2021 12:12:37 +0200 Subject: [PATCH 2/6] Handle CheckTx errors properly --- .../cosmwasm-stargate/src/cosmwasmclient.ts | 22 ++++++-- packages/stargate/src/stargateclient.spec.ts | 52 +++++++++++++++++++ packages/stargate/src/stargateclient.ts | 31 +++++++++-- 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/packages/cosmwasm-stargate/src/cosmwasmclient.ts b/packages/cosmwasm-stargate/src/cosmwasmclient.ts index 33f71156..7299ab56 100644 --- a/packages/cosmwasm-stargate/src/cosmwasmclient.ts +++ b/packages/cosmwasm-stargate/src/cosmwasmclient.ts @@ -207,6 +207,17 @@ export class CosmWasmClient { if (this.tmClient) this.tmClient.disconnect(); } + /** + * Broadcasts a signed transaction to the network and monitors its inclusion in a block. + * + * If broadcasting is rejected by the node for some reason (e.g. because of a CheckTx failure), + * an error is thrown. + * + * If the transaction is not included in a block before the provided timeout, this errors with a `TimeoutError`. + * + * If the transaction is included in a block, a `BroadcastTxResponse` is returned. The caller then + * usually needs check for execution success or failure. + */ // NOTE: This method is tested against slow chains and timeouts in the @cosmjs/stargate package. // Make sure it is kept in sync! public async broadcastTx( @@ -240,10 +251,15 @@ export class CosmWasmClient { : pollForTx(txId); }; + const broadcasted = await this.forceGetTmClient().broadcastTxSync({ tx }); + if (broadcasted.code) { + throw new Error( + `Broadcasting transaction failed with code ${broadcasted.code} (codespace: ${broadcasted.codeSpace}). Log: ${broadcasted.log}`, + ); + } + const transactionId = toHex(broadcasted.hash).toUpperCase(); return new Promise((resolve, reject) => - this.forceGetTmClient() - .broadcastTxSync({ tx }) - .then(({ hash }) => pollForTx(toHex(hash).toUpperCase())) + pollForTx(transactionId) .then(resolve, reject) .finally(() => clearTimeout(txPollTimeout)), ); diff --git a/packages/stargate/src/stargateclient.spec.ts b/packages/stargate/src/stargateclient.spec.ts index 3130ab32..ca2d2b7c 100644 --- a/packages/stargate/src/stargateclient.spec.ts +++ b/packages/stargate/src/stargateclient.spec.ts @@ -337,6 +337,58 @@ describe("StargateClient", () => { client.disconnect(); }); + it("errors immediately for a CheckTx failure", async () => { + pendingWithoutSimapp(); + const client = await StargateClient.connect(simapp.tendermintUrl); + const wallet = await DirectSecp256k1HdWallet.fromMnemonic(faucet.mnemonic); + const [{ address, pubkey: pubkeyBytes }] = await wallet.getAccounts(); + const pubkey = encodePubkey({ + type: "tendermint/PubKeySecp256k1", + value: toBase64(pubkeyBytes), + }); + const registry = new Registry(); + const invalidRecipientAddress = "tgrade1z363ulwcrxged4z5jswyt5dn5v3lzsemwz9ewj"; // wrong bech32 prefix + const txBodyFields: TxBodyEncodeObject = { + typeUrl: "/cosmos.tx.v1beta1.TxBody", + value: { + messages: [ + { + typeUrl: "/cosmos.bank.v1beta1.MsgSend", + value: { + fromAddress: address, + toAddress: invalidRecipientAddress, + amount: [ + { + denom: "ucosm", + amount: "1234567", + }, + ], + }, + }, + ], + }, + }; + const txBodyBytes = registry.encode(txBodyFields); + const { accountNumber, sequence } = (await client.getSequence(address))!; + const feeAmount = coins(2000, "ucosm"); + const gasLimit = 200000; + const authInfoBytes = makeAuthInfoBytes([pubkey], feeAmount, gasLimit, sequence); + + const chainId = await client.getChainId(); + const signDoc = makeSignDoc(txBodyBytes, authInfoBytes, chainId, accountNumber); + const { signature } = await wallet.signDirect(address, signDoc); + const txRaw = TxRaw.fromPartial({ + bodyBytes: txBodyBytes, + authInfoBytes: authInfoBytes, + signatures: [fromBase64(signature.signature)], + }); + const txRawBytes = Uint8Array.from(TxRaw.encode(txRaw).finish()); + + await expectAsync(client.broadcastTx(txRawBytes)).toBeRejectedWithError(/invalid recipient address/i); + + client.disconnect(); + }); + it("respects user timeouts rather than RPC timeouts", async () => { pendingWithoutSlowSimapp(); const client = await StargateClient.connect(slowSimapp.tendermintUrl); diff --git a/packages/stargate/src/stargateclient.ts b/packages/stargate/src/stargateclient.ts index 1d198d38..fb7e2011 100644 --- a/packages/stargate/src/stargateclient.ts +++ b/packages/stargate/src/stargateclient.ts @@ -99,6 +99,15 @@ export interface BroadcastTxSuccess { readonly gasWanted: number; } +/** + * The response after sucessfully broadcasting a transaction. + * Success or failure refer to the execution result. + * + * The name is a bit misleading as this contains the result of the execution + * in a block. Both `BroadcastTxSuccess` and `BroadcastTxFailure` contain a height + * field, which is the height of the block that contains the transaction. This means + * transactions that were never included in a block cannot be expressed with this type. + */ export type BroadcastTxResponse = BroadcastTxSuccess | BroadcastTxFailure; export function isBroadcastTxFailure(result: BroadcastTxResponse): result is BroadcastTxFailure { @@ -292,6 +301,17 @@ export class StargateClient { if (this.tmClient) this.tmClient.disconnect(); } + /** + * Broadcasts a signed transaction to the network and monitors its inclusion in a block. + * + * If broadcasting is rejected by the node for some reason (e.g. because of a CheckTx failure), + * an error is thrown. + * + * If the transaction is not included in a block before the provided timeout, this errors with a `TimeoutError`. + * + * If the transaction is included in a block, a `BroadcastTxResponse` is returned. The caller then + * usually needs check for execution success or failure. + */ public async broadcastTx( tx: Uint8Array, timeoutMs = 60_000, @@ -323,10 +343,15 @@ export class StargateClient { : pollForTx(txId); }; + const broadcasted = await this.forceGetTmClient().broadcastTxSync({ tx }); + if (broadcasted.code) { + throw new Error( + `Broadcasting transaction failed with code ${broadcasted.code} (codespace: ${broadcasted.codeSpace}). Log: ${broadcasted.log}`, + ); + } + const transactionId = toHex(broadcasted.hash).toUpperCase(); return new Promise((resolve, reject) => - this.forceGetTmClient() - .broadcastTxSync({ tx }) - .then(({ hash }) => pollForTx(toHex(hash).toUpperCase())) + pollForTx(transactionId) .then(resolve, reject) .finally(() => clearTimeout(txPollTimeout)), ); From 4c4c181e40ddbb798cc212051d48f7e9a98ac44b Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 17 May 2021 12:22:32 +0200 Subject: [PATCH 3/6] Add CHANGELOG entry for the fix --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40c1ef6f..cbf741b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ and this project adheres to ## [Unreleased] +### Fixed + +- @cosmjs/cosmwasm-stargate, @cosmjs/stargate: Fix error propagation in + `CosmWasmClient.broadcastTx` and `StargateClient.broadcastTx` ([#800]). This + bug was introduced with the switch from broadcast mode "commit" to "sync" in + version 0.25.0. + +[#800]: https://github.com/cosmos/cosmjs/issues/800 + ## [0.25.2] - 2021-05-11 ### Added From b62c4b4b58001e1bb2e9e1a33d064d9d15e3d8a1 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Tue, 18 May 2021 11:29:39 +0200 Subject: [PATCH 4/6] Fix typo sucessfully -> successfully --- packages/stargate/src/stargateclient.ts | 2 +- packages/stream/src/defaultvalueproducer.spec.ts | 2 +- packages/tendermint-rpc/src/legacy/responses.ts | 2 +- packages/tendermint-rpc/src/tendermint34/responses.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/stargate/src/stargateclient.ts b/packages/stargate/src/stargateclient.ts index fb7e2011..1add2689 100644 --- a/packages/stargate/src/stargateclient.ts +++ b/packages/stargate/src/stargateclient.ts @@ -100,7 +100,7 @@ export interface BroadcastTxSuccess { } /** - * The response after sucessfully broadcasting a transaction. + * The response after successfully broadcasting a transaction. * Success or failure refer to the execution result. * * The name is a bit misleading as this contains the result of the execution diff --git a/packages/stream/src/defaultvalueproducer.spec.ts b/packages/stream/src/defaultvalueproducer.spec.ts index 42bf74ff..1fb80ce6 100644 --- a/packages/stream/src/defaultvalueproducer.spec.ts +++ b/packages/stream/src/defaultvalueproducer.spec.ts @@ -57,7 +57,7 @@ describe("DefaultValueProducer", () => { expect(error).toEqual("oh no :("); done(); }, - complete: () => done.fail("Stream must not complete sucessfully"), + complete: () => done.fail("Stream must not complete successfully"), }); producer.update(1); diff --git a/packages/tendermint-rpc/src/legacy/responses.ts b/packages/tendermint-rpc/src/legacy/responses.ts index 55f1dbcd..ee8accb7 100644 --- a/packages/tendermint-rpc/src/legacy/responses.ts +++ b/packages/tendermint-rpc/src/legacy/responses.ts @@ -73,7 +73,7 @@ export interface BroadcastTxSyncResponse extends TxData { } /** - * Returns true iff transaction made it sucessfully into the transaction pool + * Returns true iff transaction made it successfully into the transaction pool */ export function broadcastTxSyncSuccess(res: BroadcastTxSyncResponse): boolean { // code must be 0 on success diff --git a/packages/tendermint-rpc/src/tendermint34/responses.ts b/packages/tendermint-rpc/src/tendermint34/responses.ts index 5e11184c..ba7d3970 100644 --- a/packages/tendermint-rpc/src/tendermint34/responses.ts +++ b/packages/tendermint-rpc/src/tendermint34/responses.ts @@ -73,7 +73,7 @@ export interface BroadcastTxSyncResponse extends TxData { } /** - * Returns true iff transaction made it sucessfully into the transaction pool + * Returns true iff transaction made it successfully into the transaction pool */ export function broadcastTxSyncSuccess(res: BroadcastTxSyncResponse): boolean { // code must be 0 on success From 5cd40416661a3a5ba76e678578c9cfd36b385ea9 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Tue, 18 May 2021 11:30:53 +0200 Subject: [PATCH 5/6] Add missing "to" in broadcastTx doc comments --- packages/cosmwasm-stargate/src/cosmwasmclient.ts | 2 +- packages/stargate/src/stargateclient.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cosmwasm-stargate/src/cosmwasmclient.ts b/packages/cosmwasm-stargate/src/cosmwasmclient.ts index 7299ab56..2420e150 100644 --- a/packages/cosmwasm-stargate/src/cosmwasmclient.ts +++ b/packages/cosmwasm-stargate/src/cosmwasmclient.ts @@ -216,7 +216,7 @@ export class CosmWasmClient { * If the transaction is not included in a block before the provided timeout, this errors with a `TimeoutError`. * * If the transaction is included in a block, a `BroadcastTxResponse` is returned. The caller then - * usually needs check for execution success or failure. + * usually needs to check for execution success or failure. */ // NOTE: This method is tested against slow chains and timeouts in the @cosmjs/stargate package. // Make sure it is kept in sync! diff --git a/packages/stargate/src/stargateclient.ts b/packages/stargate/src/stargateclient.ts index 1add2689..6c8a9eb9 100644 --- a/packages/stargate/src/stargateclient.ts +++ b/packages/stargate/src/stargateclient.ts @@ -310,7 +310,7 @@ export class StargateClient { * If the transaction is not included in a block before the provided timeout, this errors with a `TimeoutError`. * * If the transaction is included in a block, a `BroadcastTxResponse` is returned. The caller then - * usually needs check for execution success or failure. + * usually needs to check for execution success or failure. */ public async broadcastTx( tx: Uint8Array, From 33ff4754a7142e0b1e35775f248363c85e45738e Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Tue, 18 May 2021 11:33:21 +0200 Subject: [PATCH 6/6] Clear timeout before resolving/rejecting --- packages/cosmwasm-stargate/src/cosmwasmclient.ts | 13 ++++++++++--- packages/stargate/src/stargateclient.ts | 13 ++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/cosmwasm-stargate/src/cosmwasmclient.ts b/packages/cosmwasm-stargate/src/cosmwasmclient.ts index 2420e150..619b08b4 100644 --- a/packages/cosmwasm-stargate/src/cosmwasmclient.ts +++ b/packages/cosmwasm-stargate/src/cosmwasmclient.ts @@ -259,9 +259,16 @@ export class CosmWasmClient { } const transactionId = toHex(broadcasted.hash).toUpperCase(); return new Promise((resolve, reject) => - pollForTx(transactionId) - .then(resolve, reject) - .finally(() => clearTimeout(txPollTimeout)), + pollForTx(transactionId).then( + (value) => { + clearTimeout(txPollTimeout); + resolve(value); + }, + (error) => { + clearTimeout(txPollTimeout); + reject(error); + }, + ), ); } diff --git a/packages/stargate/src/stargateclient.ts b/packages/stargate/src/stargateclient.ts index 6c8a9eb9..fa857fcd 100644 --- a/packages/stargate/src/stargateclient.ts +++ b/packages/stargate/src/stargateclient.ts @@ -351,9 +351,16 @@ export class StargateClient { } const transactionId = toHex(broadcasted.hash).toUpperCase(); return new Promise((resolve, reject) => - pollForTx(transactionId) - .then(resolve, reject) - .finally(() => clearTimeout(txPollTimeout)), + pollForTx(transactionId).then( + (value) => { + clearTimeout(txPollTimeout); + resolve(value); + }, + (error) => { + clearTimeout(txPollTimeout); + reject(error); + }, + ), ); }