fix: Do not retry RPC requests on 4xx errors (#5634)

Any error on a transaction that used capsules was causing a `no capsules
available` error, even if unrelated to the capsule. Reason for this is
that our remote pxe client was configured by default to retry all failed
requests (but not on e2e tests), so the first attempt would raise the
actual error and consume the capsule, and all retries would fail since
the first request consumed the capsule.

Barring actually fixing capsules (still pending), a fix is not retrying
a request upon a 4xx error, so we don't unnecessarily hammer the pxe.
Note that retries had been originally introduced to handle connectivity
errors, I expect that those would not be raised as 4xx's.

noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr

@@ -21,6 +21,13 @@ contract StatefulTest {
         let _res = context.call_private_function(context.this_address(), selector, [owner.to_field(), value]);
     }
 
+    #[aztec(private)]
+    #[aztec(initializer)]
+    fn wrong_constructor() {
+        let selector = FunctionSelector::from_signature("not_exists(Field)");
+        let _res = context.call_public_function(context.this_address(), selector, [42]);
+    }
+
     #[aztec(public)]
     #[aztec(initializer)]
     fn public_constructor(owner: AztecAddress, value: Field) {

yarn-project/end-to-end/src/e2e_deploy_contract.test.ts

@@ -1,3 +1,4 @@
+import { getDeployedTestAccountsWallets } from '@aztec/accounts/testing';
 import {
   AztecAddress,
   type AztecNode,
@@ -15,8 +16,10 @@ import {
   SignerlessWallet,
   TxStatus,
   type Wallet,
+  createPXEClient,
   getContractClassFromArtifact,
   getContractInstanceFromDeployParams,
+  makeFetch,
 } from '@aztec/aztec.js';
 import {
   broadcastPrivateFunction,
@@ -157,6 +160,25 @@ describe('e2e_deploy_contract', () => {
     }, 90_000);
   });
 
+  describe('regressions', () => {
+    beforeAll(async () => {
+      ({ teardown, pxe, logger, wallet, sequencer, aztecNode } = await setup());
+    }, 100_000);
+    afterAll(() => teardown());
+
+    it('fails properly when trying to deploy a contract with a failing constructor with a pxe client with retries', async () => {
+      const { PXE_URL } = process.env;
+      if (!PXE_URL) {
+        return;
+      }
+      const pxeClient = createPXEClient(PXE_URL, makeFetch([1, 2, 3], false));
+      const [wallet] = await getDeployedTestAccountsWallets(pxeClient);
+      await expect(
+        StatefulTestContract.deployWithOpts({ wallet, method: 'wrong_constructor' }).send().deployed(),
+      ).rejects.toThrow(/Unknown function/);
+    });
+  });
+
   describe('private initialization', () => {
     beforeAll(async () => {
       ({ teardown, pxe, logger, wallet, sequencer, aztecNode } = await setup());

yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts

@@ -19,7 +19,7 @@ const log = createDebugLogger('json-rpc:json_rpc_client');
  * @param host - The host URL.
  * @param method - The RPC method name.
  * @param body - The RPC payload.
- * @param noRetry - Whether to throw a `NoRetryError` in case the response is not ok and the body contains an error
+ * @param noRetry - Whether to throw a `NoRetryError` in case the response is a 5xx error and the body contains an error
  *                  message (see `retry` function for more details).
  * @returns The parsed JSON response, or throws an error.
  */
@@ -56,7 +56,7 @@ export async function defaultFetch(
     throw new Error(`Failed to parse body as JSON: ${resp.text()}`);
   }
   if (!resp.ok) {
-    if (noRetry) {
+    if (noRetry || (resp.status >= 400 && resp.status < 500)) {
       throw new NoRetryError('(JSON-RPC PROPAGATED) ' + responseJson.error.message);
     } else {
       throw new Error('(JSON-RPC PROPAGATED) ' + responseJson.error.message);