[mbjorkqvist]

Why

/construction/payloads builds two signing payloads for every combination of operation-derived request and ingress expiry. With the 24h ingress window (up to 720 expiries) and a request body full of operations, a single call could produce tens of millions of payloads, tying up a worker for minutes and using a large amount of memory.

What

• Reject /construction/payloads requests whose number of requests times number of ingress expiries exceeds a fixed limit (equivalent to 100 requests over the full 24h window), with an invalid-request error returned before any payloads are built. Realistic transfer and neuron workflows stay well within the limit.
• Add tests for the limit boundary, for a request with the maximum number of STAKE operations, and for a realistic multi-operation neuron workflow over a 24h window.
• Add a CHANGELOG entry.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No active review issues are reported for the change limiting signing-payload generation.

No open issues 🎉

What changed in this PR

This PR limits signing-payload generation in ICP Rosetta’s /construction/payloads endpoint to prevent oversized requests from consuming excessive processing time and memory.

Changes:

• Reject requests exceeding 72,000 request–ingress-expiry pairs before building signing payloads, using overflow-safe validation.
• Add boundary, overflow, oversized STAKE request, and realistic neuron-workflow tests.
• Document the limit in the changelog.

File	Description
rs/​rosetta-api/​icp/​src/​request_handler/​tests.rs	Adds oversized-request rejection and realistic-workflow acceptance tests.
rs/​rosetta-api/​icp/​src/​request_handler/​construction_payloads.rs	Enforces the pair limit and tests boundary and overflow handling.
rs/​rosetta-api/​icp/​CHANGELOG.md	Records the new construction-payload limit.

_{Review effort: Balanced}

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 7603fbd.

Security Overview

• 🔎 Scanned files: 4 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/bd30fac0-c6f5-4bec-83e2-480d2b345946?scanId=8d5de0a1-ac1c-45f0-a4bd-95da2ed86b75&codeScanTypes=PrScan&tab=issues&detectionTypes=traditional,businessLogic,customRule,IAC,SECRETS,CICD,EOL,uncategorized

Detected Code Changes

Change Type

Relevant files

Enhancement

► rs/rosetta-api/icp/src/request_handler/construction_combine.rs     Validate signature/public key sizes and ingress expiry ordering in /construction/combine ► rs/rosetta-api/icp/src/request_handler/construction_payloads.rs     Add max constants for ingress expiry pairs and update size; add validation functions for request/ingress pairs and update size ► rs/rosetta-api/icp/src/request_handler/tests.rs     Add tests for ingestion window and signing payload limits and various request scenarios ► rs/rosetta-api/icp/CHANGELOG.md     Document fixed limit improvements to ingress expiries and update sizes

[eichhorl]

🤖 /construction/combine needs this bound too. It clones each update of the client-supplied unsigned transaction once per ingress expiry into the signed transaction.

[mbjorkqvist]

🤖 Agreed. /construction/combine now checks the number of (update, ingress expiry) pairs before building any envelopes (d4ea5a5). Copilot then pointed out that repeating an ingress expiry lets one signature pair be reused for every pair, so in 7603fbd combine also bounds the size of each client-supplied update and of each signature and public key, and requires strictly increasing ingress expiries.

[eichhorl]

🤖 This bounds the number of (request, expiry) pairs but not the size of each request. FOLLOW.followees is an unbounded Vec<u64>, and add_payloads clones the update and hashes its whole argument twice per expiry. I measured it on this branch: one FOLLOW with 2.09M followees (4.18 MB of JSON, under the 4 MiB body limit) counts as 720 pairs and passes this check, but construction_payloads takes 15.3 s, synchronously on an actix worker. Governance rejects more than MAX_FOLLOWEES_PER_TOPIC (15) followees anyway, so rejecting longer lists when parsing FollowMetadata would close this cheaply. A cap on the encoded argument before the expiry loop, like the one #12095 adds for ICRC, would also cover fields added later.

[mbjorkqvist]

🤖 Good catch, thanks for measuring. I capped the size of each canister update at 4 KiB in add_payloads, before the expiry loop, and in /construction/combine (d4ea5a5 for the argument, 7603fbd for the whole CBOR-encoded update, so nonce and the other fields are covered too). That covers every request type and any fields added later, and leaves the number of followees to governance. The largest updates Rosetta builds are manage_neuron calls of about 2.3 KB, mostly the Candid type table; a FOLLOW with 15 followees is 2,331 bytes. A new test checks that the largest requests are still accepted over the maximum ingress window, and another checks that an oversized FOLLOW is rejected.

[eichhorl]

🤖 nit: 30,000 STAKE operations (~170 B of JSON each) exceed the 4 MiB body limit, so this isn't "as many as fit". The test still covers rejection; maybe just reword.

[mbjorkqvist]

🤖 Right, reworded in d4ea5a5.

[Copilot]

🟡 Changes recommended

Critical memory-exhaustion vulnerabilities remain because unbounded nonce and signature sizes are repeatedly allocated across expiries despite the existing guards.

2 open issues

• Validate signature length before allocating per-expiry envelopes · New
• Bound client-controlled update fields before expanding expiries · New

_{Review effort: Balanced}

[mbjorkqvist]

🤖 Valid, thanks. Fixed in 7603fbd: the 4 KiB limit now applies to the whole CBOR-encoded update rather than only its argument, in both /construction/payloads and /construction/combine. should_reject_combine_exceeding_limits covers an oversized nonce and repeated expiries.

[mbjorkqvist]

🤖 The remaining worst case of 72,000 pairs × 4 KiB doesn't apply to /construction/combine. Since 7603fbd the ingress expiries must be strictly increasing, so every (update, expiry) pair needs its own two signature entries, which are looked up by a hash over the whole update. With at least ~400 bytes of JSON per signature entry, the 4 MiB body allows at most ~10k signatures, i.e. ~5k pairs, and combine stops at the first missing signature. That keeps the retained envelopes around 25 MB. /construction/payloads doesn't retain a copy of the update per pair, only the two signing payloads, so its cost is the 72,000-pair budget with at most 4 KiB hashed per pair.

[Copilot]

🔵 Needs a closer look

The cap still permits 72,000 pairs with 4-KiB updates, potentially retaining hundreds of MiB before serialization, so confirming safety requires production memory and worker-latency profiling unavailable in this review.

1 open issue

• Bound client-controlled update fields before expanding expiries

1 resolved since last review

• Validate signature length before allocating per-expiry envelopes

_{Review effort: Balanced}