Skip to content

support a new version of FullBlock serialization - #1456

Merged
arvidn merged 2 commits into
mainfrom
full-block-generator
Jun 10, 2026
Merged

support a new version of FullBlock serialization#1456
arvidn merged 2 commits into
mainfrom
full-block-generator

Conversation

@arvidn

@arvidn arvidn commented May 27, 2026

Copy link
Copy Markdown
Contributor

where the block generator is just a buffer.

This must be combined with a check in chia-blockchain that version == 1 iff the hard-fork has activated.


Note

High Risk
Changes consensus-relevant block serialization and parsing; v1 must be gated by hard-fork checks in chia-blockchain (not in this PR) to avoid accepting invalid blocks on the network.

Overview
Adds a versioned FullBlock wire format so post–hard-fork blocks can carry the transaction generator as raw length-prefixed bytes instead of a CLVM Program plus transactions_generator_ref_list.

FullBlock drops the derived #[streamable] impl in favor of a manual Streamable implementation. Version 0 keeps the legacy tail: optional Program generator and ref_list. Version 1 encodes the format in the optional prefix byte (bit 0b10, same pattern as ProofOfSpace): no ref list on the wire; optional generator is Bytes-style (4-byte length + payload) stored in new transactions_generator_buffer, with in-memory version driving encode/decode. Invalid versions return Error::InvalidFullBlock.

The Python surface (chia_rs.pyi) gains transactions_generator_buffer and version. Rust unit tests and a new tests/test_full_block.py cover round-trips, prefix-byte encoding, and v1 omitting the ref list.

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

@coveralls-official

coveralls-official Bot commented May 27, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27024420785

Coverage increased (+0.6%) to 81.583%

Details

  • Coverage increased (+0.6%) from the base build.
  • Patch coverage: 29 uncovered changes across 1 file (267 of 296 lines covered, 90.2%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
crates/chia-protocol/src/fullblock.rs 296 267 90.2%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 18879
Covered Lines: 15402
Line Coverage: 81.58%
Coverage Strength: 11957095.23 hits per line

💛 - Coveralls

@arvidn
arvidn force-pushed the full-block-generator branch 2 times, most recently from c6892ae to 3932f65 Compare May 27, 2026 14:53
@arvidn

arvidn commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

@cursor review

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3932f65. Configure here.

@arvidn
arvidn requested a review from richardkiss May 27, 2026 15:37
@richardkiss

Copy link
Copy Markdown
Contributor

This seems reasonable. It's a bit sad that we have to write the code that was previously auto-generated. A way around it would be to extend the macros to allow enum variants with data; or generalize Option to be trinary : Option3<None, Type1, Type2>. Since we're doing this in ProofOfSpace already. But maybe it's overkill.

@arvidn

arvidn commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I experimented with extending the macros, and also implementing this at the python boundary, but this ended up being far simpler. It's unfortunate that we have to manually implement this trait, but at least it's easy to review and understand what it's doing (compared to the alternatives I tested with ProofOfSpace)

@arvidn

arvidn commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

rebased over type-stub conflict. unchanged otherwise

@arvidn
arvidn force-pushed the full-block-generator branch from 3932f65 to d01897b Compare June 5, 2026 15:01

@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 d01897b. Configure here.

Comment thread crates/chia-protocol/src/fullblock.rs
richardkiss added a commit that referenced this pull request Jun 5, 2026
Add Option3<V1, V2> to chia-traits: a tri-state optional type with wire
encoding 0x00/0x01/0x02. Use it for FullBlock.transactions_generator to
cleanly express three states (no generator / v0 Program / v1 raw bytes)
without hand-written serialization code.

transactions_generator_ref_list changes from Vec<u32> to Option<Vec<u32>>:
None signals v1 format (ref_list omitted), Some signals v0 format.

This replaces the 145-line hand-written Streamable impl in PR #1456 with
a #[streamable] derive and zero hand-written serialization code. The wire
format differs by 1-2 bytes per block (negligible for a hard fork protocol
change).

Co-authored-by: Cursor <cursoragent@cursor.com>
richardkiss added a commit that referenced this pull request Jun 5, 2026
Add Option3<V1, V2> to chia-traits: a tri-state optional type with wire
encoding 0x00/0x01/0x02. Use it for FullBlock.transactions_generator to
cleanly express three states (no generator / v0 Program / v1 raw bytes)
without hand-written serialization code.

transactions_generator_ref_list changes from Vec<u32> to Option<Vec<u32>>:
None signals v1 format (ref_list omitted), Some signals v0 format.

This replaces the 145-line hand-written Streamable impl in PR #1456 with
a #[streamable] derive and zero hand-written serialization code. The wire
format differs by 1-2 bytes per block (negligible for a hard fork protocol
change).

Co-authored-by: Cursor <cursoragent@cursor.com>
richardkiss added a commit that referenced this pull request Jun 5, 2026
Add Option3<V1, V2> to chia-traits: a tri-state optional type with wire
encoding 0x00/0x01/0x02. Use it for FullBlock.transactions_generator to
cleanly express three states (no generator / v0 Program / v1 raw bytes)
without hand-written serialization code.

transactions_generator_ref_list changes from Vec<u32> to Option<Vec<u32>>:
None signals v1 format (ref_list omitted), Some signals v0 format.

This replaces the 145-line hand-written Streamable impl in PR #1456 with
a #[streamable] derive and zero hand-written serialization code. The wire
format differs by 1-2 bytes per block (negligible for a hard fork protocol
change).

Co-authored-by: Cursor <cursoragent@cursor.com>
richardkiss added a commit that referenced this pull request Jun 5, 2026
Add Option3<V1, V2> to chia-traits: a tri-state optional type with wire
encoding 0x00/0x01/0x02. Use it for FullBlock.transactions_generator to
cleanly express three states (no generator / v0 Program / v1 raw bytes)
without hand-written serialization code.

transactions_generator_ref_list changes from Vec<u32> to Option<Vec<u32>>:
None signals v1 format (ref_list omitted), Some signals v0 format.

This replaces the 145-line hand-written Streamable impl in PR #1456 with
a #[streamable] derive and zero hand-written serialization code. The wire
format differs by 1-2 bytes per block (negligible for a hard fork protocol
change).

Co-authored-by: Cursor <cursoragent@cursor.com>
@richardkiss

Copy link
Copy Markdown
Contributor

I experimented with extending the macros, and also implementing this at the python boundary, but this ended up being far simpler. It's unfortunate that we have to manually implement this trait, but at least it's easy to review and understand what it's doing (compared to the alternatives I tested with ProofOfSpace)

I was trying understand why this would be. Take a look at #1463 What am I missing?

@arvidn
arvidn requested a review from AmineKhaldi June 9, 2026 14:31
@arvidn

arvidn commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

I experimented with extending the macros, and also implementing this at the python boundary, but this ended up being far simpler. It's unfortunate that we have to manually implement this trait, but at least it's easy to review and understand what it's doing (compared to the alternatives I tested with ProofOfSpace)

I was trying understand why this would be. Take a look at #1463 What am I missing?

I don't think it's obvious that this is a simpler solution.

  1. Option3<> doesn't generalize that well. It would be cumbersome to use in ProofOfSpace for instance, where there are more fields that differ, so both Some1() and Some2() would need to be tuples.

  2. You lose named fields on the rust side. I think the ideal, pure rust, implementation would use specific enums for each case.

  3. I think you may have problems with the JSON interface. I believe it derives the json fields from the rust field names.

@arvidn
arvidn merged commit 9c3e2d1 into main Jun 10, 2026
101 of 102 checks passed
@arvidn
arvidn deleted the full-block-generator branch June 10, 2026 09:43
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