-
Notifications
You must be signed in to change notification settings - Fork 29
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
CREATE
tests
#1572
base: arith-dev
Are you sure you want to change the base?
CREATE
tests
#1572
Conversation
fixes GAS_NEXT issues for now
59c91cd
to
581a7ab
Compare
@@ -206,6 +206,7 @@ public CreateSection(Hub hub, MessageFrame frame) { | |||
if (failedCreate) { | |||
finalContextFragment = ContextFragment.nonExecutionProvidesEmptyReturnData(hub); | |||
scenarioFragment.setScenario(CREATE_FAILURE_CONDITION_WONT_REVERT); | |||
commonValues.payGasPaidOutOfPocket(hub); |
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.
omission
preOpcodeCreateeSnapshot, | ||
preOpcodeCreateeSnapshot, | ||
preOpcodeCreatorSnapshot, | ||
preOpcodeCreatorSnapshot, |
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.
typo
currentAddress, | ||
salt, | ||
hash, | ||
longToUnsignedBigInteger(frame.getWorldUpdater().get(currentAddress).getNonce())); |
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.
Even though it's unnecessary, the nonce is part of the HUB -> RLPADDR lookup for CREATE2.
@@ -134,6 +142,7 @@ private void traceCreate2(int stamp, RlpAddrOperation chunk, Trace trace) { | |||
.saltLo(chunk.salt().orElseThrow().slice(LLARGE, LLARGE)) | |||
.kecHi(chunk.keccak().orElseThrow().slice(0, LLARGE)) | |||
.kecLo(chunk.keccak().orElseThrow().slice(LLARGE, LLARGE)) | |||
.nonce(nonceBytes) |
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.
We trace the nonce in all cases.
Address address, | ||
Bytes32 salt, | ||
Bytes32 kec, | ||
BigInteger nonce) { |
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.
Adding the nonce to the interface
} | ||
|
||
long gasMinusUpfront = currentGas - upfrontGasCost; | ||
return gasMinusUpfront - gasMinusUpfront / 64; |
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.
This was a genuine mistake. the 63/64-ths business happens after paying the upfront gas cost.
|
||
final long gasMinusUpfrontGasCost = currentGas - upfrontGasCost; | ||
|
||
return gasMinusUpfrontGasCost - gasMinusUpfrontGasCost / 64; |
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.
Same mistake for CREATE2
public ToyAccount raiseNonceBy(long k) { | ||
long updatedNonce = this.getNonce() + k; | ||
return new ToyAccount(this.parent, this.getAddress(), updatedNonce, this.balance, this.balance); | ||
} |
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.
Bandaid
We unconditionally import account.nonce() to the RLPADDR module. We must therefore also include the nonce to disambiguate duplicate CREATE2's
No description provided.