EIP-4863: Beacon chain push withdrawals #4863
Conversation
ralexstokes
force-pushed
the
beacon-chain-push-withdrawals
branch
from
2e49a4d
to
ed6a38a
Compare
ralexstokes
changed the title
There was a problem hiding this comment.
Only blocker is the Reference Implementation section.
I would like to have a debate about using the term Gwei in our specifications and documentation at some point, probably on a broader stage though. I think we are doing our users (especially future users) a disservice by perpetuating the term gwei and wei instead of nanoether and attoether because most users don't understand that wei is just a small amount of ether and they think it is some other unit/measure.
| ## Reference Implementation | ||
|
|
||
| A draft PR containing a prototype implementation in Geth can be found here: | ||
|
|
||
| https://github.com/ethereum/go-ethereum/pull/24468 | ||
|
|
There was a problem hiding this comment.
| ## Reference Implementation | |
| A draft PR containing a prototype implementation in Geth can be found here: | |
| https://github.com/ethereum/go-ethereum/pull/24468 |
Either inline a reference implementation, or include a standalone reference implementation in ../assets/eip-4863/*, or leave this section out. Linking to a pull request to an entire execution client does not qualify as a "reference implementation" for an EIP, that is just called a full production client.
There was a problem hiding this comment.
i think a link to a PR here is a great way to showcase the relative complexity of this change (esp. if you are familiar w/ Geth implementation).
i can remove it if you think it blocks this EIP but i'll just link it everywhere else in that case so it seems a bit pedantic imo
There was a problem hiding this comment.
I strongly encourage you to link it elsewhere and I recommend putting it in the discussions-to link at the least. You are correct that it is a bit pedantic, but that pedantry is necessary because we are woefully understaffed in terms of EIP editors and hard and fast rules are easy to implement and allows us to maintain a certain level of quality of EIPs compared to fuzzy rules which require nuance and discussion to make decisions on.
yeah i also think we should go ahead and do the conversion Gwei -> Wei on the beacon so there is one less thing to do at the execution layer |
ralexstokes
force-pushed
the
beacon-chain-push-withdrawals
branch
from
3f8ae4f
to
6112100
Compare
|
|
||
| Implementations should take care to decode the correct amount of wei from the `EncodedWei` value, which represents a 256-bit unsigned integer value in little-endian byte order. | ||
|
|
||
| TODO: add logs? |
There was a problem hiding this comment.
Does this transaction have a receipt? Current APIs assume receipts can be accessed by tx-index, so those would break if this transaction does not output a receipt. Also, if the receipt is defined as the standard type ++ rlp([status, cumulative_transaction_gas_used, logs_bloom, logs]), then the receipt is a constant bytestring, since this always succeeds, has 0 gas, and no logs.
There was a problem hiding this comment.
i think it is likely we add some kind of logging and along with those changes we'd introduce some kind of receipt format as well.
|
|
||
| ## Security Considerations | ||
|
|
||
| Consensus-layer validation of withdrawal transactions is critical to ensure that the proper amount of ETH is withdrawn back into the execution layer. |
There was a problem hiding this comment.
This deserves a comment in the block-validity section imo.
There was a problem hiding this comment.
But the executioin-layer can't actually do the validation. It is entirely reliant upon CL validating that the proper receipts were dequeued from the beacon state.
The execution-layer can enforce some things such as type-3 TXs only being at the front of the block instead of interspersed in normal TXs, but it can't know the proper count or values (unless we added requisite merkle proofs and counters)
so specifically -- what do you want to see in the block-validity section? If a block is being inserted (via the CL events) it is assumed requisite CL validations have occured
|
|
||
| ## Security Considerations | ||
|
|
||
| Consensus-layer validation of withdrawal transactions is critical to ensure that the proper amount of ETH is withdrawn back into the execution layer. |
There was a problem hiding this comment.
Additional security concern: this type of transaction may not be propagated through anything else than the engine API or trusted blocks (i.e. part of a chain recognized by the consensus layer). The tx-pool could be spammed with useless non-processable withdrawal transactions, and execution payload builders shouldn't consider these during building, or the payload will be invalid.
There was a problem hiding this comment.
good call!
It might be worth defining this more broadly as a "system TX" or a "CL TX" and none of those types of TXs can be gossiped
All tests passed; auto-merging...(pass) eip-4863.md
|
* Add an EIP for beacon chain push withdrawals. * Update EIPS/eip-beacon_chain_push_withdrawals.md * add discussions link * use Wei over Gwei * address feedback * remove reference implementation section * rename EIP file * Update EIPS/eip-4863.md Co-authored-by: Micah Zoltu <micah@zoltu.net>
No description provided.