Skip to content

fix: uniswap_trade_on_l1_from_l2.test.ts#12389

Merged
benesjan merged 2 commits into
masterfrom
02-28-fix_uniswap_trade_on_l1_from_l2.test.ts
Mar 1, 2025
Merged

fix: uniswap_trade_on_l1_from_l2.test.ts#12389
benesjan merged 2 commits into
masterfrom
02-28-fix_uniswap_trade_on_l1_from_l2.test.ts

Conversation

@benesjan

Copy link
Copy Markdown
Contributor

Fixes #12286

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@benesjan benesjan force-pushed the 02-28-fix_uniswap_trade_on_l1_from_l2.test.ts branch from d5fcb8e to 6f74cf5 Compare February 28, 2025 20:48
@benesjan benesjan marked this pull request as ready for review February 28, 2025 20:49
@benesjan benesjan requested a review from charlielye as a code owner February 28, 2025 20:49

const { publicClient, walletClient } = createL1Clients(this.aztecNodeConfig.l1RpcUrls, MNEMONIC);

const underlyingERC20Address = await deployL1Contract(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this be in the CrossChainTestHarness and having the underlyingERC20Address passed to the CrossChainTestHarness.new function just sometimes made it all hard to debug and quite ugly so I decided to just move it here.

});

// allow anyone to mint
await underlyingERC20.write.setFreeForAll([true], {} as any);

@benesjan benesjan Feb 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just didn't work at all in cases where underlyingERC20Address was provided on the input because that was typically WETH or DAI address from forked mainnet and those contracts simply did not have setFreeForAll method. This change sneaked into master because the uniswap_trade_on_l1_from_l2.test.ts was never run in the PR that introduced the change.

@benesjan benesjan changed the title fix: uniswap_trade_on_l1_from_l2.test.ts fix: uniswap_trade_on_l1_from_l2.test.ts Feb 28, 2025
@benesjan benesjan enabled auto-merge (squash) February 28, 2025 21:27
@benesjan benesjan merged commit 288c057 into master Mar 1, 2025
@benesjan benesjan deleted the 02-28-fix_uniswap_trade_on_l1_from_l2.test.ts branch March 1, 2025 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix uniswap_trade_on_l1_from_l2.test.ts and add it to CI

2 participants