Skip to content

Align MAX_CELLS and MAX_FRAMES with c library - #406

Draft
stringhandler wants to merge 4 commits into
BlockstreamResearch:masterfrom
stringhandler:st-align-c-limits
Draft

stringhandler wants to merge 4 commits into
BlockstreamResearch:masterfrom
stringhandler:st-align-c-limits

Conversation

@stringhandler

Copy link
Copy Markdown
Contributor

No description provided.

Type bit widths saturate at usize::MAX, but NodeBounds::comp and
NodeBounds::disconnect added them to their children's cell bounds with
plain addition. A comp whose middle type is that wide, and whose children
need any extra cells, overflows: debug builds panic while computing the
bounds, and release builds wrap to a small bound which passes the Bit
Machine's limit check. Executing the program then panics with an index
out of bounds.

Saturate these sums, as libsimplicity does, so such a program fails
BitMachine::for_program with LimitError::MaxCellsExceeded.

Assisted-by: Claude
Mirror libsimplicity's consensus limits on cells and program length next
to BUDGET_MAX, so the Rust Bit Machine can enforce them. Export all three
from wrapper.c and test that the Rust values match the vendored header,
so a libsimplicity update which changes a limit fails the tests.

Assisted-by: Claude
The Bit Machine accepted programs needing up to 2^31 - 1 cells, about 400
times libsimplicity's consensus limit of CELLS_MAX (5 Mib), so it would
allocate up to 256 MiB for a program that consensus rejects. Limit cells
to CELLS_MAX instead.

The two bounds are not computed the same way. libsimplicity bounds the
cells needed with tail call optimization, which this Bit Machine lacks,
so our bound is never smaller and is sometimes much larger: a chain of
comps keeps every middle frame alive here, but only two at a time there.
So we still reject some programs which consensus accepts, but no longer
accept any which it rejects for using too many cells. To check that our
bound is never smaller, return libsimplicity's cell and frame bounds from
simplicity-sys's run_program, and compare ours with them for the test
programs and in the c_rust_merkle fuzz target.

libsimplicity rejects a program whose encoded length exceeds DAG_LEN_MAX
nodes before decoding any of them, but we decoded programs of any length.
Fail such programs with the new decode::Error::TooManyNodes.

libsimplicity has no limit on frames. Both Bit Machines need at most two
frames per node, and programs now have at most DAG_LEN_MAX nodes, so set
the frame limit to twice that. It was 2^20, which rejected long programs
that consensus accepts.

Also report MAX_FRAMES rather than MAX_CELLS as the maximum in
LimitError::MaxFramesExceeded.

Assisted-by: Claude
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