chore: make canary uniswap test similar to e2e #2767

rahul-kothari · 2023-10-10T15:43:41Z

added the cross-chain test harness in Canary as well, so that e2e tests and Canary tests look the same. This has some advantages:

In the mid-term, I know we were planning to throw away canary tests and have e2e run as canary tests. This makes it easier for cross-chain tests to be replaced in canary tests.
Cross-chain tests can now for sure be run on the sandbox. This serves as a green flag that we don't do anything wonky in e2e that isn't available in the sandbox
e2e cross-chain tests can now be used in docs via #include_code with minimal copy-paste. Without the canary version of the cross-chain test harness, I would have to hardcode a lot in the docs. This makes it easily maintainable.

rahul-kothari · 2023-10-10T15:51:48Z

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

-    const wethL1BeforeBalance = await wethContract.read.balanceOf([ownerEthAddress.toString()]);
-    // 1. Approve weth to be bridged
-    await wethContract.write.approve([wethTokenPortalAddress.toString(), wethAmountToBridge], {} as any);
+  it('should uniswap trade on L1 from L2 funds privately (swaps WETH -> DAI)', async () => {


everything from this line onwards is an exact copy-paste of end-to-end/uniswap_trade_on_l1_from_l2.test.ts!

Similar to my comment later. There should be a way such that we can simply rerun the same ones instead of needing to copy them around like this.

rahul-kothari · 2023-10-10T15:52:41Z

yarn-project/canary/src/utils.ts

+ * shared between cross chain tests.
+ */
+export class CrossChainTestHarness {
+  static async new(


had to modify the CrossChainTestHarness.new() a bit to make it workable on sandbox. Otherwise it is mostly a copy paste!

Cannot we alter the CrossChainTestHarness such that the same can be used both places without having these two that are almost just copied.

The cheatcodes are not used for example, there is probably other stuff that could also be purged such that it can be used both places.

LHerskind · 2023-10-10T16:00:37Z

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

  createDebugLogger,
  createPXEClient,
-  getL1ContractAddresses,
+  sleep as delay,


LHerskind · 2023-10-10T16:03:24Z

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

+  const wethAmountToBridge = parseEther('1');
+  const uniswapFeeTier = 3000n;
+  const minimumOutputAmount = 0n;
+  const deadlineForDepositingSwappedDai = BigInt(2 ** 32 - 1); // max uint32 - 1


It is just max uint32 not - 1, recall that max uint32 = 2**32 -1.

LHerskind · 2023-10-10T16:07:42Z

yarn-project/canary/src/utils.ts

+ * shared between cross chain tests.
+ */
+export class CrossChainTestHarness {
+  static async new(


Cannot we alter the CrossChainTestHarness such that the same can be used both places without having these two that are almost just copied.

The cheatcodes are not used for example, there is probably other stuff that could also be purged such that it can be used both places.

LHerskind · 2023-10-10T16:10:49Z

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

-    const wethL1BeforeBalance = await wethContract.read.balanceOf([ownerEthAddress.toString()]);
-    // 1. Approve weth to be bridged
-    await wethContract.write.approve([wethTokenPortalAddress.toString(), wethAmountToBridge], {} as any);
+  it('should uniswap trade on L1 from L2 funds privately (swaps WETH -> DAI)', async () => {


Similar to my comment later. There should be a way such that we can simply rerun the same ones instead of needing to copy them around like this.

socket-security · 2023-10-11T13:10:35Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@libp2p/mplex	9.0.4	None	`+0`	179 kB	npm-service-account-libp2p

LHerskind

Minor nits. Really nice stuff 👍

LHerskind · 2023-10-11T12:53:14Z

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

-    logger('DAI balance after swap  : ' + daiL2BalanceAfterSwap.toString());
-  }, 140_000);
-});
+uniswapL1L2TestSuite(setupRPC, () => Promise.resolve(), EXPECTED_FORKED_BLOCK);


LHerskind · 2023-10-11T13:13:05Z

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

@@ -356,11 +322,4 @@ export class CrossChainTestHarness {
    const unshieldReceipt = await unshieldTx.wait();
    expect(unshieldReceipt.status).toBe(TxStatus.MINED);
  }
-
-  async stop() {


Should you not still keep the stop, just without the aztecNode now?

no teardown() from e2e setup is enough! teardown() has access to the node and kills it

LHerskind · 2023-10-11T13:32:34Z

yarn-project/archiver/src/archiver/config.ts

@@ -71,7 +72,7 @@ export function getConfigEnvVars(): ArchiverConfig {
    rollupAddress: ROLLUP_CONTRACT_ADDRESS ? EthAddress.fromString(ROLLUP_CONTRACT_ADDRESS) : EthAddress.ZERO,
    registryAddress: REGISTRY_CONTRACT_ADDRESS ? EthAddress.fromString(REGISTRY_CONTRACT_ADDRESS) : EthAddress.ZERO,
    inboxAddress: INBOX_CONTRACT_ADDRESS ? EthAddress.fromString(INBOX_CONTRACT_ADDRESS) : EthAddress.ZERO,
-    outboxAddress: EthAddress.ZERO,
+    outboxAddress: OUTBOX_CONTRACT_ADDRESS ? EthAddress.fromString(OUTBOX_CONTRACT_ADDRESS) : EthAddress.ZERO,


LHerskind · 2023-10-11T13:32:55Z

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

@@ -26,9 +26,9 @@ export class CrossChainTestHarness {
    const ethAccount = EthAddress.fromString((await walletClient.getAddresses())[0]);
    const owner = wallet.getCompleteAddress();
    const l1ContractAddresses = (await pxeService.getNodeInfo()).l1ContractAddresses;
-
+    


AztecBot · 2023-10-11T14:25:42Z

Benchmark results

All benchmarks are run on txs on the Benchmarking contract on the repository. Each tx consists of a batch call to create_note and increment_balance, which guarantees that each tx has a private call, a nested private call, a public call, and a nested public call, as well as an emitted private note, an unencrypted log, and public storage read and write.

L2 block published to L1

Each column represents the number of txs on an L2 block published to L1.

Metric	8 txs	32 txs	128 txs
l1_rollup_calldata_size_in_bytes	45444	179588	716132
l1_rollup_calldata_gas	222996	868184	3449456
l1_rollup_execution_gas	842083	3595292	22204825
l2_block_processing_time_in_ms	1091	4176	15124
note_successful_decrypting_time_in_ms	340	1056	3573
note_trial_decrypting_time_in_ms	42	107	136
l2_block_building_time_in_ms	8986	37382	150507
l2_block_rollup_simulation_time_in_ms	6725	27816	106372
l2_block_public_tx_process_time_in_ms	2218	9434	43478

L2 chain processing

Each column represents the number of blocks on the L2 chain where each block has 16 txs.

Metric	10 blocks	20 blocks	30 blocks
node_history_sync_time_in_ms	31315	76095	137120
note_history_successful_decrypting_time_in_ms	4774	13206	20023
note_history_trial_decrypting_time_in_ms	146	255	262
node_database_size_in_bytes	1191378	1902330	2755457
pxe_database_size_in_bytes	54187	108338	162578

Circuits stats

Stats on running time and I/O sizes collected for every circuit run across all benchmarks.

Circuit	circuit_simulation_time_in_ms	circuit_input_size_in_bytes	circuit_output_size_in_bytes
private-kernel-init	55.67791411042945	56577	14745
private-kernel-ordering	30.117331288343557	20137	8089
base-rollup	871	631604	810
root-rollup	38.24390243902439	4072	1097
private-kernel-inner	51.95524691358025	72288	14745
public-kernel-private-input	51.574074074074076	37359	14745
public-kernel-non-first-iteration	31.22530864197531	37401	14745
merge-rollup	1.0348837209302326	2592	873

rahul-kothari requested review from LHerskind and Maddiaa0 October 10, 2023 15:43

rahul-kothari changed the title ~~make canary uniswap test similar to e2e~~ chore: make canary uniswap test similar to e2e Oct 10, 2023

rahul-kothari force-pushed the rk/canary_similar branch from a43c633 to d4fd732 Compare October 10, 2023 15:49

rahul-kothari commented Oct 10, 2023

View reviewed changes

rahul-kothari force-pushed the rk/canary_similar branch from d4fd732 to 5f00639 Compare October 10, 2023 16:02

LHerskind reviewed Oct 10, 2023

View reviewed changes

rahul-kothari force-pushed the rk/canary_similar branch 2 times, most recently from 0cd6197 to 4af81f3 Compare October 11, 2023 12:15

rahul-kothari requested a review from LHerskind October 11, 2023 12:33

rahul-kothari force-pushed the rk/canary_similar branch from b90038c to e4a649d Compare October 11, 2023 13:05

LHerskind reviewed Oct 11, 2023

View reviewed changes

rahul-kothari enabled auto-merge (squash) October 11, 2023 13:23

rahul-kothari added 6 commits October 11, 2023 13:29

make canary uniswap test similar to e2e

5fecf87

alter e2e crosschain harness to be the same as canary

5e9a044

e2e and canary use same suite for uniswap

f7beb68

remove uneeded imports

6ac2a3a

store outbox in config

55df0d4

deadline comment

1b9f555

LHerskind reviewed Oct 11, 2023

View reviewed changes

LHerskind approved these changes Oct 11, 2023

View reviewed changes

prettier is saving my life

72a3e52

rahul-kothari force-pushed the rk/canary_similar branch from d7e06bf to 72a3e52 Compare October 11, 2023 13:42

rahul-kothari merged commit 93d458b into master Oct 11, 2023

rahul-kothari deleted the rk/canary_similar branch October 11, 2023 14:05

AztecBot mentioned this pull request Oct 11, 2023

chore(master): Release 0.8.10 #2766

Merged

spypsy mentioned this pull request Oct 13, 2023

e2e: Move all canary tests to end-to-end directory #2303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: make canary uniswap test similar to e2e #2767

chore: make canary uniswap test similar to e2e #2767

rahul-kothari commented Oct 10, 2023

rahul-kothari Oct 10, 2023

LHerskind Oct 10, 2023

rahul-kothari Oct 10, 2023

LHerskind Oct 10, 2023

LHerskind Oct 10, 2023

LHerskind Oct 10, 2023

LHerskind Oct 10, 2023

LHerskind Oct 10, 2023

socket-security bot commented Oct 11, 2023

LHerskind left a comment

LHerskind Oct 11, 2023

LHerskind Oct 11, 2023

rahul-kothari Oct 11, 2023 •

edited

Loading

LHerskind Oct 11, 2023

LHerskind Oct 11, 2023

AztecBot commented Oct 11, 2023

chore: make canary uniswap test similar to e2e #2767

chore: make canary uniswap test similar to e2e #2767

Conversation

rahul-kothari commented Oct 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

socket-security bot commented Oct 11, 2023

LHerskind left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahul-kothari Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AztecBot commented Oct 11, 2023

Benchmark results

L2 block published to L1

L2 chain processing

Circuits stats

rahul-kothari Oct 11, 2023 •

edited

Loading