Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: not retrying unrecoverable errors #1752

Merged
merged 3 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]> {
Expand All @@ -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[]> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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 () => {
Expand Down
6 changes: 4 additions & 2 deletions yarn-project/aztec-rpc/src/database/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 7 additions & 3 deletions yarn-project/aztec-rpc/src/database/memory_db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
10 changes: 5 additions & 5 deletions yarn-project/end-to-end/src/e2e_aztec_js_browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/src/fixtures/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion yarn-project/types/src/interfaces/aztec_rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down