From 443273c20c2b24b206cfbf4198c8c6b179cf0b37 Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 23 Aug 2023 09:54:44 +0000 Subject: [PATCH] refactor: not throwing when adding a recipient or account twice --- .../src/aztec_rpc_server/aztec_rpc_server.ts | 21 ++++++++++++---- .../test/aztec_rpc_test_suite.ts | 24 ++++++++++++------- .../aztec-rpc/src/database/database.ts | 6 +++-- .../aztec-rpc/src/database/memory_db.ts | 10 +++++--- .../types/src/interfaces/aztec_rpc.ts | 1 - 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index ff7a7afb8240..d4fb2b540d06 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -92,10 +92,17 @@ 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( + `Skipped adding account "${completeAddress.toReadableString()}" because he/she was registered before.`, + ); + } } public async getAccounts(): Promise { @@ -114,8 +121,12 @@ export class AztecRPCServer implements AztecRPC { } public async registerRecipient(recipient: CompleteAddress): Promise { - 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(`Skipped adding recipient "${recipient.toReadableString()}" because he/she was registered before.`); + } } public async getRecipients(): Promise { diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts index 1e238c9cad99..0747e1149efa 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/test/aztec_rpc_test_suite.ts +++ b/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,7 +57,7 @@ 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(), @@ -65,18 +65,24 @@ export const aztecRpcTestSuite = (testName: string, aztecRpcSetup: () => Promise ); 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 () => { diff --git a/yarn-project/aztec-rpc/src/database/database.ts b/yarn-project/aztec-rpc/src/database/database.ts index d1cae3100a8a..e03467c5c9d9 100644 --- a/yarn-project/aztec-rpc/src/database/database.ts +++ b/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; + addCompleteAddress(address: CompleteAddress): Promise; /** * Retrieves the complete address corresponding to the provided aztec address. diff --git a/yarn-project/aztec-rpc/src/database/memory_db.ts b/yarn-project/aztec-rpc/src/database/memory_db.ts index c6b213159def..551569a2a428 100644 --- a/yarn-project/aztec-rpc/src/database/memory_db.ts +++ b/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 { + public addCompleteAddress(completeAddress: CompleteAddress): Promise { 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 { diff --git a/yarn-project/types/src/interfaces/aztec_rpc.ts b/yarn-project/types/src/interfaces/aztec_rpc.ts index 337f171f19af..199ffe75788b 100644 --- a/yarn-project/types/src/interfaces/aztec_rpc.ts +++ b/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;