fix(antd): accept empty tx_hashes for all-already-stored wave-batch finalize - #239
Merged
Conversation
…inalize A wave-batch prepare where every chunk is already stored reports zero payments, and ant-core + REST both accept the resulting no-payment finalize — but the gRPC FinalizeUpload handler rejected an empty tx_hashes map outright, so a repeated/deduplicated upload could prepare over gRPC but never get its DataMap (ant-sdk#233). Extract the wave-batch validation into a shared resolve_wave_tx_hashes (mirroring resolve_winner_pool_hashes) used by both transports: - an empty map is valid exactly when prepare reported no payments; otherwise it errors naming the expected payment count - every reported quote must have a receipt: ant-core only rejects a missing one after the pending upload has been consumed, so catch the incomplete map up front while the upload_id is still retryable - unknown extra entries stay tolerated (ant-core ignores them) Validation keeps running before the pending upload is consumed on both transports, so all rejections preserve retry state. Wire-neutral: proto3 cannot distinguish absent from empty maps, so no proto shape change and no client regen — proto/OpenAPI/flow-doc comments updated only. Fixes #233 Linear: V2-1026 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Hermes reviewVerdict: no material findings; merge looks safe. Reviewed exact head
Verification on this head:
Review panel: five seats recommended merge; one objected that an unexpectedly empty payment intent could permit unpaid storage. Direct inspection of the pinned — Hermes |
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
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.
Summary
Fixes #233 (reported by @JimCollinson): the gRPC
UploadService.FinalizeUploadhandler rejected an emptytx_hashesmap withinvalid_argument, even for the valid wave-batch case where prepare reported zero payments because every chunk is already stored. ant-core and REST both accept that no-payment finalize, so a repeated/content-deduplicated public upload could prepare over gRPC but never finalize and return its DataMap.The second half of #233 — the handler consuming
upload_idbefore validating — was already fixed on main by the 0.12.0 multi-batch rework (peek-before-consume, mirroring REST); this PR completes the remaining half and hardens the same failure mode one step further.Change
Extracted the wave-batch validation into a shared
resolve_wave_tx_hashesinrest/upload.rs(mirroringresolve_winner_pool_hashes), now used by both transports. The shape peek captures the prepare's expected quote hashes, and the resolver checks the request against them before the pending upload is consumed:build_paid_chunks— after the upload_id had been consumed, stranding the prepared upload exactly like antd gRPC FinalizeUpload rejects empty tx_hashes for all-already-stored uploads #233's empty-map case. Now it's a pre-consumeinvalid_argument/400naming the missing quote.payment_intent.payments).Also de-duplicates the previously identical ~35-line parse block that existed in both handlers.
Wire compatibility
None — proto3 cannot distinguish an absent map from an empty one, so this is purely daemon-side validation. No proto shape change, no client regen; all typed gRPC bindings inherit the fix.
upload.proto/openapi.yaml/docs/external-signer-flow.mdcomment-only updates document the empty-map contract. REST behavior change is strictly early-error: a present-but-emptytx_hasheswith payments expected now 400s before consuming the upload instead of failing in ant-core after consuming it (absenttx_hashesremains a 400 as before).Tests
New deterministic unit tests over the pure resolver (same convention as the
resolve_winner_pool_hashessuite — there is no network-backed gRPC harness in this crate):empty_map_valid_when_no_payments_expected— the antd gRPC FinalizeUpload rejects empty tx_hashes for all-already-stored uploads #233 repro: zero payments → empty map → Okempty_map_rejected_when_payments_expected— protective rejection kept, pre-consumemissing_receipt_rejected_before_consuming_upload— partial map caught before the upload_id is consumedfull_coverage_resolves_with_extra_entries_tolerated— 0x-prefixed + bare hex, extras passbad_hex_rejected— parse errors preservedcargo test(50 passed),cargo clippy --all-targets -- -D warnings,cargo fmt --checkall green.Validation ordering (validate → only then consume) is structural in both handlers, so every rejection above leaves the prepared upload present and retryable.
Linear
V2-1026
🤖 Generated with Claude Code