serde_2026 emission + trusted readers (minimal) - #1511
Conversation
Coverage Report for CI Build 35951548026Coverage increased (+0.3%) to 82.429%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
🟡 Changes recommended
The new Python API code introduces a memory-safety issue by creating slices from PyBuffer values after the buffers may have been dropped (potential UB) and needs correction before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates generator emission and related tooling to support serde_2026 generators post-HF2, including new trusted readers and hashing paths needed by non-consensus consumers (e.g., Python bindings) while extending consensus flags to signal the generator format.
Changes:
- Add serde_2026-aware Python APIs (
tree_hash_2026,get_puzzle_and_solution_for_coin_2026) and exposeINTERNED_GENERATORto Python/stubs. - Update consensus logic to enable
INTERNED_GENERATORfrom hard fork 2 and dispatch generator parsing inadditions_and_removals. - Expand tests/fuzzing to cover serde_2026 parsing, size bounds, and DAG-aware hashing behavior.
File summaries
| File | Description |
|---|---|
| wheel/src/api.rs | Adds Python bindings for serde_2026 hashing and puzzle/solution extraction; exports INTERNED_GENERATOR. |
| wheel/python/chia_rs/chia_rs.pyi | Updates Python type stubs for new APIs/flag. |
| wheel/generate_type_stubs.py | Updates stub generator output for new APIs/flag. |
| crates/clvm-utils/src/tree_hash.rs | Adds tests validating DAG-aware cached hashing and format-dispatched hashing equivalence. |
| crates/chia-consensus/src/spendbundle_validation.rs | Enables INTERNED_GENERATOR at hard fork 2 and updates tests accordingly. |
| crates/chia-consensus/src/solution_generator.rs | Updates tests to round-trip serde_2026 using explicit trusted parser. |
| crates/chia-consensus/src/serde_2026.rs | Removes auto-dispatch parser, adds explicit trusted serde_2026 reader and updates docs/tests. |
| crates/chia-consensus/src/build_interned_block/additional_tests.rs | Adds serde_2026 emission/round-trip and tree-hash agreement tests. |
| crates/chia-consensus/src/build_interned_block.rs | Documents that InternedBlockBuilder always emits serde_2026. |
| crates/chia-consensus/src/additions_and_removals.rs | Dispatches generator parsing based on INTERNED_GENERATOR. |
| crates/chia-consensus/fuzz/fuzz_targets/serde-2026-size-bound.rs | Updates fuzz target to use the consensus serde_2026 parser and clarifies intent. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Rename test_serde_2026_round_trip to test_serde_2026_builder_matches_classic; reword doc comment to describe an agreement test against an independently built classic generator, not a round trip (serde_2026 has no canonical encoding). - Extend the interned-block-builder fuzzer to also build a classic (solution_generator_backrefs) reference generator for the same spends, run it under classic (non-INTERNED_GENERATOR) rules, and assert the normalized spends agree with the interned run. Either run erroring while the other succeeds is now a fuzzer finding instead of Corpus::Reject. - Use tree_hash_cached + TreeCache in test_serde_2026_tree_hash_2026_agrees so the test exercises the same DAG-aware hashing path as the wheel's tree_hash_2026(). - serde-2026-size-bound fuzz target: also compute tree_hash_cached on the parsed node after a successful node_from_bytes_2026 parse, so libfuzzer's per-input timeout catches slow-hashing inputs that still parse within the size bound. - test_blob_size_cap: compare parsed trees directly with node_eq_two instead of via classic node_to_bytes, since classic serialization of a DAG with shared subtrees expands it. - wheel/src/api.rs::run_generator_and_find_coin: drop the stale 'until wallets support backrefs' comment and move node_to_bytes serialization of puzzle/solution inside the py.detach closure. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5a2de40. Configure here.
InternedBlockBuilder::finalize() now always emits the generator in serde_2026 format (interned serialization, magic-prefixed); there is no classic-emission mode, since cost accounting by interned vbytes and serde_2026 acceptance both activate at the same height. node_from_bytes_auto drops its max_blob_size parameter and cap: it's a policy-free trusted reader (used by additions_and_removals and other RPC-style reads of already-stored, already-validated blobs), not a consensus entry point. The cost-derived size cap is enforced separately by node_from_bytes_2026 on the consensus path. Because a policy-free reader must accept whatever a backrefs/serde_2026 blob decodes to, anything hashing its result needs to be DAG-aware (tree_hash_cached) rather than the naive tree_hash walker, which can diverge exponentially on crafted inputs; test_tree_hash_cached_deep_dag pins that property. additions_and_removals switches from node_from_bytes_backrefs to node_from_bytes_auto so it can read stored post-HF2 (serde_2026) blobs. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds tree_hash_auto(blob) -> bytes32: sniffs the encoding (classic, backrefs, or serde_2026) via node_from_bytes_auto, then hashes with tree_hash_cached. This is the DAG-safe hash trusted readers need to compute the post-HF2 generator_root from unvalidated wire bytes. run_chia_program, get_puzzle_and_solution_for_coin(2) switch from node_from_bytes_backrefs to node_from_bytes_auto so they can also accept stored serde_2026-encoded generators/puzzles/solutions. Regenerates chia_rs.pyi via generate_type_stubs.py for the new stub. Co-authored-by: Cursor <cursoragent@cursor.com>
Every chia-blockchain call site dispatches on block.version or fork height, never on the byte prefix, so the sniffing convenience isn't needed anywhere. Replace it with explicit-format functions: a function either parses classic-only, serde_2026-only, or dispatches on a ConsensusFlags value the caller passes. - Remove node_from_bytes_auto (chia-consensus) and tree_hash_auto (wheel), replaced by a non-sniffing node_from_bytes_2026_trusted helper and a new tree_hash_2026 pyfunction. - additions_and_removals now dispatches the generator parse on ConsensusFlags::INTERNED_GENERATOR instead of sniffing. - Revert run_chia_program, get_puzzle_and_solution_for_coin[2] to classic-only backrefs parsing (this branch had switched them to sniffing; upstream main never did), and restore the SERDE_2026_MAGIC_PREFIX registration to its original position so that hunk drops out of the diff. - Add byte-native get_puzzle_and_solution_for_coin_2026, sharing the run+find-coin tail with the "2" variant. - Expose INTERNED_GENERATOR as a python constant, and OR it into get_flags_for_height_and_constants at hard_fork2_height. - Retarget tests that exercised the removed sniffers at the explicit functions instead of dropping coverage, and fold a few Richard's review nits (duplicate bundle-vector literal, hand-rolled classic generator builder in test) into shared helpers. Co-authored-by: Cursor <cursoragent@cursor.com>
Matches get_puzzle_and_solution_for_coin2's style: a single &Coin instead of three descriptive primitives (parent/amount/puzzle_hash). Co-authored-by: Cursor <cursoragent@cursor.com>
- Rename test_serde_2026_round_trip to test_serde_2026_builder_matches_classic; reword doc comment to describe an agreement test against an independently built classic generator, not a round trip (serde_2026 has no canonical encoding). - Extend the interned-block-builder fuzzer to also build a classic (solution_generator_backrefs) reference generator for the same spends, run it under classic (non-INTERNED_GENERATOR) rules, and assert the normalized spends agree with the interned run. Either run erroring while the other succeeds is now a fuzzer finding instead of Corpus::Reject. - Use tree_hash_cached + TreeCache in test_serde_2026_tree_hash_2026_agrees so the test exercises the same DAG-aware hashing path as the wheel's tree_hash_2026(). - serde-2026-size-bound fuzz target: also compute tree_hash_cached on the parsed node after a successful node_from_bytes_2026 parse, so libfuzzer's per-input timeout catches slow-hashing inputs that still parse within the size bound. - test_blob_size_cap: compare parsed trees directly with node_eq_two instead of via classic node_to_bytes, since classic serialization of a DAG with shared subtrees expands it. - wheel/src/api.rs::run_generator_and_find_coin: drop the stale 'until wallets support backrefs' comment and move node_to_bytes serialization of puzzle/solution inside the py.detach closure. Co-authored-by: Cursor <cursoragent@cursor.com>
The classic reference now runs uncapped (it is a semantic reference, not a budget check; the two encodings charge different base costs), and the fuzzer asserts both encodings agree on Ok/Err, not just that classic succeeds when interned does. Co-authored-by: Cursor <cursoragent@cursor.com>
…sibling `get_puzzle_and_solution_for_coin2`, `get_spends_for_trusted_block`, and `get_spends_for_trusted_block_with_conditions` took the generator as a `Program`, but every one of them only ever calls `.as_ref()` on it and already takes `flags` — the `Program` type was a false promise of "classic CLVM" (`Program.from_bytes` validates classic serialization on construction, so a serde_2026 generator couldn't even reach these functions). Take the generator as bytes instead and let `flags` pick the decoder, at one place: - Wheel signatures take a plain readable buffer (`PyBuffer<u8>`, like `tree_hash`) instead of `Program`. Passing a `Program` is now a TypeError; callers pass `bytes(program)`. - `get_puzzle_and_solution_for_coin2` now dispatches on `INTERNED_GENERATOR` itself, so `get_puzzle_and_solution_for_coin_2026` is redundant. Deleted it. - `get_coinspends_for_trusted_block` / `_with_conditions` take `generator: &[u8]` instead of `&Program` (zero ripple: every caller passed `&Program`, which already deref-coerces to `&[u8]`). - Regenerated `chia_rs.pyi`, added a serde_2026 case to `test_get_puzzle_and_solution.py` and `test_get_spends_for_block.py`. Co-authored-by: Cursor <cursoragent@cursor.com>
…thout SIMPLE_GENERATOR Bugbot flagged that check_generator_node() only enforced the quote shape when SIMPLE_GENERATOR was set, so a caller setting INTERNED_GENERATOR alone (bypassing get_flags_for_height_and_constants()) got zero quote enforcement on serde_2026 generators. On deployed nodes SIMPLE_GENERATOR (soft_fork9) is always active by the time INTERNED_GENERATOR (hard_fork2) is, but nothing enforced that coupling at the API level. This is meant as a minimal stopgap: a future PR is expected to remove the "run the generator" step entirely (storing a list of spends directly), at which point this check becomes moot. Co-authored-by: Cursor <cursoragent@cursor.com>
5a2de40 to
7f8e8b6
Compare
arvidn
left a comment
There was a problem hiding this comment.
I think we should also make the change to not require the block generator to be quoted, and just directly interpret it as a list of spends, i.e. skipping the run_program() call entirely. That could either be done in this PR or in another one. But I think we should include the change before cutting a release to move this into chia-blockchain (since that will trigger some extra work with re-generating the test chains).

This makes block building emit generators in serde_2026 format, and adds the readers that trusted (non-consensus) code needs to open them.
InternedBlockBuilder::finalize()always emits serde_2026, so classic backrefs can be deprecated at HF2solution_generator_2026(), the serde_2026 counterpart ofsolution_generator_backrefs()tree_hash_2026()— hash a serde_2026 generator, caching common sub-trees. This is what thegenerator_rootcheck uses for v1 blocks.additions_and_removals()dispatches onINTERNED_GENERATORin the flags it already takes.get_puzzle_and_solution_for_coin2,get_spends_for_trusted_block, andget_spends_for_trusted_block_with_conditionsnow take the generator as a plain buffer, not aProgram. If you're passing aProgram, passbytes(program)instead.INTERNED_GENERATORis exposed to python, andget_flags_for_height_and_constantsincludes it athard_fork2_height.Note
High Risk
Changes consensus generator encoding, deserialization gates, and fork-height flags at HF2; mistakes could cause network splits or incorrect block validation.
Overview
Hard fork 2 turns on
INTERNED_GENERATOR: block builders alwaysfinalize()to magic-prefixed serde_2026 generators, and consensus/trusted code picks the format from flags (or height) instead of sniffing bytes.serde_2026 parsing is split into
node_from_bytes_2026(consensus size cap) andnode_from_bytes_2026_trusted(post-validation reads).node_from_bytes_autois removed.check_generator_nodeenforces the simple (q …) shape whenINTERNED_GENERATORis set, including withoutSIMPLE_GENERATOR.Python bindings take generator bytes (
ReadableBuffer), exposeINTERNED_GENERATORandtree_hash_2026(DAG-aware hash for v1generator_root). Trusted helpers (get_spends_for_trusted_block,get_puzzle_and_solution_for_coin2,additions_and_removals) decode serde_2026 when the flag is set.Tests and fuzz targets compare interned vs classic generators on semantics (normalized spends/conditions), not wire cost.
Reviewed by Cursor Bugbot for commit 7f8e8b6. Bugbot is set up for automated code reviews on this repo. Configure here.