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

rm unused engine_exchangeTransitionConfigurationV1 calling signature #132

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

tersec
Copy link
Contributor

@tersec tersec commented Feb 15, 2024

engine_exchangeTransitionConfigurationV1 MUST not be used anymore by clients, and nimbus-eth2 does not use it as of status-im/nimbus-eth2#5889, so remove it.

nimbus-eth1 can still choose to support it (though, must not require it for Dencun-scheduled networks), so leaving TransitionConfigurationV1 in place. The removed signature is for client-side calling only, and does not affect nimbus-eth1.

https://github.com/ethereum/execution-apis/blob/a0d03086564ab1838b462befbc083f873dcf0c0f/src/engine/cancun.md#deprecate-engine_exchangetransitionconfigurationv1 says:

This document introduces deprecation of engine_exchangeTransitionConfigurationV1. The deprecation is specified as follows:

  1. Consensus layer clients MUST NOT call this method.

  2. Execution layer clients MUST NOT surface an error message to the user if this method is not called.

@tersec
Copy link
Contributor Author

tersec commented Feb 15, 2024

https://github.com/status-im/nimbus-eth1/blob/966adcb1245d814a26fd5f082c4e5ca01bb1fce9/hive_integration/nodocker/engine/auths_tests.nim#L65-L75 does appear to use the client version, so, won't merge.

It's not clear if it's used for any specific reason or whether it's a stand-in for just a relatively innocuous message to send to an EL to check the JWT auth. If the latter, a bit unfortunate to pin a client RPC call signature which specs require not to be used, as just some arbitrary call, if an alternative exists.

Copy link
Member

@arnetheduck arnetheduck left a comment

Choose a reason for hiding this comment

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

needs merge

@tersec tersec merged commit 402e71c into master Sep 30, 2024
16 of 18 checks passed
@tersec tersec deleted the eRJ branch September 30, 2024 02:47
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