Skip to content

harden clvm_serialize() by avoiding recursion - #1518

Draft
arvidn wants to merge 1 commit into
mainfrom
clvm-serialization
Draft

arvidn wants to merge 1 commit into
mainfrom
clvm-serialization

Conversation

@arvidn

@arvidn arvidn commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

this is a layer converting python structures into arguments for clvm programs. It's supposed to only be called by trusted code, but this hardens it a bit by using its own stack for the conversion, rather than using the program stack frames (with recursion).


Note

Medium Risk
Touches core PyO3 paths for Program.to and run_rust args; logic is refactored but heavily tested, with moderate risk if stack ordering or serialize/convert semantics diverged subtly.

Overview
Hardens Python→CLVM conversion by replacing recursive clvm_convert and clvm_serialize with explicit heap-backed work stacks (ConvertOp / SerializeOp). Deep or hostile nesting no longer grows the native call stack (which could SIGSEGV past PyO3’s unwind boundary); depth is bounded by Vec growth instead. Behavior is preserved: pairs/lists use deferred Cons / BuildList ops, allocator failures go through a shared py_mem_err helper, and clvm_serialize still parses Program values into trees while other types fall back to convert mode.

Adds broad pytest coverage for Program.to (scalars, tuples, lists, fake SExp, __bytes__, errors) and for run_rust argument serialization (lists, embedded programs vs atoms, nested cases), including a moderately deep nesting smoke test.

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

@arvidn

arvidn commented Sep 15, 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 31615ca. Configure here.

@coveralls-official

Copy link
Copy Markdown

Coverage Report for CI Build 34947671934

Coverage increased (+0.2%) to 82.294%

Details

  • Coverage increased (+0.2%) from the base build.
  • Patch coverage: 3 uncovered changes across 1 file (70 of 73 lines covered, 95.89%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
crates/chia-protocol/src/program.rs 73 70 95.89%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 20208
Covered Lines: 16630
Line Coverage: 82.29%
Coverage Strength: 11484477.49 hits per line

💛 - Coveralls

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.

1 participant