Skip to content

Comments

fix: debug_traceTransaction default param#9788

Open
macfarla wants to merge 8 commits intohyperledger:mainfrom
macfarla:debug-trace-tx-empty-options
Open

fix: debug_traceTransaction default param#9788
macfarla wants to merge 8 commits intohyperledger:mainfrom
macfarla:debug-trace-tx-empty-options

Conversation

@macfarla
Copy link
Contributor

@macfarla macfarla commented Feb 11, 2026

PR description

When reviewing docs PR 1909, claude picked up an inconsistency with default parameters when no params are supplied vs empty {} params - as described in #9787

Docs still needs correcting here to say disableMemory is true by default (all other booleans default to false) https://besu.hyperledger.org/public-networks/reference/api#debug_tracetransaction

Fixed Issue(s)

fixes #9787

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
  • hive tests: Engine or other RPCs modified?

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla marked this pull request as draft February 12, 2026 00:51
…ue, matching the documented API behavior

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla marked this pull request as ready for review February 12, 2026 05:42
usmansaleem
usmansaleem previously approved these changes Feb 12, 2026
Copy link
Member

@usmansaleem usmansaleem left a comment

Choose a reason for hiding this comment

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

LGTM

@macfarla macfarla marked this pull request as draft February 12, 2026 23:28
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla requested a review from usmansaleem February 12, 2026 23:59
@macfarla macfarla dismissed usmansaleem’s stale review February 13, 2026 00:00

I have pushed more changes

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Copy link
Contributor Author

@lu-pinto can you review this one - the default param behavior is currently inconsistent between RPCs - this came up from looking at a docs PR hyperledger/besu-docs#1909 - in one spot we say the trace flags are all false by default but looking at the code that's not the case, and actually you currently get different behavior between passing no options and empty {} options

@macfarla macfarla marked this pull request as ready for review February 13, 2026 01:26
if (disableStackNullable() != null) {
builder.traceStack(!disableStack());
}
var defaultTracerConfig = builder.traceOpcodes(opcodes()).build();
Copy link
Member

Choose a reason for hiding this comment

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

nit: I know this was like this before but this name defaultTracerConfig could be misleading. At this stage these option are not default anymore, they are a mix of user set params and defaults, so probably would just call it tracerConfig

Copy link
Member

@lu-pinto lu-pinto left a comment

Choose a reason for hiding this comment

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

Looks good, I think the default behavior for Geth is also to not show memory if not enabled. Make sense to align cases with empty params with no params at all

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.

debug_traceTransaction default param behavior

3 participants