Skip to content

Tracer option for debug trace RPC calls #8509

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

Draft
wants to merge 67 commits into
base: main
Choose a base branch
from

Conversation

usmansaleem
Copy link
Member

@usmansaleem usmansaleem commented Apr 2, 2025

PR description

Necessary code modifications to support #8326

Fixed Issue(s)

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

Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>

# Conflicts:
#	ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/processor/TransactionTracer.java
#	ethereum/core/src/main/java/org/hyperledger/besu/ethereum/debug/TraceOptions.java
#	ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
@usmansaleem usmansaleem requested a review from Copilot April 8, 2025 03:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/processor/TransactionTracer.java:167

  • [nitpick] Although the short-circuit evaluation ensures safety, consider adding a clarifying comment that explains the assumption that options.config() is always a StructLogTracerConfig when options.tracerType() is DEFAULT_TRACER for improved readability and future maintainability.
private boolean isDefaultTracerAndMemoryEnabled(final TraceOptions<? extends TracerConfig> options) {

Comment on lines 78 to 79
// TODO: Validate how does invalid tracer name is handled
var tracerType = TracerType.fromString(tracer());
Copy link
Preview

Copilot AI Apr 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding explicit null and invalid-value validation for the 'tracer' property prior to calling TracerType.fromString to prevent potential NullPointerException or misconfiguration. For example, if 'tracer' is null, return a sensible default or throw a descriptive error.

Suggested change
// TODO: Validate how does invalid tracer name is handled
var tracerType = TracerType.fromString(tracer());
// Validate tracer property
String tracerValue = tracer();
if (tracerValue == null) {
throw new IllegalArgumentException("Tracer property cannot be null");
}
var tracerType = TracerType.fromString(tracerValue);

Copilot uses AI. Check for mistakes.

usmansaleem added 30 commits May 8, 2025 16:38
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
Signed-off-by: Usman Saleem <[email protected]>
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.

1 participant