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

SUAVE dns #196

Merged
merged 24 commits into from
Feb 23, 2024
Merged

SUAVE dns #196

merged 24 commits into from
Feb 23, 2024

Conversation

jinmel
Copy link
Contributor

@jinmel jinmel commented Feb 15, 2024

📝 Summary

Introduces SuaveEthRemoteBackendEnpointV2 flag to support multi domain kettle. suave.eth.remote-endpoint now accepts rpc endpoint configuration formatted in chain_id=rpc_url

📚 References

Generalizing suave execution namespace for multiple chains: flashbots/suave-specs#79


  • I have seen and agree to CONTRIBUTING.md

@jinmel jinmel changed the title Introduce SuaveEthRemoteBackendEndpointV2 flag to support multiple Introduce SuaveEthRemoteBackendEndpointV2 flag to support multiple chains Feb 15, 2024
@jinmel jinmel changed the title Introduce SuaveEthRemoteBackendEndpointV2 flag to support multiple chains Introduce SuaveEthRemoteBackendEndpointV2 flag Feb 15, 2024
@jinmel jinmel changed the title Introduce SuaveEthRemoteBackendEndpointV2 flag Generalized suavex namespace Feb 15, 2024
@jinmel jinmel changed the title Generalized suavex namespace Multichain support for confidential eth backends Feb 15, 2024
@jinmel
Copy link
Contributor Author

jinmel commented Feb 15, 2024

Should I keep the backwards compatibility by introducing new precompiles with different signatures? (i.e. buildEthBlockV2, simulateBundleV2 ...) It seems like the integration tests are failing due to changes in precompiles @dmarzzz @ferranbt

@jinmel jinmel changed the title Multichain support for confidential eth backends SUAVE dns Feb 21, 2024
Copy link
Collaborator

@ferranbt ferranbt left a comment

Choose a reason for hiding this comment

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

Overall looks good, could you write a unit test for the contracts_suave.go change and then another e2e integration? (suave/e2e).

Another question, I think the kettle should also define the port of the service. WDYT?

@jinmel
Copy link
Contributor Author

jinmel commented Feb 21, 2024

@ferranbt Yeah I was also wondering if the kettle should set the port as well but that wouldn't be dns anymore. And also, if we are thinking of usage patterns like:

Suavex.simulateTxn("goerlisuavex", ...)

we should call this endpoint aliasing rather than dns. How about we just map a nickname such as "goerli" to a full url like "http://goerli.my-rpc.com:8545" rather than mimicking dns?

@jinmel
Copy link
Contributor Author

jinmel commented Feb 21, 2024

@ferranbt After giving it some thought, I believe it's a better practice for developers if we resolve an alias name directly to its full URL, port included. It simplifies things by hiding the complexities of the URL endpoint. Let me know if you think otherwise

Copy link
Collaborator

@ferranbt ferranbt left a comment

Choose a reason for hiding this comment

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

Only one comment to resolve but it is minor since the tests work.

core/vm/contracts_suave.go Show resolved Hide resolved
@ferranbt ferranbt merged commit d513b40 into flashbots:main Feb 23, 2024
4 checks passed
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.

2 participants