Skip to content

Fix/trusted extraction serialization error - #1444

Open
GRYPHON000 wants to merge 4 commits into
Chia-Network:mainfrom
GRYPHON000:fix/trusted-extraction-serialization-error
Open

Fix/trusted extraction serialization error#1444
GRYPHON000 wants to merge 4 commits into
Chia-Network:mainfrom
GRYPHON000:fix/trusted-extraction-serialization-error

Conversation

@GRYPHON000

@GRYPHON000 GRYPHON000 commented May 16, 2026

Copy link
Copy Markdown

Summary

This PR changes trusted block spend extraction to return an explicit GeneratorRuntimeError when a puzzle reveal or solution cannot be serialized into Program.

Previously, get_coinspends_for_trusted_block() and get_coinspends_with_conditions_for_trusted_block() used Program::from_clvm(...).unwrap_or_default(). If serialization failed, trusted extraction silently substituted Program::default().

Change

Replace the silent default fallback with explicit error propagation:

let puzzle_program = Program::from_clvm(&a, puzzle)
    .map_err(|_| ValidationErr(ErrorCode::GeneratorRuntimeError))?;
let solution_program = Program::from_clvm(&a, solution)
    .map_err(|_| ValidationErr(ErrorCode::GeneratorRuntimeError))?;

This makes the existing trusted-extraction serialization limitation explicit to callers instead of returning default puzzle/solution data. It follows prior maintainer feedback that returning an error or surfacing an extraction failure is preferable to silently substituting default programs.

Testing
I have not run the full local test suite on this branch.

Expected checks:

  • cargo fmt
  • cargo test -p chia-consensus
  • cargo test --workspace

I kept the change minimal and localized to the two trusted block spend extraction paths.


Note

Medium Risk
Changes trusted coinspend extraction to fail instead of silently substituting default Programs, which may affect downstream callers that relied on partial results from malicious/oversized generators.

Overview
Trusted block spend extraction now propagates serialization failures for puzzle reveals and solutions instead of falling back to Program::default().

Both get_coinspends_for_trusted_block() and get_coinspends_with_conditions_for_trusted_block() map Program::from_clvm() errors to ValidationErr(ErrorCode::GeneratorRuntimeError), making oversized/malicious generator payloads surface as explicit extraction failures.

Reviewed by Cursor Bugbot for commit 65aea87. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread crates/chia-consensus/src/run_block_generator.rs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c2016f5. Configure here.

Comment thread crates/chia-consensus/src/run_block_generator.rs Outdated
@arvidn

arvidn commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

the reason for the current behavior is that a block with a single malicious CoinSpend in it shouldn't block non-malicious CoinSpends. By failing the whole call, terminating the loop, you increase the blast radius of an attack.

@arvidn arvidn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see other comment

@GRYPHON000

Copy link
Copy Markdown
Author

My goal here was to avoid silently returning Program::default() when serialization fails, but I agree that returning an error for the whole call means a single problematic CoinSpend can prevent extraction of all the others i want to know what is the preferred behavior here i mean would you rather skip the offending CoinSpend and continue, or expose some kind of per-spend extraction failure instead?Thanks.

@arvidn

arvidn commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

since the latest soft-fork, I don't believe the attack is possible anymore, where a puzzle or solution becomes unreasonably large. Is there an actual case on the blockchain that's triggering the failure and fallback to a default program?

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