-
Notifications
You must be signed in to change notification settings - Fork 114
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(superchain): enable local testing #762
base: main
Are you sure you want to change the base?
Conversation
ae0da92
to
77d1d24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be a great improvement, but as it stands its not obvious why failures occur if trying to run validations locally without being connected to tailscale exit nodes. Forcing external users to "read the docs" to troubleshoot validations that previously worked is likely to cause friction. So I think it hurts the devx right now.
If we're going to merge this work, I think we should lift the L1 client connection so that we only attempt that once, instead of inside each validation. If the connection is successful, we pass that client connection to each underlying validation that needs it. If the connection fails (due to no tailscale connection), we should give a clear error message upfront (without running the full validation suite) about what to try (i.e. replace the rpcUrl in the relevant areas, and the url must be to an L1 archive node), and/or default/fallback to the current public full node urls
2bbdd8a
to
60809e7
Compare
60809e7
to
7c20698
Compare
7c20698
to
4353e9a
Compare
Description
This PR modifies the
superchain
configuration initialization logic to replace thePublicRPC
endpoints not only in CI but also when running validation tests locally, as many of them require access to an L1 archive node in order to function properly. The previously-hardcoded URLs are extracted totest_rpc
config values next to the existingpublic_rpc
values they override.This change required a modification to the
proxyd-l1-ci
IP whitelists to whitelist internal Tailnet IPs.Additional Context
See a discussion of the issue here.
The initial PR whitelisting Tailnet IPs was misconfigured, but then fixed in this followup PR.