-
Notifications
You must be signed in to change notification settings - Fork 900
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
Decode deposit log data without web3j #8394
base: main
Are you sure you want to change the base?
Decode deposit log data without web3j #8394
Conversation
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.
just a small optimization
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/DepositLogDecoder.java
Outdated
Show resolved
Hide resolved
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.
PR Overview
This pull request refactors the deposit log decoding functionality to remove the dependency on Web3J and directly decode log data using raw byte slicing. It also updates the tests to verify both correct decoding and proper error handling when given data of an incorrect length, and fully removes the DepositContract class that was previously used for decoding.
Reviewed Changes
File | Description |
---|---|
ethereum/core/src/test/java/org/hyperledger/besu/ethereum/core/encoding/DepositLogDecoderTest.java | Added tests verifying the new byte slicing-based decoder and exception handling. |
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/encoding/DepositLogDecoder.java | Refactored the deposit log decoder to use raw byte slicing and added detailed JavaDocs. |
ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/DepositContract.java | Removed unused code related to web3j-based deposit log decoding. |
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
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.
worth a changelog entry?
and also can we now change the web3j dependency to be test scope only?
PR description
Decode the deposit log data without relying on Web3J
Fixed Issue(s)
fixes #8391
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 spotlessApply
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests