Merge pull request #805 from cosmos/fix-boadcast-error

Fix error propagation for checkTx errors

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

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);
+        },
+      ),
     );
   }
 

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))!;

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);
+        },
+      ),
     );
   }
 

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);

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

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