-
Notifications
You must be signed in to change notification settings - Fork 19
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
Bring back TXCREATE #177
base: main
Are you sure you want to change the base?
Bring back TXCREATE #177
Conversation
1) It must be fully transmitted in the transaction. | ||
2) It is accessible to the EVM, but it can't be fully loaded into EVM memory. | ||
|
||
For these reasons, define cost of each of the `initcodes` items same as calldata (16 gas for non-zero bytes, 4 for zero bytes -- see EIP-2028). The intrinsic gas of an `InitcodeTransaction` is extended by the sum of all those items' costs. |
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 wonder how this interacts with EIP-7623, probably in its terms tokens_in_calldata
should be extended with counting the tokens of initcodes
(which in the end is the same as extending intrinsic gas with initcodes
cost and applying calldata cost floor)
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.
yea, good catch. I think the intent is that initcodes is charged for just like regular calldata. I propose we delay speccing this, just in case 7623 undergoes some last minute change. I'll leave an issue to handle this
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.
It's contract data. Perhaps we just outright charge the floor rate for the txcreate initcode, and say that encompasses the INITCODE_WORD_COST as well as all EOF code validation costs.
/// creates contract and returns the address or failure otherwise | ||
|
||
// init_data.length can be 0, but the first 2 words are mandatory | ||
let size := calldatasize() | ||
if lt(size, 64) { revert(0, 0) } | ||
|
||
let tx_initcode_index := calldataload(0) | ||
let tx_initcode_hash := calldataload(0) | ||
let salt := calldataload(32) |
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 we should keccak the tx_initcode_hash with the salt. Having salt only deployment as the principal toehold feels like opening up to a race, with no connection to the deployed code. Subsequent ERC contracts can be deployed that do keep this feature, but if the salt only contract goes first the ERC contract set could get front run by literally anyone.
Notionally keccak(0xef0001 || tx_initcode_hash || salt)
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.
yup, same realization in the "big thread". initcode_hash missing would actually not only open a race but break the feature of cross-chain deployments to same address, with no option of recovery! Will fix in a second.
is the addition of 0xef0001
magic just for good measure or do you have a scenario where a collision can be crafted somehow?
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'm not a huge fan of the particular choice of magic value coinciding with EOF magic value. For now I'll leave it out for brevity, but I'll add a ticket to track figuring this out properly when time comes
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.
It can be another magic value, but I would like the address mining to be in it's own "namespace" and adding a fixed unique value does that. Any mini-salt is fine.
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.
ah, good point. How do we pick such "namespace" though... 0xff
again (and rely on different sizes of pre-image). Or do we want to be creative? I'll put in 0xff to publish and let's work from there
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.
ah, good point. How do we pick such "namespace" though... 0xff
again (and rely on different sizes of pre-image). Or do we want to be creative? I'll put in 0xff to publish and let's work from there
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.
My idea with 0xef0001
is that is is the premable to EOF.
For the ERCs I plan on 0x87650A
, 0x87650B
, and 0x87650C
assuming 8765 is the ERC number.
So it could be the EIP number in hex as an option too.
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.
Having salt only deployment as the principal toehold feels like opening up to a race, with no connection to the deployed code.
The same can be said about init_data, right? Seems like the salt should be the hash of all of calldata for this to be secure.
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.
If the contract is being called via EXTDELEGATECALL, the caller address is also hashed into deploy address automatically, doesn't this alleviate front-running risk?
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.
Yes, is that the expected usage? EOAs are not able to use EXTDELEGATECALL (they can with EIP-7702 but we can't assume that of every EOA). I wouldn't make this contract insecure if used via normal EXTCALL.
let tx_initcode_hash := calldataload(0) | ||
let salt := calldataload(32) | ||
|
||
mstore8(0, 0xff) // a magic value to ensure a specific preimage space |
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'm not convinced about the necessity of extra magic value here, there's already another 0xff
prepended in final hashing of TXCREATE itself, isnt't that enough?
The cost of validation is covered by transaction's initcode cost (part of intrinsic cost.)
As discussed in EOF Implementers Call 65 it seems like there is much more argumentation backing the previous design, which utilized TXCREATE and a new InitcodeTransaction type. This has become evident once more feedback from app-layer developers surfaced.
The new EIP (in exchange for EIP-7698) is yet to be written, with the expanded rationale, for now we only dip the toe in the water by reverting (with suitable refresher changes) the change which removed TXCREATE and introduced EOF creation txs.
This PR is divided into commits, starting from "raw" revert, then followed by extra updates on top.
One of these updates which isn't only a "refresher change" is the removal of
initcode_hash
from the hashing scheme, following discussions in this issue, as summarized in the so-called "Scenario 1b" from this notes document outlining our options