search

ethereum / execution-apis

Collection of APIs provided by Ethereum execution layer clients
Creative Commons Zero v1.0 Universal
908 stars 352 forks source link

engine: Rename DepositRequest to DepositReceipts #544

Closed mkalinin closed 2 months ago

mkalinin commented 2 months ago

Renames DepositRequest to DepositReceipt for the purpose of devnet-0.

The recent change introduced in https://github.com/ethereum/execution-apis/pull/535 to align with the EIP-7685 requests created a slight discrepancy between the CL spec and implementations which still uses DepositReceipt and the Engine API server implementation on EL side. This renaming is a temporal quick fix to facilitate interop between CL and EL clients during planned devnet-0. The decision is to do a proper renaming after the devnet-0 launch and to incorporate it into further devnets.

lightclient commented 2 months ago

The decision is to do a proper renaming after the devnet-0 launch and to incorporate it into further devnets.

Why can't CL just rename now for devnet-0? Seems a little weird to switch this back to DepositReceipts just to again change to DepositRequests? Or is there a different idea for how it might be represented in the future?

onbjerg commented 2 months ago

I also think having every EL switch to from requests to receipts just to change it back to requests later again instead of having CLs rename the 1 field once seems a bit funky:)

mkalinin commented 2 months ago

A proper renaming on CL side involves changing the name in the testing format and maybe other adjustments — this is gonna take some time. What suggested above is to marshal deposit_receipts as deposit_requests on the CL side, rather than doing it on the EL? It would be great to quickly resolve this question on the call this week but there is no call.

I am fine either way and I do see the argument in favour of CL making the change as doing and undoing on EL seems really weird.

StefanBratanov commented 2 months ago

For Teku we already use depositRequests for the EL Engine API and we map it to depositReceipts on CL, so now essentially we have to revert that change. I think to minimise changes on the EL side now and after devnet-0, it is easier to keep the spec as it is.

mkalinin commented 2 months ago

Closing this PR in favour of: "CL marshals deposit_receipts as deposit_requests and after devnet-0 undergoes a full renaming”, the rough consensus is reached in the Discord

cc @ralexstokes