fix(checkpoint): drop parse-side total OL-log payload cap#154
Open
prajwolrg wants to merge 1 commit into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Commit: 7328020
|
12 tasks
prajwolrg
added a commit
to alpenlabs/alpen
that referenced
this pull request
Jun 17, 2026
Bumps asm v0.1-alpha.13 -> v0.1-alpha.14, strata-common rc25 -> rc26, and zkaleido beta.4 -> beta.5, adapting to the breaking changes they bring: - L1Payload::new now validates SSZ bounds and returns Result: unwrap at test sites, propagate at the checkpoint-submission RPC. - The ASM worker's submit_block now takes a BlockHash and returns the processed commitments; the BlockSubmitter adapter converts the commitment and discards the returned vec. - L1DataProvider gained get_l1_block_height; implement it against the bitcoin client mirroring the existing fetch helpers. - The checkpoint sidecar proptest leaned on asm's checkpoint_sidecar_strategy, which can emit OL logs exceeding the new 16 KiB total-payload cap and panics; use a locally bounded strategy until alpenlabs/asm#154 lands.
CheckpointSidecar::new enforced MAX_TOTAL_LOG_PAYLOAD_BYTES, a hand-rolled 16 KiB cap layered on top of the SSZ schema caps. ASM only parses checkpoint payloads out of Bitcoin transactions, where overall size is already bounded implicitly by the transaction limit and the real budget is checked downstream at construction. Enforcing a 16 KiB total here is both incoherent with the schema (which permits MAX_OL_LOGS_PER_CHECKPOINT logs of MAX_LOG_PAYLOAD_LEN each) and actively wrong: a checkpoint valid on-chain but carrying more than 16 KiB of log data would fail to parse. Remove the constant, its manual validation, and the OLLogsTotalPayloadTooLarge variant; the sidecar now enforces only the schema caps (state-diff size, log count, per-log size), which SSZ decoding already guarantees. Also rework the sidecar/payload proptest strategies, previously keyed off arbitrary magic sizes and the removed cap, to draw against the real SSZ constants, and wire the previously-unexercised checkpoint_sidecar and checkpoint_payload strategies into ssz_proptest so their round-trip and tree-hash invariants are actually checked.
prajwolrg
force-pushed
the
fix/checkpoint-sidecar-log-payload-cap
branch
from
261eb60 to
6b43450
Compare
prajwolrg
changed the title
krsnapaudel
approved these changes
Jun 18, 2026
| /// Maximum total OL log payload size per checkpoint (16 KiB per SPS-ol-chain-structures). | ||
| pub const MAX_TOTAL_LOG_PAYLOAD_BYTES: usize = 16 * 1024; | ||
|
|
||
| // Re-export OLLog for consumers parsing checkpoint sidecar logs |
There was a problem hiding this comment.
This comment should be with the export.
| /// truncating before materializing keeps a draw bounded while still exercising both regimes — large | ||
| /// payloads fill the budget after a few logs, near-empty payloads extend the prefix toward the | ||
| /// [`MAX_OL_LOGS_PER_CHECKPOINT`] count cap. | ||
| fn bounded_ol_logs(sizes: Vec<usize>) -> Vec<OLLog> { |
|
@prajwolrg This is breaking in a way since OL block assembly uses the constant. I created this ticket to update block assembly. |
storopoli
approved these changes
Jun 18, 2026
bewakes
approved these changes
Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CheckpointSidecar::newenforcedMAX_TOTAL_LOG_PAYLOAD_BYTES— a hand-rolled 16 KiB cap on the summed OL-log payload, layered on top of the SSZ schema caps. That cap doesn't belong on the parse side. ASM only parses checkpoint payloads out of Bitcoin transactions, where overall size is already bounded implicitly by the transaction limit, and the real budget is enforced downstream at construction. Enforcing a 16 KiB total here is both incoherent with the schema (which permitsMAX_OL_LOGS_PER_CHECKPOINTlogs ofMAX_LOG_PAYLOAD_LENeach) and actively wrong: a checkpoint that is valid on-chain but carries more than 16 KiB of log data would fail to parse.This removes the constant, its manual
validate_ol_logs_payloadscheck, and theOLLogsTotalPayloadTooLargeerror variant. The sidecar now enforces only the schema caps (state-diff size, log count, per-log size), all of which SSZ decoding already guarantees.It also reworks the sidecar/payload proptest strategies, which were keyed off arbitrary magic sizes (
0..1024,0..512, aMAX_LOGS = 10/MAX_TOTAL_LOG_PAYLOAD_BYTES / MAX_LOGSper-log bound) and the now-removed cap. They now draw against the real SSZ constants (OL_DA_DIFF_MAX_SIZE,MAX_OL_LOGS_PER_CHECKPOINT,MAX_LOG_PAYLOAD_LEN,MAX_PROOF_LEN). The log strategy hands over candidate payload sizes socheckpoint_sidecar_strategycan apply a proptest-tractability budget (reusingOL_DA_DIFF_MAX_SIZEas a representative ceiling) before materializing any bytes — keeping a draw bounded rather than allocating the schema's multi-MiB worst case.Finally, the previously-unexercised
checkpoint_sidecar_strategy/checkpoint_payload_strategyare wired intossz_proptest(capped at 32 cases, since each case spans the full valid size range), so their round-trip and tree-hash invariants are actually checked.Type of Change
Notes to Reviewers
The dropped
MAX_TOTAL_LOG_PAYLOAD_BYTESandOLLogsTotalPayloadTooLargevariant were used only inside this crate — no downstream consumers. If 16 KiB is actually a consensus rule the ASM must enforce at parse time (the comment cited SPS-ol-chain-structures), this should be reconsidered; the working assumption here is that the total-payload budget is a construction-side concern checked downstream.Checklist
Related Issues
N/A