Skip to content

feat(ffi): external-signer merkle-batch upload support - #202

Merged
Nic-dorman merged 3 commits into
mainfrom
feat/ffi-external-signer-merkle
Jul 6, 2026
Merged

feat(ffi): external-signer merkle-batch upload support#202
Nic-dorman merged 3 commits into
mainfrom
feat/ffi-external-signer-merkle

Conversation

@Nic-dorman

Copy link
Copy Markdown
Collaborator

What

Extends the external-signer FFI (ant-ffi) to support merkle-batched uploads. Previously prepare_* returned InvalidInput ("wave-batch only") whenever an upload was large enough to trigger merkle batching; now the merkle path is fully wired. Mirrors the antd daemon's existing gRPC/REST merkle mapping — no ant-core changes.

Follow-up to #199. Tracked as Linear V2-570.

Merkle vs wave-batch

wave-batch (already shipped) merkle (this PR)
on-chain call payForQuotes((addr,uint256,bytes32)[]) payForMerkleTree(uint8 depth, PoolCommitment[], uint64 ts)
finalize input {quote_hash: tx_hash} map winner_pool_hash (from the MerklePaymentMade event)
prepare returns payments[] + total_amount depth + pool_commitments[] + merkle_payment_timestamp

Changes (all in ant-ffi)

  • New records PoolCommitmentEntry { pool_hash, candidates } and CandidateNodeEntry { rewards_address, amount }. PreparedUploadInfo gains depth / pool_commitments / merkle_payment_timestamp (empty/0 for wave-batch; payment_type is "wave_batch" or "merkle").
  • stash_prepared maps ExternalPaymentInfo::Merkle to the payForMerkleTree inputs. already_stored is now derived from the prepared preflight (already_stored_addresses vs total_chunks) so it's correct for both shapes (merkle never produces per-quote payments).
  • finalize_upload_merkle / finalize_upload_merkle_with_progress — finalize a merkle upload with the winner_pool_hash; validates the hash before taking the session so bad input is lossless.
  • take_session routes wave vs merkle and rejects a mis-routed finalize (finalize_upload on a merkle upload, or vice-versa) with a clear error without consuming the prepared upload. Same post-payment non-retryability caveat as wave-batch (external-signer finalize: post-payment storage failure strands the payment (PreparedUpload consumed, not retryable) ant-client#140).
  • Unit tests for winner_pool_hash/hash validation + visibility parsing.

Verification

  • cargo test -p ant-ffi ✅ (4 pass)
  • cargo clippy -p ant-ffi --all-targets -- -D warnings
  • cargo fmt -p ant-ffi --check
  • UniFFI generation from the release dylib exposes finalizeUploadMerkle / finalizeUploadMerkleWithProgress + PoolCommitmentEntry / CandidateNodeEntry for both Swift and Kotlin.

Scope / follow-up

App-side work — building the payForMerkleTree calldata and parsing winner_pool_hash from the MerklePaymentMade receipt event — is part B, landing with the ant-mobile-* app repos (needs this in a released SDK first).

🤖 Generated with Claude Code

Nic and others added 3 commits July 6, 2026 15:52
Extends the external-signer FFI surface to handle merkle-batched uploads,
which previously returned InvalidInput ("wave-batch only"). Mirrors the
antd daemon's gRPC/REST merkle mapping.

- New records PoolCommitmentEntry + CandidateNodeEntry; PreparedUploadInfo
  gains depth / pool_commitments / merkle_payment_timestamp (empty/0 for
  wave-batch; payment_type discriminates "wave_batch" vs "merkle").
- stash_prepared maps ExternalPaymentInfo::Merkle -> the payForMerkleTree
  inputs (depth, pool commitments with 0x pool_hash + candidate
  rewards_address/price, timestamp). already_stored now derived from the
  prepared preflight (works for both shapes).
- finalize_upload_merkle / _with_progress: finalize a merkle upload with
  the winner_pool_hash from the MerklePaymentMade event; validates the hash
  before taking the session (bad input is lossless).
- take_session routes wave vs merkle and rejects a mis-routed finalize with
  a clear error WITHOUT consuming the prepared upload (retryable). Same
  post-payment non-retryability caveat as wave (ant-client#140).
- Unit tests for hash validation + visibility parsing.

Verified: cargo test / clippy -D warnings / fmt --check clean; UniFFI
generates finalizeUploadMerkle(WithProgress) + PoolCommitmentEntry /
CandidateNodeEntry for both Swift and Kotlin. App-side calldata
(payForMerkleTree + MerklePaymentMade parsing) is the follow-up (V2-570
part B, lands with the ant-mobile-* repos).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The ffi/rust/ant-ffi subtree had no CI — ci.yml only covers antd/ and
antd-rust/, go-ci.yml covers antd-go/, and release.yml builds the antd
binary. So FFI PRs (#199, #202) had no automated signal; build/clippy/
fmt/test/bindgen were only ever run locally.

Adds ffi-ci.yml mirroring the check-antd job (fmt --all --check, clippy
--all-targets --all-features --locked -D warnings, cargo doc, cargo test),
scoped to ffi/rust/**, plus a UniFFI generation smoke step (generate Swift
+ Kotlin from the release dylib) that catches broken #[uniffi::export]
surfaces a plain build misses. No Xcode/NDK needed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bump ant-ffi's ant-core pin from the untagged rev 95a23bee (0.2.9) to the
released tag ant-cli-v0.2.10 (0.3.0), matching antd v0.11.0 (#200) so the
whole SDK ships on one ant-core. ant-protocol stays 2.2.2 (already aligned).

0.3.0 removed `UploadEvent::WaveComplete` — dropped its arm in
map_upload_event; per-chunk `ChunkStored` already carries the running total.

Verified against 0.3.0: build, clippy -D warnings, fmt --check, tests
(4 pass), and UniFFI Swift+Kotlin generation (incl. the merkle surface).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@dirvine dirvine left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes review

Reviewed PR #202 at f21d9331cccdce100f1e7ec37819b37ecda6f7f7. I do not see a blocker in the FFI merkle wiring.

Verified

  • GitHub Check (ant-ffi)
  • cargo build -p ant-ffi
  • cargo test -p ant-ffi --no-fail-fast ✅ — 4 tests pass
  • cargo clippy -p ant-ffi --all-targets -- -D warnings
  • cargo fmt -p ant-ffi --check
  • cargo doc --locked --no-deps
  • Release dylib + uniffi-bindgen Swift/Kotlin generation ✅; generated bindings expose finalizeUploadMerkle, finalizeUploadMerkleWithProgress, PoolCommitmentEntry, CandidateNodeEntry, poolCommitments, and merklePaymentTimestamp.

Review notes

  • The merkle path validates winner_pool_hash before taking the session, and take_session rejects wave/merkle mis-routing without consuming the prepared upload. That preserves the same lossless bad-input behaviour we required for #199.
  • The post-payment storage/network failure caveat is still documented as non-retryable and points at WithAutonomi/ant-client#140 / WithAutonomi/ant-sdk#201, which is honest about the current core limitation.
  • The PreparedUploadInfo merkle fields match the core PreparedMerkleBatch public material: depth, pool commitments, candidate reward addresses/prices, and merkle payment timestamp. I did not run an end-to-end payForMerkleTree mobile-wallet flow; that remains app-side follow-up as the PR body says.
  • Adding FFI CI is a useful improvement. Small mercy: future reviewers may now have fewer opportunities to pretend local build logs are a lifestyle choice.

Approved, with the same production caveat as #199: this is fine as the SDK surface, but production mobile external-signer UX still needs #140/#201 before post-payment storage failures are retry-safe.

@Nic-dorman
Nic-dorman merged commit 7aad524 into main Jul 6, 2026
1 check passed

@dirvine dirvine left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes follow-up review

I need to retract my earlier approval. A parallel pass found one merkle-specific retry bug that I agree is blocker-class for the external-signer contract.

Blocking: a valid-but-wrong winner_pool_hash consumes the prepared merkle upload

finalize_merkle_inner validates that winner_pool_hash is 32-byte hex, then immediately removes the merkle session:

  • ffi/rust/ant-ffi/src/client.rs:898-899

The semantic validation — “is this winner actually one of the prepared candidate pools?” — happens later inside ant-core after the PreparedUpload has already been moved by value:

  • ant-core/src/data/client/merkle.rs:1288-1305 (finalize_merkle_batch returns Winner pool ... not found in candidate pools)

That means a mobile app can parse the wrong log/event/receipt field, or use a receipt from the wrong network/tx, and still pass a syntactically valid 32-byte hash. The call then errors, but the only prepared upload state has already been removed and consumed, so the caller cannot retry with the corrected winner_pool_hash.

This is distinct from the known post-payment storage-failure limitation tracked in WithAutonomi/ant-client#140 / WithAutonomi/ant-sdk#201. This failure happens before chunk storage and before any retry-state problem; it is semantic bad input from the app side, and the FFI contract currently says bad input/mis-routing is lossless.

Please either:

  1. validate the winner against the prepared merkle batch before removing/consuming the session; or
  2. change ant-core to return the PreparedUpload / retry state on proof-generation errors as well, then keep/reinsert the session on this error; or
  3. if neither can happen in this PR, document this explicitly as another non-retryable-after-payment failure mode. I would prefer not to ship it that way unless the app work is definitely prepared to treat a wrong parsed winner hash as a paid-upload terminal failure.

Non-blocking follow-ups

  • Merkle external-signer finalization currently reports storage_cost_atto = "0" and gas_cost_wei = "0" because ant-core hardcodes those fields in the external-signer merkle path (ant-core/src/data/client/file.rs:1870-1878). The FFI result docs say those are total paid costs and only zero if all chunks pre-existed. That should be documented or fixed in core, otherwise mobile/accounting can read a paid merkle upload as free.
  • Progress bridge tasks are only awaited on success; on error the ? returns before flushing the bridge (client.rs:876-883, :909-916, and download :978-983). Better pattern: store the core result, await the bridge, then return result?, so callback lifetime is deterministic on mobile.

Requesting changes for the winner_pool_hash session-consumption issue. Less fun than approving twice, but less exciting than a paid upload nobody can retry.

@dirvine

dirvine commented Jul 6, 2026

Copy link
Copy Markdown

Follow-up from the second review pass: the blocker remains the winner_pool_hash session-consumption issue in my requested-changes review above. A few documentation/API consistency points are also worth fixing while this PR is open:

  • docs/external-signer-flow.md:322-332 still says the merkle path is “out of scope”, says SDKs see payment_type == "merkle_batch", and describes the examples as future work. This PR exposes the FFI/mobile merkle path now, and the discriminator in PreparedUploadInfo is "merkle", not "merkle_batch" (ffi/rust/ant-ffi/src/lib.rs:86-96). Mobile implementors following the doc will branch on the wrong value. Quietly unhelpful.
  • connect_for_external_signer docs still say prepare then finalize_upload (ffi/rust/ant-ffi/src/client.rs:621-626). With this PR the top-level contract should say: prepare, inspect PreparedUploadInfo.payment_type, then call finalize_upload for wave_batch or finalize_upload_merkle for merkle.
  • PreparedUploadInfo.already_stored says finalize may be called with “an empty map / any winner hash” (ffi/rust/ant-ffi/src/lib.rs:117-119). The implementation still validates a 32-byte winner_pool_hash before taking the session, and finalizer shape must match the stored payment type. Suggested wording: no payment is needed; call the finalizer matching payment_type. For wave-batch, use finalize_upload(upload_id, {}); for merkle, a valid 32-byte winner hash is still required if the payment type is merkle.
  • Contract method naming is inconsistent in repo docs/comments: FFI uses payForMerkleTree, while daemon/proto comments still mention payForMerkleTree2 in places. Please pick the canonical name/signature and align the docs so the app team has one thing to implement.

I’d treat these as medium/low follow-ups behind the session-consumption blocker, not separate blockers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants