-
Notifications
You must be signed in to change notification settings - Fork 33
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
CREATE
tests
#1572
Conversation
fixes GAS_NEXT issues for now
59c91cd
to
581a7ab
Compare
We unconditionally import account.nonce() to the RLPADDR module. We must therefore also include the nonce to disambiguate duplicate CREATE2's
...test/java/net/consensys/linea/zktracer/instructionprocessing/createTests/failure/Create.java
Outdated
Show resolved
Hide resolved
case THIRTY_TWO -> program.push(0x20); | ||
case MSIZE -> program.op(MSIZE); | ||
case MAX -> program.push("ff".repeat(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.
Here you may consider to do the following.
Modify the enum to:
public enum SizeParameter {
ZERO(0),
TWELVE(12), // - 3 - 1
THIRTEEN(13), // - 3 + 0
FOURTEEN(14), // - 3 + 1
THIRTY_TWO(32),
MSIZE,
MAX;
public int getValue() {
if (this == MSIZE || this == MAX) {
throw new UnsupportedOperationException("...");
}
return this.value;
}
public boolean isAnyOf(SizeParameter... sizeParameters) {
for (SizeParameter sizeParameter : sizeParameters) {
if (this == sizeParameter) {
return true;
}
}
return false;
}
public boolean willRaiseException() {
return this.isAnyOf(MAX);
}
}
Then, the code above would become:
switch (sizeParameter) {
case MSIZE -> program.op(MSIZE);
case MAX -> program.push("ff".repeat(32));
deafult -> program.push(sizeParameter.getValue();
}
However, it is not a big deal to leave it as it is, but this could be a good approach for the future.
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.
Another approach may be using Either
, where you basically either store an int
or another type, that could be an enum
, but it is probably not worth facing that complexity for this simple scenario.
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.
also, what translates to an integer may have a more meaningful name rather than ZERO, TWELVE
etc.
…DE of the createe if the createe gets loaded into the trace
down to 7 failing CREATE tests, none of which (AFAICT) related to the CREATE's themselves.
@@ -625,7 +625,7 @@ public void traceContextEnter(MessageFrame frame) { | |||
frameType, | |||
newChildContextNumber(), | |||
this.deploymentStatusOf(frame.getContractAddress()), | |||
frame.getValue(), | |||
frame.getApparentValue(), |
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.
Important bug fix, required e.g. for DELEGATECALL to provide the CallFrame with the correct value
i.e. callValue
Otherwise the project doesn't work properly with updated constraints
triggers scenario/CALL_FAILURE_WILL_REVERT using a self call and only 2 or 3 iterations (and blows up for STATICCALL) with an exception.
No description provided.