Skip to content

Conversation

@emlowe
Copy link
Contributor

@emlowe emlowe commented Jan 10, 2026

For some temporary debugging for simulator errors propogate up the stack trace if some exception happens during block validation (which causes the UNKNOWN error to be propogated)


Note

Adds stack trace propagation for block pre-validation failures to aid debugging.

  • Extends PreValidationResult with extra_string to carry traceback text
  • On exceptions in _pre_validate_block, returns Err.UNKNOWN with extra_string populated
  • In FullNode.add_block, includes extra_string in raised error message when pre-validation fails

Written by Cursor Bugbot for commit 1782220. This will update automatically on new commits. Configure here.

@emlowe emlowe added Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes CI CI changes labels Jan 10, 2026
required_iters: uint64 | None # Iff error is None
conds: SpendBundleConditions | None # Iff error is None and block is a transaction block
timing: uint32 # the time (in milliseconds) it took to pre-validate the block
extra_string: str = ""
Copy link

Choose a reason for hiding this comment

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

Temporary debugging code included in production

Low Severity

The PR description explicitly states this is "For some temporary debugging for simulator errors." This suggests the code adding extra_string field and stack trace propagation was intended for temporary debugging and may not be meant for production. The stack trace is already being logged via log.error(f"Exception: {error_stack}"), so adding it to the ValueError message and storing it in a Streamable field appears to be debugging-specific behavior that could be removed once the simulator issues are resolved.

🔬 Verification Test

Why verification test was not possible: This is a code quality/intentionality issue rather than a functional bug. The code functions correctly - it adds stack trace information to error messages. The concern is whether this temporary debugging code should be committed to production, as explicitly stated in the PR description. This requires human judgment about whether the feature is intended to be permanent or should be removed after debugging is complete.

Additional Locations (2)

Fix in Cursor Fix in Web

@github-actions
Copy link
Contributor

File Coverage Missing Lines
chia/consensus/multiprocess_validation.py 50.0% lines 162
Total Missing Coverage
2 lines Unknown 50%

@coveralls-official
Copy link

Pull Request Test Coverage Report for Build 20869836106

Details

  • 1 of 2 (50.0%) changed or added relevant lines in 1 file are covered.
  • 31 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.008%) to 90.827%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/consensus/multiprocess_validation.py 1 2 50.0%
Files with Coverage Reduction New Missed Lines %
chia/full_node/full_node.py 1 87.21%
chia/_tests/simulation/test_simulation.py 1 96.49%
chia/_tests/core/test_farmer_harvester_rpc.py 2 98.06%
chia/timelord/timelord_launcher.py 2 70.0%
chia/server/node_discovery.py 3 80.86%
chia/wallet/wallet_node.py 4 86.39%
chia/daemon/server.py 7 80.62%
chia/timelord/timelord.py 11 72.0%
Totals Coverage Status
Change from base Build 20864058213: 0.008%
Covered Lines: 102890
Relevant Lines: 113110

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI CI changes coverage-diff Exclude_Notes Use this label if the changes in the PR should be excluded from the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants