-
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
Devnet-5: bug fixes for 7702 #8148
Devnet-5: bug fixes for 7702 #8148
Conversation
* move common code to AbstractExtCodeOperation class Signed-off-by: Daniel Lehrner <[email protected]>
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.
LGTM
Signed-off-by: Daniel Lehrner <[email protected]>
Signed-off-by: Daniel Lehrner <[email protected]>
…llehrner/besu into feat/issue-8147/devnet5_7702_fixes
Signed-off-by: Daniel Lehrner <[email protected]>
@@ -47,7 +47,7 @@ class CodeDelegationProcessorTest { | |||
|
|||
@Mock private Transaction transaction; | |||
|
|||
@Mock private DelegatedCodeService authorizedCodeService; | |||
@Mock private CodeDelegationService authorizedCodeService; |
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.
@Mock private CodeDelegationService authorizedCodeService; | |
@Mock private CodeDelegationService delegationCodeService; |
@@ -66,7 +66,7 @@ default Optional<Address> delegatedCodeAddress() { | |||
* | |||
* @return true if the account has delegated code otherwise false. | |||
*/ | |||
default boolean hasDelegatedCode() { | |||
default boolean hasCodeDelegation() { |
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.
very much a nit pick: I think I actually prefer hasDelegatedCode in this case, since it is clear the code is actually delegated. hasCodeDelegation
could be read as "has code delegation feature but maybe it's not currently delegated"
@@ -78,9 +78,9 @@ public interface AccountState { | |||
/** | |||
* The optional EVM bytecode if the account has set a 7702 code delegation. | |||
* | |||
* @return the delegated code (which may be empty). | |||
* @return the code of the target account (which may be empty). |
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.
* @return the code of the target account (which may be empty). | |
* @return the code of the target account that this account delegates to (which may be empty). |
...checking my understanding
PR description
Fixed Issue(s)
fixes #8147
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