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(e2e): deploy and configure both layers with the e2e script #717

Merged
merged 15 commits into from
Nov 23, 2022

Conversation

tmigone
Copy link
Contributor

@tmigone tmigone commented Sep 22, 2022

Motivation

Previously the e2e testing framework would allow running tests on either an L1 or L2 deployment, with cross-chain messages not being a possibility.

This PR updates the e2e framework so we can now deploy both L1 and L2, configure them fully (including bridge) and have the ability to send L1 <> L2 messages. This requires setting up Arbitrum Nitro local testnodes using the Edge & Node patched fork: https://github.com/edgeandnode/nitro. A new simple scenario that uses the bridge to send GRT from L1 to L2 is also included.

Also updating the GitHub CI action to make use of this, a tl;dr of what the action will now be doing:

  • Start an L1 node (geth) and an L2 sequencer connected to it.
  • Deploy arbitrum's contracts to L1/L2
  • Deploy The Graph protocol to L1/L2
  • Configure the protocol (L1, L2, bridge)
  • Run e2e tests on both L1 and L2 to check everything looks good
  • Run e2e scenarios on both L1 and L2
    • Publish subgraphs, add signal
    • Open allocations
    • Send GRT from L1 to L2
    • Close allocations (Not working yet, need to figure out a way of advancing time)

Coming up next:

  • Fully test the bridge configuration (including cross checking the addresses on both ends, contract ownership, auth, etc).
  • Test more operations
    • Send GRT to L2 without ticket autoredeem
    • Send GRT with additional calldata
    • Send GRT from L2 to L1

Changes

  • Update @arbitrum/sdk to v3.0.0-rc1
  • Improve some deployment tasks
  • Add some new nitro specific tasks
  • Update e2e testing framework to allow running L1 and L2 tests on a configured L1/L2 deployment (including bridge)
  • Update github action to run the L1/L2 e2e tests on nitro local testnodes.
  • Added a new e2e scenario to send GRT from L1 to L2

@tmigone tmigone changed the base branch from dev to pcv/l2-bridge September 22, 2022 21:44
@tmigone tmigone marked this pull request as draft September 22, 2022 21:45
@tmigone tmigone changed the title Tmigone/e2e l2 bridge feat(e2e): deploy and setup both layers with the e2e script Sep 22, 2022
@tmigone tmigone changed the title feat(e2e): deploy and setup both layers with the e2e script feat(e2e): deploy and configure both layers with the e2e script Sep 22, 2022
@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from b3b7d84 to 68fdb01 Compare September 23, 2022 19:00
@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Base: 91.58% // Head: 91.58% // No change to project coverage 👍

Coverage data is based on head (ae1d70b) compared to base (caf5ab5).
Patch has no changes to coverable lines.

Additional details and impacted files
@@                      Coverage Diff                      @@
##           pcv/fix-bridge-callhook-data     #717   +/-   ##
=============================================================
  Coverage                         91.58%   91.58%           
=============================================================
  Files                                42       42           
  Lines                              1997     1997           
  Branches                            361      361           
=============================================================
  Hits                               1829     1829           
  Misses                              168      168           
Flag Coverage Δ
unittests 91.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from 537b649 to d372172 Compare September 27, 2022 15:44
@tmigone tmigone requested a review from pcarranzav September 27, 2022 21:56
@tmigone tmigone marked this pull request as ready for review September 27, 2022 21:57
@tmigone tmigone requested a review from abarmat September 27, 2022 21:57
Copy link
Contributor

@abarmat abarmat left a comment

Choose a reason for hiding this comment

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

I tried running the e2e locally, it worked OK when doing L1 only, then I set the L2_NETWORK variable to localhost and got some errors, I'm not sure how to set it to make it run on a local env.

I got an error when doing L1_NETWORK=localnitrol1 L2_NETWORK=localnitrol2 yarn test:e2e -> ProviderError: sender doesn't have enough funds to send tx. The max upfront cost is: 31501500000000 and the sender's account only has: 0 - should I run the nitro setup separately?

@tmigone
Copy link
Contributor Author

tmigone commented Oct 3, 2022

I tried running the e2e locally, it worked OK when doing L1 only, then I set the L2_NETWORK variable to localhost and got some errors, I'm not sure how to set it to make it run on a local env.

The script makes the assumption that you are either running only on L1 or running both on L1/L2. I think running ONLY on L2 using a hardhat node (which is what you get with localhost) is not really a valid use case.

I got an error when doing L1_NETWORK=localnitrol1 L2_NETWORK=localnitrol2 yarn test:e2e -> ProviderError: sender doesn't have enough funds to send tx. The max upfront cost is: 31501500000000 and the sender's account only has: 0 - should I run the nitro setup separately?

Yes you need to setup the nitro testnodes separately. You can check the TESTING.md file for instructions.

Copy link
Contributor

@abarmat abarmat left a comment

Choose a reason for hiding this comment

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

I managed to get it running by looking at the instructions

Copy link
Member

@pcarranzav pcarranzav left a comment

Choose a reason for hiding this comment

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

Looking good, but let's block the merge until the base branch is done with audits

@tmigone tmigone requested a review from pcarranzav October 3, 2022 21:29
@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from cdc479f to fdecdc9 Compare October 3, 2022 21:36
@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from fa359e9 to 6de2ea9 Compare October 6, 2022 17:39
@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from af67ed0 to 4a8e57b Compare November 2, 2022 13:38
@tmigone tmigone changed the base branch from pcv/l2-bridge to pcv/fix-bridge-callhook-data November 2, 2022 13:38
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
* feat(e2e): run L1 and L2 together in e2e tests

Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
@tmigone tmigone force-pushed the tmigone/e2e-l2-bridge branch from 4a8e57b to ae1d70b Compare November 2, 2022 14:10
Base automatically changed from pcv/fix-bridge-callhook-data to pcv/l2-bridge November 23, 2022 14:38
@pcarranzav pcarranzav merged commit 60afb9e into pcv/l2-bridge Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants