feat: cap addPieces batches at 40, tighten piece metadata limits#814
feat: cap addPieces batches at 40, tighten piece metadata limits#814rvagg wants to merge 2 commits into
Conversation
Enforce a 40-piece ceiling per addPieces/createDataSetAndAddPieces via validateAddPiecesBatch, wired into addPieces, createDataSetAndAddPieces, commit, and presignForCommit so oversized batches fail early with TooManyPiecesError rather than reverting on-chain at the FVM event-size limit. Sync piece metadata caps to the contract: MAX_KEYS_PER_PIECE 5 -> 3, MAX_VALUE_LENGTH 128 -> 96. Ref: filecoin-project/curio#1272 Ref: FilOzone/filecoin-services#496 Ref: https://www.notion.so/filecoindev/addPieces-Batch-Limits-and-Proposed-Changes-36ddc41950c180d39f45f0da8982090c
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | bcb6af1 | Commit Preview URL Branch Preview URL |
Jun 03 2026, 07:00 AM |
There was a problem hiding this comment.
Pull request overview
This PR introduces an explicit batch-size ceiling (40 pieces) for addPieces / createDataSetAndAddPieces flows so oversized batches fail early with a clear SDK/core error, and aligns piece metadata limits with the on-chain contract caps.
Changes:
- Add
validateAddPiecesBatch()and enforce aMAX_ADD_PIECES_BATCH_SIZEof 40 across core SP APIs and SDK commit/presign/pull paths. - Introduce
TooManyPiecesErrorfor oversized batches and add tests covering the new validation behavior. - Tighten metadata limits to match contract constraints (piece keys: 5 → 3, value length: 128 → 96) and update docs/tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/synapse-sdk/src/test/storage.test.ts | Adds SDK-level tests asserting TooManyPiecesError is raised for oversized presign/commit/pull batches. |
| packages/synapse-sdk/src/storage/context.ts | Enforces batch-size validation early in presignForCommit, pull, and commit to fail before network/chain work. |
| packages/synapse-core/test/metadata.test.ts | Updates expectations for new metadata caps (value length and keys per piece). |
| packages/synapse-core/test/add-pieces.test.ts | Adds unit tests for validateAddPiecesBatch (empty, max, above max). |
| packages/synapse-core/src/utils/metadata.ts | Updates METADATA_LIMITS constants to match contract caps. |
| packages/synapse-core/src/utils/constants.ts | Introduces SIZE_CONSTANTS.MAX_ADD_PIECES_BATCH_SIZE = 40. |
| packages/synapse-core/src/sp/create-dataset-add-pieces.ts | Validates piece batch size before signing/requesting create+add flow. |
| packages/synapse-core/src/sp/add-pieces.ts | Adds validateAddPiecesBatch() and applies it to addPieces. |
| packages/synapse-core/src/errors/warm-storage.ts | Introduces TooManyPiecesError surfaced when batch exceeds the cap. |
| docs/src/content/docs/developer-guides/storage/storage-operations.mdx | Updates documentation to reflect piece metadata max keys change (5 → 3). |
| AGENTS.md | Updates stated validation limits to match new contract-aligned caps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Maximum pieces per addPieces (or createDataSetAndAddPieces) call. | ||
| * | ||
| * On-chain limitations fail batch sizes above 41; we constrain to 40 here to | ||
| * catch those failures early and surface informative errors. | ||
| */ | ||
| MAX_ADD_PIECES_BATCH_SIZE: 40, |
There was a problem hiding this comment.
Why is 1 a good margin? Would 2 be better? What if we use no safety margin?
There was a problem hiding this comment.
It's not really intended as a margin, it's intended as a round number cause exposing 41 invites more questions .. but I guess the comment also invites questions. I was trying to be vague so as to not explain the details too much. We know >41 breaks the fvm event limit. We are relatively confident that fully-loaded metadata now shouldn't break ~40 with gas. I have a small amount of confidence that create+add with fully loaded metadata shouldn't break it but I need to wait till we have everything merged and set up before I can measure that—but even then, gas on mainnet is very different to devnet and different still from calibnet so we're doing a bit of stabbing in the dark without actually running it on mainnet.
If we need to come back and lower it, then I'd do it in increments of 5, just to make the number not look to odd.
There was a problem hiding this comment.
Got it! I don't think 40 is better than 41 here, it's as magic a number. If we decide to keep 40, I suggest to add to the document that we're doing this to not raise suspicion
| /** | ||
| * Maximum pieces per addPieces (or createDataSetAndAddPieces) call. | ||
| * | ||
| * On-chain limitations fail batch sizes above 41; we constrain to 40 here to | ||
| * catch those failures early and surface informative errors. | ||
| */ | ||
| MAX_ADD_PIECES_BATCH_SIZE: 40, |
There was a problem hiding this comment.
Why is 1 a good margin? Would 2 be better? What if we use no safety margin?
github-project-automation
Bot
moved this from 📌 Triage
to ✔️ Approved by reviewer
in FOC
Enforce a 40-piece ceiling per addPieces/createDataSetAndAddPieces via validateAddPiecesBatch, wired into addPieces, createDataSetAndAddPieces, commit, and presignForCommit so oversized batches fail early with TooManyPiecesError rather than reverting on-chain at the FVM event-size limit.
Sync piece metadata caps to the contract: MAX_KEYS_PER_PIECE 5 -> 3, MAX_VALUE_LENGTH 128 -> 96.
Ref: filecoin-project/curio#1272
Ref: FilOzone/filecoin-services#496
Ref: https://www.notion.so/filecoindev/addPieces-Batch-Limits-and-Proposed-Changes-36ddc41950c180d39f45f0da8982090c