Add InternedBlockBuilder for INTERNED_GENERATOR cost model - #1436
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new InternedBlockBuilder to build block generators under the post-HF2 INTERNED_GENERATOR cost model, and exposes it through the Python wheel API and type stubs.
Changes:
- Adds
InternedBlockBuilder(Rust +#[pymethods]) that computes generator cost via interning andtotal_cost_from_tree(). - Registers
InternedBlockBuilderin the Python module (wheel/src/api.rs). - Updates Python type stubs (
chia_rs.pyi) and the stub generator script.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/chia-consensus/src/build_interned_block.rs | Introduces InternedBlockBuilder and Python bindings for interned-tree cost accounting. |
| crates/chia-consensus/src/lib.rs | Exposes the new build_interned_block module. |
| wheel/src/api.rs | Adds InternedBlockBuilder to the Python module initialization. |
| wheel/python/chia_rs/chia_rs.pyi | Adds InternedBlockBuilder to the published Python type stubs. |
| wheel/generate_type_stubs.py | Updates stub generation template to include InternedBlockBuilder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage Report for CI Build 28476633082Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.2%) to 81.804%Details
Uncovered Changes
Coverage Regressions182 previously-covered lines in 13 files lost coverage.
Coverage Stats
💛 - Coveralls |
cdcdeb0 to
303e80f
Compare
4016bb3 to
66bb8aa
Compare
|
The |
We run benchmarks on self-hosted runners that only run one job at a time to avoid being loaded. I don't believe this explanation. I suppose this PR doesn't affect running generators though, does it? |
A clean, separate builder that avoids the Serializer/sentinel/restore complexity of BlockBuilder. Cost is computed from total_cost_from_tree on the interned quoted generator tree after each add; serialization happens once in finalize() via node_to_bytes_backrefs. Includes Python bindings matching the BlockBuilder interface. Made-with: Cursor
Register InternedBlockBuilder with add_class in wheel/src/api.rs and add matching stubs to chia_rs.pyi and generate_type_stubs.py. The #[pymethods] block was already in build_interned_block.rs but the class was not exported to the Python module. Co-authored-by: Cursor <cursoragent@cursor.com>
…rom_tree removed in #1435) Co-authored-by: Cursor <cursoragent@cursor.com>
- finalize() now uses tracked self.generator_cost instead of recomputing - Add test_generator_cost_accuracy() to verify tracked cost matches computed cost - Add test_basic_functionality() for basic builder operations - Add test_build_interned_block() comprehensive test with cost accuracy checks - All tests verify that the optimization doesn't break correctness Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Move BuildBlockResult, MAX_SKIPPED_ITEMS, MIN_COST_THRESHOLD, and
skip_result() to build_block_common.rs; both builders import from there,
eliminating the duplicate public types (comments #3211738737, #3211751221)
- Fix mid-loop spend_list corruption: accumulate into local_spend_list and
only assign self.spend_list after the loop completes, matching BlockBuilder's
approach (comment #3261994808)
- Fix intern_tree_limited limit: use u32::MAX as usize to match the validator
in run_block_generator.rs, preventing cost estimate divergence (comment
#3211751228)
- Fix Python wrapper: replace expect("spend bundle") with fallible extraction
via collect::<PyResult<_>>() so invalid inputs raise Python exceptions
(comment #3211738755)
- Fix test flags: add ConsensusFlags::INTERNED_GENERATOR to run_block_generator2
call so the validator uses the same cost model as the builder (comment
#3291326984)
Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix PyResult collect: use explicit closure return type so CastError converts to PyErr (fixes wheel build failure) - Use NodePtr::NIL directly instead of allocator.nil() (suggestion #3302802859) - Don't increment num_skipped when block is simply full; only count skips caused by a bundle's own cost (comment #3302825553) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Instead of calling intern_tree_limited on the full spend_list after every add_spend_bundles call (O(n) per call → O(n²) total), track a running upper bound via raw_vbytes() — a non-deduplicating tree walk that adds each spend's cost as it is parsed, O(spend_size) per add. Design: - upper_bound_vbytes starts at WRAPPER_VBYTES (11) for (q . ((spend_list))) - Each spend contributes raw_vbytes(item) + 3 (list cons cell) to the bound - Fast path: if upper_bound * cost_per_byte + block_cost + cost <= limit, commit without any intern call - Slow path (upper bound exceeded): compute exact cost via intern_tree_limited on spend_list directly (wrapper constant added separately, no allocation) - finalize() always computes exact cost for the returned total This also eliminates compute_generator_cost() which mutated the allocator to build a temporary wrapper node. Co-authored-by: Cursor <cursoragent@cursor.com>
BlockBuilder (pre-HF2) and BlockBuilder2026 (post-HF2) are parallel implementations with separate lifetimes — BlockBuilder will be removed after the hard fork. Sharing code via build_block_common would complicate that deletion, so each file defines BuildBlockResult, MAX_SKIPPED_ITEMS, MIN_COST_THRESHOLD, and skip_result() independently. Also renames build_interned_block.rs -> build_block_2026.rs to match. Co-authored-by: Cursor <cursoragent@cursor.com>
The validator interns the full (q . ((spend_list))) tree so shared atoms (q, nil) get deduplicated. Our previous approach interned spend_list and added WRAPPER_VBYTES=11, which over-counted by ~5 vbytes when nil/q were already present in the tree (as they always are in practice). Fix: exact_generator_cost builds the wrapper nodes and interns from root, same as the validator. finalize() interns from the root it already builds for serialization, avoiding a second wrapper allocation. WRAPPER_VBYTES is now used only for the upper-bound fast path where an overestimate is safe and desirable. Co-authored-by: Cursor <cursoragent@cursor.com>
Add test_finalize_cost_matches_consensus with five spends sharing the same puzzle (subtree dedup). Asserts finalize() cost equals run_block_generator2(..., INTERNED_GENERATOR).cost and upper bound >= exact. Fix make_test_coin_spend to use correct puzzle hash for run_spendbundle. Co-authored-by: Cursor <cursoragent@cursor.com>
Keeps build_interned_block.rs focused on implementation so diff against build_compressed_block.rs is easier to review. Co-authored-by: Cursor <cursoragent@cursor.com>
Move only the extra unit tests to build_interned_block/unit_tests.rs; the expensive integration test stays in the source file matching build_compressed_block.rs structure. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep constants-at-new API; revert drive-by renames and comment churn so diff against build_compressed_block.rs highlights the cost-model swap. Move extra tests to build_interned_block/additional_tests.rs. Co-authored-by: Cursor <cursoragent@cursor.com>
Register an interned-block-builder fuzz target that checks finalize() cost against run_block_generator2(INTERNED_GENERATOR) and round-trips spends. Extend test_generator_cost_accuracy with the same consensus check and fail if the fixture bundle is missing. Co-authored-by: Cursor <cursoragent@cursor.com>
This is done now in |
The test referenced a non-existent bundle and silently passed on CI until assert!(exists) was added. Use a hex-named test-bundles fixture whose aggregate signature validates under run_block_generator2. Co-authored-by: Cursor <cursoragent@cursor.com>
f289f6e to
d303c9f
Compare
Loop the same hex-named fixture pool as test_build_block and assert finalize() cost matches run_block_generator2 for each single-bundle block. Co-authored-by: Cursor <cursoragent@cursor.com>
It does not. This PR should not change consensus in any way, even if it were imported by chia-blockchain. It's all new, opt-in functionality. |
spelling Co-authored-by: Arvid Norberg <arvid.norberg@gmail.com>
Co-authored-by: Arvid Norberg <arvid.norberg@gmail.com>
Restore the add_spend_bundles doc comment, drop the unnecessary num_skipped bump on the MIN_COST early Done path, and make finalize/py_finalize preserve builder state on error. Co-authored-by: Cursor <cursoragent@cursor.com>
Reject bundles via declared cost overflow so the test asserts six KeepGoing skips followed by Done. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0b4258f. Configure here.
finalize() took the signature via mem::take but left spend_list, block_cost, and byte_cost intact, so a second finalize() or a later add_spend_bundles() call would emit a generator still containing the prior spends under a signature that no longer covers them (Bugbot flagged this on #1436). py_finalize() already reset the builder on success in Python; move that behavior into the Rust finalize() itself and have py_finalize() just delegate. State is still preserved on error so callers can inspect/retry. Co-authored-by: Cursor <cursoragent@cursor.com>
finalize() took the signature via mem::take but left spend_list, block_cost, and byte_cost intact, so a second finalize() or a later add_spend_bundles() call would emit a generator still containing the prior spends under a signature that no longer covers them (Bugbot flagged this on #1436). py_finalize() already reset the builder on success in Python; move that behavior into the Rust finalize() itself and have py_finalize() just delegate. State is still preserved on error so callers can inspect/retry. Co-authored-by: Cursor <cursoragent@cursor.com>
Block2026Builder: anytime block builder for post-HF2 (serde_2026). Greedily packs spend bundles with upper-bound cost estimates, validates with exact interned_vbytes costing, and refines in a background thread. Exposes best() -> (generator_bytes, included_indices) and is usable as a Python context manager. Coexists with InternedBlockBuilder (#1436): that one is the simple synchronous builder, this one is the aggressive opt-in anytime builder. tree_hash_auto(): deserializes via node_from_bytes_auto (auto-detects classic / backrefs / serde_2026) then computes the tree hash. Needed for generator_root computation on serde_2026 blocks. Program.from_program_bytes(): wraps raw bytes as a Program without CLVM structure validation. Needed for serde_2026 generators which are validated at execution time by run_block_generator / node_from_bytes_auto. Program::parse() and from_json_dict updated to handle serde_2026 magic prefix for Streamable / RPC round-trips. Non-consensus CLVM readers (additions_and_removals, run_chia_program, get_puzzle_and_solution, tree_hash_auto) switched to node_from_bytes_auto to transparently accept all formats; the non-consensus call sites cap the deserializer at the physical blob size, while additions_and_removals derives its cap from the network cost constants via max_canonical_blob_size(). Co-authored-by: Cursor <cursoragent@cursor.com>
Block2026Builder: anytime block builder for post-HF2 (serde_2026). Greedily packs spend bundles with upper-bound cost estimates, validates with exact interned_vbytes costing, and refines in a background thread. Exposes best() -> (generator_bytes, included_indices) and is usable as a Python context manager. Coexists with InternedBlockBuilder (#1436): that one is the simple synchronous builder, this one is the aggressive opt-in anytime builder. tree_hash_auto(): deserializes via node_from_bytes_auto (auto-detects classic / backrefs / serde_2026) then computes the tree hash. Needed for generator_root computation on serde_2026 blocks. Program.from_program_bytes(): wraps raw bytes as a Program without CLVM structure validation. Needed for serde_2026 generators which are validated at execution time by run_block_generator / node_from_bytes_auto. Program::parse() and from_json_dict updated to handle serde_2026 magic prefix for Streamable / RPC round-trips. Non-consensus CLVM readers (additions_and_removals, run_chia_program, get_puzzle_and_solution, tree_hash_auto) switched to node_from_bytes_auto to transparently accept all formats; the non-consensus call sites cap the deserializer at the physical blob size, while additions_and_removals derives its cap from the network cost constants via max_canonical_blob_size(). Co-authored-by: Cursor <cursoragent@cursor.com>
Block2026Builder: anytime block builder for post-HF2 (serde_2026). Greedily packs spend bundles with upper-bound cost estimates, validates with exact interned_vbytes costing, and refines in a background thread. Exposes best() -> (generator_bytes, included_indices) and is usable as a Python context manager. Coexists with InternedBlockBuilder (#1436): that one is the simple synchronous builder, this one is the aggressive opt-in anytime builder. tree_hash_auto(): deserializes via node_from_bytes_auto (auto-detects classic / backrefs / serde_2026) then computes the tree hash. Needed for generator_root computation on serde_2026 blocks. Program.from_program_bytes(): wraps raw bytes as a Program without CLVM structure validation. Needed for serde_2026 generators which are validated at execution time by run_block_generator / node_from_bytes_auto. Program::parse() and from_json_dict updated to handle serde_2026 magic prefix for Streamable / RPC round-trips. Non-consensus CLVM readers (additions_and_removals, run_chia_program, get_puzzle_and_solution, tree_hash_auto) switched to node_from_bytes_auto to transparently accept all formats; the non-consensus call sites cap the deserializer at the physical blob size, while additions_and_removals derives its cap from the network cost constants via max_canonical_blob_size(). Co-authored-by: Cursor <cursoragent@cursor.com>

Do
diff -u crates/chia-consensus/src/build_compressed_block.rs crates/chia-consensus/src/build_interned_block.rsto see how the new filebuild_interned_block.rsis just a minimal diff from existing code.Summary
Add
InternedBlockBuilder, a block generator builder for the post-HF2INTERNED_GENERATORcost model.UnlikeBlockBuilder, which estimates serialization cost incrementally using a sentinel-based approach,InternedBlockBuildercomputes cost exactly by interning the full quoted generator tree after eachadd_spend_bundles()call and applyingtotal_cost_from_tree(). Serialization happens once infinalize()vianode_to_bytes_backrefs.Like
BlockBuilder, cost is tracked incrementally. Each spend's vbyte contribution is estimated by interning it in an isolated scratch allocator (spend_vbytes); by the triangle inequality this sum is a safe upper bound on the true interned cost of the combined tree.cost()returns that upper bound.finalize()builds the full quoted generator, serializes withnode_to_bytes_backrefs, re-interns from the root to get the exact cost, and asserts it's withinmax_block_cost_clvm.Changes
crates/chia-consensus/src/build_interned_block.rs(new):InternedBlockBuilderwith Rust and Python (#[pymethods]) interfacewheel/src/api.rs: registerInternedBlockBuilderwithadd_classwheel/python/chia_rs/chia_rs.pyi+generate_type_stubs.py: type stubsTest plan
cargo test -p chia-consensuspassesfrom chia_rs import InternedBlockBuilder; b = InternedBlockBuilder(); b.cost()returns 20from chia_rs import InternedBlockBuilder; b = InternedBlockBuilder(constants); b.cost()returns expected wrapper costCheck chia_rs.pyipassesMade with Cursor
Note
Medium Risk
Touches consensus-critical block cost limits and generator assembly; incorrect bounds could reject valid txs or overfill blocks, though tests assert parity with the INTERNED_GENERATOR validation path.
Overview
Adds
InternedBlockBuilder, a mempool-style block assembler for the post-HF2INTERNED_GENERATORcost model, alongside the existing compressedBlockBuilder.It greedily packs prioritized spend bundles under
max_block_cost_clvmusing incremental accounting: callers pass CLVM execution + conditions cost only (no generator byte cost). Per-spend interned vbytes are estimated in a scratch allocator (spend_vbytes); the running sum is a safe upper bound via the triangle inequality.cost()exposes that bound;finalize()builds the quoted(q . (spend_list))generator, serializes withnode_to_bytes_backrefs, re-interns from the root for the exact consensus cost (aligned withrun_block_generator2+INTERNED_GENERATOR), and aggregates BLS signatures. Rejection uses the same skip cap and near-full heuristics as the compressed builder.Python / wheel:
InternedBlockBuilderis registered inwheel/src/api.rswith stubs inchia_rs.pyiandgenerate_type_stubs.py(ConsensusConstantsat__new__;finalize()without extra constants). The module is exported fromchia-consensusviabuild_interned_block.Tests cover upper-bound ≥ exact cost, consensus cost parity, limits/skips, and an optional heavy integration test over
test-bundles.Reviewed by Cursor Bugbot for commit 030aacb. Bugbot is set up for automated code reviews on this repo. Configure here.