feat: not retrying unrecoverable errors (#1752)

Fixes #1511 
Fixes #1724

With this PR all the errors thrown in the server code are considered to
be unrecoverable. Recoverable errors should not be errors and should be
handled (or shown only as warnings). For example I refactored the
`registerAccount` and `registerRecipient` to not throw if we add the
same recipient/account twice because that situation is easily
recoverable (just ignore it).

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [ ] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [ ] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts

@@ -92,10 +92,15 @@ export class AztecRPCServer implements AztecRPC {
   }
 
   public async registerAccount(privKey: PrivateKey, partialAddress: PartialAddress) {
-    const pubKey = this.keyStore.addAccount(privKey);
     const completeAddress = await CompleteAddress.fromPrivateKeyAndPartialAddress(privKey, partialAddress);
-    await this.db.addCompleteAddress(completeAddress);
-    this.synchroniser.addAccount(pubKey, this.keyStore);
+    const wasAdded = await this.db.addCompleteAddress(completeAddress);
+    if (wasAdded) {
+      const pubKey = this.keyStore.addAccount(privKey);
+      this.synchroniser.addAccount(pubKey, this.keyStore);
+      this.log.info(`Added account: ${completeAddress.toReadableString()}`);
+    } else {
+      this.log.info(`Account "${completeAddress.toReadableString()}" already registered.`);
+    }
   }
 
   public async getAccounts(): Promise<CompleteAddress[]> {
@@ -114,8 +119,12 @@ export class AztecRPCServer implements AztecRPC {
   }
 
   public async registerRecipient(recipient: CompleteAddress): Promise<void> {
-    await this.db.addCompleteAddress(recipient);
-    this.log.info(`Added recipient: ${recipient.toString()}`);
+    const wasAdded = await this.db.addCompleteAddress(recipient);
+    if (wasAdded) {
+      this.log.info(`Added recipient: ${recipient.toReadableString()}`);
+    } else {
+      this.log.info(`Recipient "${recipient.toReadableString()}" already registered.`);
+    }
   }
 
   public async getRecipients(): Promise<CompleteAddress[]> {

yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts

@@ -1,4 +1,4 @@
-import { AztecAddress, CompleteAddress, Fr, FunctionData, TxContext } from '@aztec/circuits.js';
+import { AztecAddress, CompleteAddress, Fr, FunctionData, Point, TxContext } from '@aztec/circuits.js';
 import { Grumpkin } from '@aztec/circuits.js/barretenberg';
 import { ConstantKeyPair } from '@aztec/key-store';
 import {
@@ -57,26 +57,32 @@ export const aztecRpcTestSuite = (testName: string, aztecRpcSetup: () => Promise
       expect(recipient).toEqual(completeAddress);
     });
 
-    it('cannot register the same account twice', async () => {
+    it('does not throw when registering the same account twice (just ignores the second attempt)', async () => {
       const keyPair = ConstantKeyPair.random(await Grumpkin.new());
       const completeAddress = await CompleteAddress.fromPrivateKeyAndPartialAddress(
         await keyPair.getPrivateKey(),
         Fr.random(),
       );
 
       await rpc.registerAccount(await keyPair.getPrivateKey(), completeAddress.partialAddress);
-      await expect(async () =>
-        rpc.registerAccount(await keyPair.getPrivateKey(), completeAddress.partialAddress),
-      ).rejects.toThrow(`Complete address corresponding to ${completeAddress.address} already exists`);
+      await rpc.registerAccount(await keyPair.getPrivateKey(), completeAddress.partialAddress);
     });
 
-    it('cannot register the same recipient twice', async () => {
+    it('cannot register a recipient with the same aztec address but different pub key or partial address', async () => {
+      const recipient1 = await CompleteAddress.random();
+      const recipient2 = new CompleteAddress(recipient1.address, Point.random(), Fr.random());
+
+      await rpc.registerRecipient(recipient1);
+      await expect(() => rpc.registerRecipient(recipient2)).rejects.toThrow(
+        `Complete address with aztec address ${recipient1.address}`,
+      );
+    });
+
+    it('does not throw when registering the same recipient twice (just ignores the second attempt)', async () => {
       const completeAddress = await CompleteAddress.random();
 
       await rpc.registerRecipient(completeAddress);
-      await expect(() => rpc.registerRecipient(completeAddress)).rejects.toThrow(
-        `Complete address corresponding to ${completeAddress.address} already exists`,
-      );
+      await rpc.registerRecipient(completeAddress);
     });
 
     it('successfully adds a contract', async () => {

yarn-project/aztec-rpc/src/database/database.ts

@@ -127,9 +127,11 @@ export interface Database extends ContractDatabase {
   /**
    * Adds complete address to the database.
    * @param address - The complete address to add.
-   * @returns Empty promise.
+   * @returns A promise resolving to true if the address was added, false if it already exists.
+   * @throws If we try to add a CompleteAddress with the same AztecAddress but different public key or partial
+   * address.
    */
-  addCompleteAddress(address: CompleteAddress): Promise<void>;
+  addCompleteAddress(address: CompleteAddress): Promise<boolean>;
 
   /**
    * Retrieves the complete address corresponding to the provided aztec address.

yarn-project/aztec-rpc/src/database/memory_db.ts

@@ -121,15 +121,19 @@ export class MemoryDB extends MemoryContractDatabase implements Database {
     });
   }
 
-  public addCompleteAddress(completeAddress: CompleteAddress): Promise<void> {
+  public addCompleteAddress(completeAddress: CompleteAddress): Promise<boolean> {
     const accountIndex = this.addresses.findIndex(r => r.address.equals(completeAddress.address));
     if (accountIndex !== -1) {
+      if (this.addresses[accountIndex].equals(completeAddress)) {
+        return Promise.resolve(false);
+      }
+
       throw new Error(
-        `Complete address corresponding to ${completeAddress.address.toString()} already exists in memory database`,
+        `Complete address with aztec address ${completeAddress.address.toString()} but different public key or partial key already exists in memory database`,
       );
     }
     this.addresses.push(completeAddress);
-    return Promise.resolve();
+    return Promise.resolve(true);
   }
 
   public getCompleteAddress(address: AztecAddress): Promise<CompleteAddress | undefined> {

yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts

@@ -53,7 +53,7 @@ const ethRpcUrl = ETHEREUM_HOST;
 const hdAccount = mnemonicToAccount(MNEMONIC);
 const privateKey = new PrivateKey(Buffer.from(hdAccount.getHdKey().privateKey!));
 
-const aztecRpcClient = createAztecRpcClient(aztecRpcUrl, makeFetch([1, 2, 3], false));
+const aztecRpcClient = createAztecRpcClient(aztecRpcUrl, makeFetch([1, 2, 3], true));
 let wallet: Wallet;
 
 /**

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

@@ -57,7 +57,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => {
   let page: Page;
 
   beforeAll(async () => {
-    testClient = AztecJs.createAztecRpcClient(SANDBOX_URL!, AztecJs.makeFetch([1, 2, 3], false));
+    testClient = AztecJs.createAztecRpcClient(SANDBOX_URL!, AztecJs.makeFetch([1, 2, 3], true));
     await AztecJs.waitForSandbox(testClient);
 
     app = new Koa();
@@ -111,7 +111,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => {
     const result = await page.evaluate(
       async (rpcUrl, privateKeyString) => {
         const { PrivateKey, createAztecRpcClient, makeFetch, getUnsafeSchnorrAccount } = window.AztecJs;
-        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false));
+        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], true));
         const privateKey = PrivateKey.fromString(privateKeyString);
         const account = getUnsafeSchnorrAccount(client, privateKey);
         await account.waitDeploy();
@@ -136,7 +136,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => {
     const result = await page.evaluate(
       async (rpcUrl, contractAddress, PrivateTokenContractAbi) => {
         const { Contract, AztecAddress, createAztecRpcClient, makeFetch } = window.AztecJs;
-        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false));
+        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], true));
         const owner = (await client.getAccounts())[0].address;
         const wallet = await AztecJs.getSandboxAccountsWallet(client);
         const contract = await Contract.at(AztecAddress.fromString(contractAddress), PrivateTokenContractAbi, wallet);
@@ -156,7 +156,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => {
       async (rpcUrl, contractAddress, transferAmount, PrivateTokenContractAbi) => {
         console.log(`Starting transfer tx`);
         const { AztecAddress, Contract, createAztecRpcClient, makeFetch } = window.AztecJs;
-        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false));
+        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], true));
         const accounts = await client.getAccounts();
         const owner = accounts[0].address;
         const receiver = accounts[1].address;
@@ -178,7 +178,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => {
     const txHash = await page.evaluate(
       async (rpcUrl, privateKeyString, initialBalance, PrivateTokenContractAbi) => {
         const { PrivateKey, DeployMethod, createAztecRpcClient, makeFetch, getUnsafeSchnorrAccount } = window.AztecJs;
-        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false));
+        const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], true));
         let accounts = await client.getAccounts();
         if (accounts.length === 0) {
           // This test needs an account for deployment. We create one in case there is none available in the RPC server.

yarn-project/end-to-end/src/fixtures/utils.ts

@@ -80,7 +80,7 @@ const createRpcServer = async (
 ): Promise<AztecRPC> => {
   if (SANDBOX_URL) {
     logger(`Creating JSON RPC client to remote host ${SANDBOX_URL}`);
-    const jsonClient = createJsonRpcClient(SANDBOX_URL, makeFetch([1, 2, 3], false));
+    const jsonClient = createJsonRpcClient(SANDBOX_URL, makeFetch([1, 2, 3], true));
     await waitForRPCServer(jsonClient, logger);
     logger('JSON RPC client connected to RPC Server');
     return jsonClient;

yarn-project/types/src/interfaces/aztec_rpc.ts

@@ -81,7 +81,6 @@ export interface AztecRPC {
    *          This is because we don't have the associated private key and for this reason we can't decrypt
    *          the recipient's notes. We can send notes to this account because we can encrypt them with the recipient's
    *          public key.
-   * @throws If the recipient is already registered.
    */
   registerRecipient(recipient: CompleteAddress): Promise<void>;