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 diff --git a/packages/cosmwasm-stargate/src/cosmwasmclient.ts b/packages/cosmwasm-stargate/src/cosmwasmclient.ts index 33f71156..619b08b4 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 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! public async broadcastTx( @@ -240,12 +251,24 @@ 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())) - .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.spec.ts b/packages/stargate/src/stargateclient.spec.ts index c12f1199..ca2d2b7c 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); @@ -341,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); @@ -373,12 +421,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))!; diff --git a/packages/stargate/src/stargateclient.ts b/packages/stargate/src/stargateclient.ts index 1d198d38..fa857fcd 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 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 + * 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 to check for execution success or failure. + */ public async broadcastTx( tx: Uint8Array, timeoutMs = 60_000, @@ -323,12 +343,24 @@ 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())) - .then(resolve, reject) - .finally(() => clearTimeout(txPollTimeout)), + pollForTx(transactionId).then( + (value) => { + clearTimeout(txPollTimeout); + resolve(value); + }, + (error) => { + clearTimeout(txPollTimeout); + reject(error); + }, + ), ); } 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