Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

daniellehrner
Copy link
Contributor

@daniellehrner daniellehrner commented Mar 6, 2025

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?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

Copy link
Contributor

@pinges pinges left a 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

@daniellehrner daniellehrner requested a review from Copilot March 7, 2025 08:59

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.

@daniellehrner daniellehrner requested a review from pinges March 7, 2025 10:16
Copy link
Contributor

@macfarla macfarla left a 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Web3j dependency from parsing deposit logs
3 participants