-
Notifications
You must be signed in to change notification settings - Fork 879
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
Improve error message for INVALID OPERATION opcode #8141
Improve error message for INVALID OPERATION opcode #8141
Conversation
Signed-off-by: Karim Taam <[email protected]>
Signed-off-by: Karim Taam <[email protected]>
public String toString() { | ||
return name(); | ||
} | ||
}); |
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.
Instead of having the factory method here, move this ExceptionalHaltReason
definition to its class, e.g.:
static ExceptionalHaltReason newInvalidOperation(final long opcode) {
return new ExceptionalHaltReason() {
@Override
public String name() {
return INVALID_OPERATION.name();
}
@Override
public String getDescription() {
return "invalid opcode: 0x%02x".formatted(opcode);
}
@Override
public String toString() {
return name();
}
};
}
And then call it like so here:
new OperationResult(0L, ExceptionalHaltReason.newInvalidOperation(getOpcode()));
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.
not sure it's better but I don't have a strong opinion about that so I will do the change
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 keeping all ExceptionHaltReason
definitions in one place is best
Signed-off-by: Karim Taam <[email protected]>
Signed-off-by: Karim Taam <[email protected]>
PR description
This PR updates the behavior of the invalid operation opcode to return an error that includes the problematic opcode in the message, replacing the previous generic message.
Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests