Repository navigation
feat: gzip-compress DAPI responses - #5577
PastaPastaPasta wants to merge 2 commits into
Conversation
DAPI sent every response uncompressed: rs-dapi claimed Envoy handled compression, but the gateway had no compressor filter. - rs-dapi gzip-compresses Platform and Core responses for clients that list gzip in grpc-accept-encoding; other clients are unaffected. - Native rs-dapi-client clients accept gzip responses (pure-Rust flate2, no new C dependency). The wasm client is unchanged: browsers negotiate HTTP Content-Encoding with the gateway themselves. - The gateway gzips grpc-web responses (32 KiB window) for clients that send Accept-Encoding; native gRPC is never HTTP-compressed. - A gzip response that decompresses past the client's decode limit is not retried, like an uncompressed oversized response, so it no longer bans healthy nodes. On a mainnet sample, gzip saves 65% of response bytes overall (29% without responses over 64 KiB); single-identity proofs barely change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
DAPI server gzip responses packages/rs-dapi/src/server/grpc.rs, packages/rs-dapi/src/server/mod.rs, packages/rs-dapi/src/services/platform_service/protobuf_request_errors.rs |
Platform and Core server constructors set request decoding limits, a shared response encoding limit, and gzip response compression. The unified server uses these constructors. Platform service tests check gzip negotiation and verify that a response exceeding the client’s decompressed message limit returns a non-retryable status. |
Client gzip support and retry handling packages/dapi-grpc/Cargo.toml, packages/rs-dapi-client/src/transport.rs, packages/rs-dapi-client/src/transport/* |
The transport feature enables tonic/gzip. Native client constructors accept gzip responses, and pooled client creation uses the new constructors. Decompression-limit ResourceExhausted errors are not retried. Wasm client constructors and transport re-exports are added. |
Protocol-specific compression details packages/rs-dapi/doc/DESIGN.md, packages/rs-dapi/src/clients/drive_client.rs, packages/dashmate/test/unit/templates/envoyTemplate.spec.js |
The design text distinguishes compression for native gRPC, gRPC-Web, JSON-RPC, and the local Drive hop. The Envoy test checks that the compressor follows CORS, precedes grpc_web, and lists four configured content types. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Possibly related issues
- DAPI responses are not compressed (rs-dapi defers to Envoy, Envoy has no compressor) #5576: The issue reports missing DAPI response compression. This change adds negotiated gzip responses for native DAPI and tests the Envoy gRPC-Web compressor configuration.
Suggested reviewers: lklimek, quantumexplorer, thepastaclaw, shumkov
Merge Risk: 
b7d36
-
The compression changes are ready to merge with no identified correctness, availability, security, or data-integrity risk.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 52.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: adding gzip compression for DAPI responses when clients support it. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
-
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
|
@coderabbitai review 🤖 Posted autonomously by Claude on behalf of pasta. |
✅ Action performed
|
|
🔍 Review in progress — actively reviewing now (commit b7d36f1) · triage: normal |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The gzip implementation is correctly scoped across native gRPC, grpc-web, and WASM transports, with no blocking correctness, security, or layering defects found. Three non-blocking test and maintainability improvements remain: Core response compression lacks direct coverage, the retry guard depends on tonic's error wording, and the Envoy test can throw a TypeError instead of reporting an assertion failure. The Rust workspace test failure is unrelated to this PR; one browser test check was still in progress in the supplied CI snapshot.
🟡 2 suggestion(s) | 💬 1 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — This is a broad cross-cutting change to gRPC transport, server compression, gateway configuration, client retry handling, and protocol error mapping, but it does not alter consensus, funds, cryptography, key handling, or storage. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-dapi/src/server/grpc.rs`:
- [SUGGESTION] packages/rs-dapi/src/server/grpc.rs:55-62: Core response compression lacks direct test coverage
`core_server()` now enables gzip response compression, but the added integration coverage exercises only `platform_server()`. Because `core_server()` is a separate constructor, a future edit could remove or misconfigure its `send_compressed(CompressionEncoding::Gzip)` call without any test detecting the regression. Add a Core-service compression test, or expose the constructor under `cfg(test)` and cover it with a small Core harness.
In `packages/rs-dapi-client/src/transport/grpc.rs`:
- [SUGGESTION] packages/rs-dapi-client/src/transport/grpc.rs:133-137: Document the tonic-version dependency of the decompression-limit prefix
`can_retry()` identifies the client-local decompression limit by prefix-matching tonic's English status message. That wording is an implementation detail rather than a stable status discriminator. The existing test will catch a wording change when it runs, but the constant should explicitly identify the tonic decoder source and version so a future tonic upgrade prompts a deliberate review instead of silently changing retry classification if the test is skipped or weakened.
In `packages/dashmate/test/unit/templates/envoyTemplate.spec.js`:
- [NITPICK] packages/dashmate/test/unit/templates/envoyTemplate.spec.js:46-49: Avoid a TypeError when the Envoy compressor block has no content types
If the rendered compressor block contains no matching content-type lines, `match()` returns `null` and the test throws a TypeError before reaching an assertion. This produces a misleading failure and obscures the actual template regression. Normalize the match result to an empty array before mapping it.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Rust workspace test failure is pre-existing and unrelated — The supplied CI snapshot shows
Rust workspace tests / Testsfailing in the untouchedpackages/rs-sdk/src/platform/identities_contract_keys_query.rsbecauseQuerySettingsinitializers omitverified_protocol_version. The changed files do not include this module, and the failure is unrelated to the gzip transport changes.- Follow-up: Track or fix the
rs-sdkQuerySettings initializer issue separately on the appropriate development branch.
- Follow-up: Track or fix the
| /// Core service with the message limits and response compression DAPI serves it with. | ||
| /// See [platform_server]. | ||
| fn core_server(service: CoreServiceImpl) -> CoreServer<CoreServiceImpl> { | ||
| CoreServer::new(service) | ||
| .max_decoding_message_size(MAX_CORE_DECODING_BYTES) | ||
| .max_encoding_message_size(MAX_ENCODING_BYTES) | ||
| .send_compressed(CompressionEncoding::Gzip) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Core response compression lacks direct test coverage
core_server() now enables gzip response compression, but the added integration coverage exercises only platform_server(). Because core_server() is a separate constructor, a future edit could remove or misconfigure its send_compressed(CompressionEncoding::Gzip) call without any test detecting the regression. Add a Core-service compression test, or expose the constructor under cfg(test) and cover it with a small Core harness.
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)
There was a problem hiding this comment.
Not adding this one here. A Core harness that gets a successful (and therefore compressible) response needs a real StreamingServiceImpl and a Core RPC backend: every Core unary method goes to Core RPC, and errors carry no body to compress. core_server() is the same three builder calls as platform_server(), and gzip negotiation itself is tonic behaviour that the Platform test already exercises end to end. Happy to add it if someone wants a Core service test fixture for other reasons.
🤖 Posted autonomously by Claude on behalf of pasta.
| /// Start of the status tonic returns when a compressed response decompresses past the | ||
| /// client's `max_decoding_message_size`. The limit is the client's own, so every node | ||
| /// would fail the same way: this must not ban nodes like other `ResourceExhausted` | ||
| /// statuses (an uncompressed oversized response is `OutOfRange`, also not retried). | ||
| const DECOMPRESS_LIMIT_ERROR: &str = "Error decompressing: size limit"; |
There was a problem hiding this comment.
🟡 Suggestion: Document the tonic-version dependency of the decompression-limit prefix
can_retry() identifies the client-local decompression limit by prefix-matching tonic's English status message. That wording is an implementation detail rather than a stable status discriminator. The existing test will catch a wording change when it runs, but the constant should explicitly identify the tonic decoder source and version so a future tonic upgrade prompts a deliberate review instead of silently changing retry classification if the test is skipped or weakened.
| /// Start of the status tonic returns when a compressed response decompresses past the | |
| /// client's `max_decoding_message_size`. The limit is the client's own, so every node | |
| /// would fail the same way: this must not ban nodes like other `ResourceExhausted` | |
| /// statuses (an uncompressed oversized response is `OutOfRange`, also not retried). | |
| const DECOMPRESS_LIMIT_ERROR: &str = "Error decompressing: size limit"; | |
| /// Prefix emitted by tonic 0.14's decoder when the client's own | |
| /// `max_decoding_message_size` is exceeded while decompressing a response. | |
| /// Re-check this prefix against tonic's decoder implementation on every tonic | |
| /// upgrade; this failure is client-local and must not cause healthy nodes to be | |
| /// retried or banned. | |
| const DECOMPRESS_LIMIT_ERROR: &str = "Error decompressing: size limit"; |
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)
There was a problem hiding this comment.
Done in b7d36f1: the doc now names tonic 0.14's codec::decode as the source and says to re-check it on tonic upgrades. should_not_ban_nodes_for_a_gzip_response_over_the_client_limit also fails if the wording changes (verified by removing the guard).
🤖 Posted autonomously by Claude on behalf of pasta.
| const contentTypes = envoyConfig | ||
| .slice(compressor, grpcWeb) | ||
| .match(/^\s+- application\/[\w+-]+$/gm) | ||
| .map((line) => line.trim().slice(2)); |
There was a problem hiding this comment.
💬 Nitpick: Avoid a TypeError when the Envoy compressor block has no content types
If the rendered compressor block contains no matching content-type lines, match() returns null and the test throws a TypeError before reaching an assertion. This produces a misleading failure and obscures the actual template regression. Normalize the match result to an empty array before mapping it.
| const contentTypes = envoyConfig | |
| .slice(compressor, grpcWeb) | |
| .match(/^\s+- application\/[\w+-]+$/gm) | |
| .map((line) => line.trim().slice(2)); | |
| const contentTypes = (envoyConfig | |
| .slice(compressor, grpcWeb) | |
| .match(/^\s+- application\/[\w+-]+$/gm) ?? []) | |
| .map((line) => line.trim().slice(2)); |
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)
There was a problem hiding this comment.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
…st match Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Waiting for bot review — coderabbitai ✓ · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
|
@coderabbitai review 🤖 Posted autonomously by Claude on behalf of pasta. |
|
|
@coderabbitai review 🤖 Posted autonomously by Claude on behalf of pasta. |
✅ Action performed
|
Issue being fixed or feature implemented
DAPI sends every response uncompressed. rs-dapi's Drive client says compression is "handled by Envoy", but the gateway has no compressor filter, and neither rs-dapi nor rs-dapi-client enables tonic compression. Checked live against mainnet evonodes (DAPI 4.1.x): native gRPC responses carry no
grpc-encodingeven when the client sendsgrpc-accept-encoding: gzip, and grpc-web responses carry noContent-Encodingeven when the client sendsAccept-Encoding: gzip, br.Replaces #5576.
What was done?
grpc-accept-encoding. Clients that don't list it get uncompressed responses as before. Compressed requests are still not accepted. The server setup moves intoplatform_server()/core_server(), so the test harness serves exactly the production configuration.Content-Encodingwith the gateway and decompress on their own. Also,grpc-accept-encodingisn't in the gateway's CORS allow-list, so sending it from a browser would fail the preflight.max_decoding_message_size, tonic returnsResourceExhausted("Error decompressing: size limit…") instead of theOutOfRangean uncompressed oversized response gets.ResourceExhaustedis retryable and bans the node, so without this fix one oversized response would ban healthy nodes one after another, for a limit that lives in the client. That status is now not retryable, the same asOutOfRange.application/grpc-web*). It sits abovegrpc_web, so the trailer frame is compressed too. Nativeapplication/grpcresponses are never HTTP-compressed; rs-dapi compresses those messages itself. It useswindow_bits: 15(zlib's 32 KiB window) instead of Envoy's 4 KiB default. On the mainnet sample, the default window would cut savings on responses over 64 KiB from 82.6% to 5.5%, and over all messages from 55.2% to 12.3%. The cost is about 144 KiB of zlib state per in-flight compressed response.transportfeature enablestonic/gzip.packages/rs-dapi/doc/DESIGN.mdis corrected. The rs-dapi → Drive hop stays uncompressed because it's local.js-dapi-client in Node (
@grpc/grpc-js) already sendsgrpc-accept-encoding: identity,deflate,gzip, so it gets compressed responses without changes.Measurements (real responses)
162 read-only responses collected from 3 mainnet evonodes on 2026-10-25 (protocol 13, height 458,086), plus 227 from testnet. Each message was compressed on its own, the way gRPC per-message compression works. Savings are by bytes, gzip level 6, which is what tonic uses.
getFinalizedEpochInfosgetCurrentQuorumsInfogetDocuments(DPNS, DashPay)getDataContract/getDataContractsgetEpochsInfoTestnet (protocol 14): 47.8% overall, 43.1% excluding responses over 64 KB.
The savings come from embedded payloads: contract schemas, documents, epoch and quorum data. Small proofs that are mostly 32-byte Merk hashes don't compress. 37 of the 119 mainnet messages, almost all single-identity proofs of 1–4 KB, come out slightly larger, by at most 23 bytes. I did not add a size threshold. tonic has none built in, and the loss on those messages is tiny next to the gains on the large ones.
zstd-3 saves within 1–4 points of gzip-6 at about 4× less CPU. The cost is a C dependency, so it is left for later. Measured CPU for gzip-6 (single thread, Apple M4 Pro, zlib): median 24 µs to compress a message, 5 µs to decompress, about 117 MB/s compression throughput. That's small next to the cost of producing a proof.
The percentages depend on the query mix, not on production traffic. Mainnet nodes were on 4.1.x, so v5 response shapes may differ somewhat.
How Has This Been Tested?
should_gzip_responses_only_for_clients_that_accept_gzip. It serves the productionplatform_server()configuration over an in-memory connection and checks three things: a plain tonic client gets nogrpc-encoding, a client built withrs_dapi_client::transport::new_platform_clientgetsgrpc-encoding: gzip, and both decode to the same message.should_not_ban_nodes_for_a_gzip_response_over_the_client_limit. A compressible response goes through the real server to a client with a 1 KiB decode limit, and the test asserts the status is not retryable. Without the retry fix it fails with exactly thatResourceExhaustedstatus.corsandgrpc_weband restricts it to the grpc-web content types.cargo test -p rs-dapi --lib protobuf_request_errors,cargo test -p rs-dapi --lib server::andcargo test -p rs-dapi-client --all-features.cargo clippy -p rs-dapi -p rs-dapi-client -p dapi-grpc --all-features --all-targets --no-deps -- -D warnings.cargo check --target wasm32-unknown-unknownforrs-dapi-clientandwasm-sdk. flate2 is not in the wasm dependency tree.packages/dashmate/scripts/validate-envoy-config.js --smoke: all 8 config variants and the rate-limit smoke checks pass on bothenvoyproxy/envoy:v1.39.0and the pinneddashpay/envoy:1.39.0-impr.1.Accept-Encoding: gzip:content-encoding: gzip, 4,396 → 2,083 bytes on the wire, message and trailer frame intact after decompression.Accept-Encoding: unchanged response.Accept-Encoding: gzip: compressed.grpc-encoding: gzip, 4,361 → 2,035 bytes, decompresses to the exact payload). Envoy does not HTTP-compressapplication/grpc.Breaking Changes
None. Compression only happens when the client asks for it, so old clients see no change, and new clients work against old nodes.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
b7d36f1/skip-botsproceeds without the ones not yet reported/self-reviewedCargo.lock,packages/dapi-grpc/Cargo.toml,packages/rs-dapi-client/src/transport.rsand 3 more) — QuantumExplorer or shumkovdashmate(packages/dashmate/templates/platform/gateway/envoy.yaml.dot,packages/dashmate/test/unit/templates/envoyTemplate.spec.js) — ktechmidas or shumkovrust-dapi(packages/rs-dapi/doc/DESIGN.md,packages/rs-dapi/src/clients/drive_client.rs,packages/rs-dapi/src/server/grpc.rsand 2 more) — QuantumExplorer or lklimek or shumkovWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit