Skip to content

miner: disable tracer in vm.Config to prevent conflicts during block synchronization#2206

Open
maoueh wants to merge 2 commits into0xPolygon:developfrom
maoueh:fix/miner-no-live-tracing
Open

miner: disable tracer in vm.Config to prevent conflicts during block synchronization#2206
maoueh wants to merge 2 commits into0xPolygon:developfrom
maoueh:fix/miner-no-live-tracing

Conversation

@maoueh
Copy link
Copy Markdown
Contributor

@maoueh maoueh commented Apr 29, 2026

Description

If running bor with live tracing enabled, once reaching live, miner starts to also call the live tracing hooks causing problems because only the block synchronization should call the live tracer.

Changes

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Changes only for a subset of nodes

Breaking changes

None

Nodes audience

People running Live Tracing + bor

Testing

  • I have tested this code manually on Mainnet

Copilot AI review requested due to automatic review settings April 29, 2026 16:30
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copy link
Copy Markdown
Contributor

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.

Pull request overview

This PR prevents mining-time EVM execution from invoking the chain-level vm.Config.Tracer hooks, avoiding conflicts when “live tracing” is enabled during block synchronization.

Changes:

  • In miner/worker.go, explicitly clears vm.Config.Tracer in the miner’s derived VM config to ensure tracing hooks don’t run during block production.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread miner/worker.go Outdated
Comment thread miner/worker.go
// in which should be active only for live tracing and no for mining. So here, we explicitly
// sets the tracer to nil to avoid having the miner tracing a block while it's produced conflicting
// with the live tracing.
cfg.Tracer = nil
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

This change alters miner EVM behavior when the chain VM config has a tracer set, but there’s no regression test to ensure the worker always strips the tracer from its vm.Config. Please add a unit test in miner/worker_test.go that sets a non-nil tracer on backend.chain.GetVMConfig(), calls w.vmConfig(), and asserts the returned config has Tracer == nil (and ideally that the chain’s VM config tracer remains unchanged).

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 30, 2026 14:45
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

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.

Pull request overview

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread miner/worker.go
Comment on lines 2374 to +2380
cfg := *w.chain.GetVMConfig()
// The miner copies its vm.Config from the chain instance, which may include
// a vm.Config.Tracer intended only for live tracing, not for mining. Clear
// the tracer here to prevent the miner from tracing block production and
// conflicting with live tracing.
cfg.Tracer = nil

Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

Consider adding a unit test to lock in this behavior: when the underlying chain VM config has a non-nil Tracer (e.g., from the VMTrace/live tracing flag), worker.vmConfig() should return a config with Tracer cleared. This prevents regressions where mining accidentally reuses the live tracer again.

Copilot uses AI. Check for mistakes.
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.

2 participants