From 95d1350b23b6205ff2a7d3de41a37e0bc9ee7640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Tue, 22 Aug 2023 18:25:57 +0200 Subject: [PATCH] feat: no unlimited retries by default in aztec.js (#1723) 1. Fixes #1722 2. Fixes e2e_aztec_js_browser test inter-dependence (running a test case with `it.only` failed because it expected a previous `it` to be executed first) --- .../src/aztec_rpc_client/aztec_rpc_client.ts | 3 +- .../src/uniswap_trade_on_l1_from_l2.test.ts | 4 +- .../end-to-end/src/aztec_rpc_sandbox.test.ts | 4 +- .../src/e2e_aztec_js_browser.test.ts | 90 ++++++++++--------- yarn-project/end-to-end/src/fixtures/utils.ts | 4 +- .../foundation/src/json-rpc/client/index.ts | 8 +- .../src/json-rpc/client/json_rpc_client.ts | 20 ----- 7 files changed, 58 insertions(+), 75 deletions(-) diff --git a/yarn-project/aztec.js/src/aztec_rpc_client/aztec_rpc_client.ts b/yarn-project/aztec.js/src/aztec_rpc_client/aztec_rpc_client.ts index 432d33b80df..09ccdcad49f 100644 --- a/yarn-project/aztec.js/src/aztec_rpc_client/aztec_rpc_client.ts +++ b/yarn-project/aztec.js/src/aztec_rpc_client/aztec_rpc_client.ts @@ -12,8 +12,7 @@ import { TxReceipt, } from '@aztec/types'; -export { mustSucceedFetch } from '@aztec/foundation/json-rpc/client'; -export { mustSucceedFetchUnlessNoRetry } from '@aztec/foundation/json-rpc/client'; +export { makeFetch } from '@aztec/foundation/json-rpc/client'; export const createAztecRpcClient = (url: string, fetch = defaultFetch): AztecRPC => createJsonRpcClient( diff --git a/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts b/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts index 724e32ec44f..350a9ca2551 100644 --- a/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts +++ b/yarn-project/canary/src/uniswap_trade_on_l1_from_l2.test.ts @@ -10,7 +10,7 @@ import { createDebugLogger, getL1ContractAddresses, getSandboxAccountsWallet, - mustSucceedFetch, + makeFetch, sleep, waitForSandbox, } from '@aztec/aztec.js'; @@ -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, mustSucceedFetch); +const aztecRpcClient = createAztecRpcClient(aztecRpcUrl, makeFetch([1, 2, 3], false)); let wallet: Wallet; /** diff --git a/yarn-project/end-to-end/src/aztec_rpc_sandbox.test.ts b/yarn-project/end-to-end/src/aztec_rpc_sandbox.test.ts index 84082670d0e..8aecfd37424 100644 --- a/yarn-project/end-to-end/src/aztec_rpc_sandbox.test.ts +++ b/yarn-project/end-to-end/src/aztec_rpc_sandbox.test.ts @@ -1,10 +1,10 @@ import { aztecRpcTestSuite } from '@aztec/aztec-rpc'; -import { createAztecRpcClient, mustSucceedFetchUnlessNoRetry, waitForSandbox } from '@aztec/aztec.js'; +import { createAztecRpcClient, makeFetch, waitForSandbox } from '@aztec/aztec.js'; const { SANDBOX_URL = 'http://localhost:8080' } = process.env; const setup = async () => { - const aztecRpc = createAztecRpcClient(SANDBOX_URL, mustSucceedFetchUnlessNoRetry); + const aztecRpc = createAztecRpcClient(SANDBOX_URL, makeFetch([1, 2, 3], true)); await waitForSandbox(aztecRpc); return aztecRpc; }; diff --git a/yarn-project/end-to-end/src/e2e_aztec_js_browser.test.ts b/yarn-project/end-to-end/src/e2e_aztec_js_browser.test.ts index 6d7584dd5f1..d2d684a2fd5 100644 --- a/yarn-project/end-to-end/src/e2e_aztec_js_browser.test.ts +++ b/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.mustSucceedFetch); + testClient = AztecJs.createAztecRpcClient(SANDBOX_URL!, AztecJs.makeFetch([1, 2, 3], false)); await AztecJs.waitForSandbox(testClient); app = new Koa(); @@ -110,8 +110,8 @@ conditionalDescribe()('e2e_aztec.js_browser', () => { it('Creates an account', async () => { const result = await page.evaluate( async (rpcUrl, privateKeyString) => { - const { PrivateKey, createAztecRpcClient, mustSucceedFetch, getUnsafeSchnorrAccount } = window.AztecJs; - const client = createAztecRpcClient(rpcUrl!, mustSucceedFetch); + const { PrivateKey, createAztecRpcClient, makeFetch, getUnsafeSchnorrAccount } = window.AztecJs; + const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false)); const privateKey = PrivateKey.fromString(privateKeyString); const account = getUnsafeSchnorrAccount(client, privateKey); await account.waitDeploy(); @@ -129,44 +129,14 @@ conditionalDescribe()('e2e_aztec.js_browser', () => { }, 15_000); it('Deploys Private Token contract', async () => { - const txHash = await page.evaluate( - async (rpcUrl, privateKeyString, initialBalance, PrivateTokenContractAbi) => { - const { PrivateKey, DeployMethod, createAztecRpcClient, mustSucceedFetch, getUnsafeSchnorrAccount } = - window.AztecJs; - const client = createAztecRpcClient(rpcUrl!, mustSucceedFetch); - 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. - const privateKey = PrivateKey.fromString(privateKeyString); - await getUnsafeSchnorrAccount(client, privateKey).waitDeploy(); - accounts = await client.getAccounts(); - } - const owner = accounts[0]; - const tx = new DeployMethod(owner.publicKey, client, PrivateTokenContractAbi, [ - initialBalance, - owner.address, - ]).send(); - await tx.wait(); - const receipt = await tx.getReceipt(); - console.log(`Contract Deployed: ${receipt.contractAddress}`); - return receipt.txHash.toString(); - }, - SANDBOX_URL, - privKey.toString(), - initialBalance, - PrivateTokenContractAbi, - ); - - const txResult = await testClient.getTxReceipt(AztecJs.TxHash.fromString(txHash)); - expect(txResult.status).toEqual(AztecJs.TxStatus.MINED); - contractAddress = txResult.contractAddress!; + await deployPrivateTokenContract(); }, 30_000); it("Gets the owner's balance", async () => { const result = await page.evaluate( async (rpcUrl, contractAddress, PrivateTokenContractAbi) => { - const { Contract, AztecAddress, createAztecRpcClient, mustSucceedFetch } = window.AztecJs; - const client = createAztecRpcClient(rpcUrl!, mustSucceedFetch); + const { Contract, AztecAddress, createAztecRpcClient, makeFetch } = window.AztecJs; + const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false)); const owner = (await client.getAccounts())[0].address; const wallet = await AztecJs.getSandboxAccountsWallet(client); const contract = await Contract.at(AztecAddress.fromString(contractAddress), PrivateTokenContractAbi, wallet); @@ -174,7 +144,7 @@ conditionalDescribe()('e2e_aztec.js_browser', () => { return balance; }, SANDBOX_URL, - contractAddress.toString(), + (await getPrivateTokenAddress()).toString(), PrivateTokenContractAbi, ); logger('Owner balance:', result); @@ -185,8 +155,8 @@ conditionalDescribe()('e2e_aztec.js_browser', () => { const result = await page.evaluate( async (rpcUrl, contractAddress, transferAmount, PrivateTokenContractAbi) => { console.log(`Starting transfer tx`); - const { AztecAddress, Contract, createAztecRpcClient, mustSucceedFetch } = window.AztecJs; - const client = createAztecRpcClient(rpcUrl!, mustSucceedFetch); + const { AztecAddress, Contract, createAztecRpcClient, makeFetch } = window.AztecJs; + const client = createAztecRpcClient(rpcUrl!, makeFetch([1, 2, 3], false)); const accounts = await client.getAccounts(); const owner = accounts[0].address; const receiver = accounts[1].address; @@ -197,10 +167,50 @@ conditionalDescribe()('e2e_aztec.js_browser', () => { return await contract.methods.getBalance(receiver).view({ from: receiver }); }, SANDBOX_URL, - contractAddress.toString(), + (await getPrivateTokenAddress()).toString(), transferAmount, PrivateTokenContractAbi, ); expect(result).toEqual(transferAmount); }, 60_000); + + const deployPrivateTokenContract = async () => { + 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)); + 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. + const privateKey = PrivateKey.fromString(privateKeyString); + await getUnsafeSchnorrAccount(client, privateKey).waitDeploy(); + accounts = await client.getAccounts(); + } + const owner = accounts[0]; + const tx = new DeployMethod(owner.publicKey, client, PrivateTokenContractAbi, [ + initialBalance, + owner.address, + ]).send(); + await tx.wait(); + const receipt = await tx.getReceipt(); + console.log(`Contract Deployed: ${receipt.contractAddress}`); + return receipt.txHash.toString(); + }, + SANDBOX_URL, + privKey.toString(), + initialBalance, + PrivateTokenContractAbi, + ); + + const txResult = await testClient.getTxReceipt(AztecJs.TxHash.fromString(txHash)); + expect(txResult.status).toEqual(AztecJs.TxStatus.MINED); + contractAddress = txResult.contractAddress!; + }; + + const getPrivateTokenAddress = async () => { + if (!contractAddress) { + await deployPrivateTokenContract(); + } + return contractAddress; + }; }); diff --git a/yarn-project/end-to-end/src/fixtures/utils.ts b/yarn-project/end-to-end/src/fixtures/utils.ts index 53fa13034ad..04bf9ea89fa 100644 --- a/yarn-project/end-to-end/src/fixtures/utils.ts +++ b/yarn-project/end-to-end/src/fixtures/utils.ts @@ -15,12 +15,12 @@ import { getL1ContractAddresses, getSandboxAccountsWallet, getUnsafeSchnorrAccount, + makeFetch, } from '@aztec/aztec.js'; import { CompleteAddress, PrivateKey, PublicKey } from '@aztec/circuits.js'; import { DeployL1Contracts, deployL1Contract, deployL1Contracts } from '@aztec/ethereum'; import { ContractAbi } from '@aztec/foundation/abi'; import { Fr } from '@aztec/foundation/fields'; -import { mustSucceedFetch } from '@aztec/foundation/json-rpc/client'; import { DebugLogger, createDebugLogger } from '@aztec/foundation/log'; import { retryUntil } from '@aztec/foundation/retry'; import { PortalERC20Abi, PortalERC20Bytecode, TokenPortalAbi, TokenPortalBytecode } from '@aztec/l1-artifacts'; @@ -80,7 +80,7 @@ const createRpcServer = async ( ): Promise => { if (SANDBOX_URL) { logger(`Creating JSON RPC client to remote host ${SANDBOX_URL}`); - const jsonClient = createJsonRpcClient(SANDBOX_URL, mustSucceedFetch); + const jsonClient = createJsonRpcClient(SANDBOX_URL, makeFetch([1, 2, 3], false)); await waitForRPCServer(jsonClient, logger); logger('JSON RPC client connected to RPC Server'); return jsonClient; diff --git a/yarn-project/foundation/src/json-rpc/client/index.ts b/yarn-project/foundation/src/json-rpc/client/index.ts index 6de7b7cd552..7e348e5323f 100644 --- a/yarn-project/foundation/src/json-rpc/client/index.ts +++ b/yarn-project/foundation/src/json-rpc/client/index.ts @@ -1,7 +1 @@ -export { - createJsonRpcClient, - mustSucceedFetch, - mustSucceedFetchUnlessNoRetry, - defaultFetch, - makeFetch, -} from './json_rpc_client.js'; +export { createJsonRpcClient, defaultFetch, makeFetch } from './json_rpc_client.js'; diff --git a/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts b/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts index a53799f7840..876f3d0fed8 100644 --- a/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts +++ b/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts @@ -65,13 +65,6 @@ export async function defaultFetch( return responseJson; } -/** - * A fetch function with retries. - */ -export async function mustSucceedFetch(host: string, rpcMethod: string, body: any, useApiEndpoints: boolean) { - return await retry(() => defaultFetch(host, rpcMethod, body, useApiEndpoints), 'JsonRpcClient request'); -} - /** * Makes a fetch function that retries based on the given attempts. * @param retries - Sequence of intervals (in seconds) to retry. @@ -90,19 +83,6 @@ export function makeFetch(retries: number[], noRetry: boolean, log?: DebugLogger }; } -/** - * A fetch function with retries unless the error is a NoRetryError. - */ -export async function mustSucceedFetchUnlessNoRetry( - host: string, - rpcMethod: string, - body: any, - useApiEndpoints: boolean, -) { - const noRetry = true; - return await retry(() => defaultFetch(host, rpcMethod, body, useApiEndpoints, noRetry), 'JsonRpcClient request'); -} - /** * Creates a Proxy object that delegates over RPC and satisfies RemoteObject. * The server should have ran new JsonRpcServer().