Skip to content

feat(ffi): re-add external-signer upload surface (wave-batch) - #199

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

feat(ffi): re-add external-signer upload surface (wave-batch)#199
Nic-dorman merged 7 commits into
mainfrom
feat/ffi-external-signer

Conversation

@Nic-dorman

@Nic-dorman Nic-dorman commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

What

Re-adds the external-signer upload flow to ant-ffi (dropped in #150 during the ant-core refresh). The app constructs the on-chain payment, an external wallet signs it, and the client holds no private key — the store-policy-safe path for mobile.

ant-core already has the full two-phase flow (file/data_prepare_upload_with_visibilityfinalize_upload); this wires a UniFFI-friendly surface over it.

New Client API

  • connect_for_external_signer(peers, rpc_url, token, vault) — configures the EVM network with no wallet (quotes/price queries work; payment is signed off-device).
  • prepare_data_upload / prepare_file_upload(…, visibility)PreparedUploadInfo — encrypts, collects quotes, returns the payForQuotes payment entries + an opaque upload_id.
  • finalize_upload(upload_id, {quote_hash: tx_hash})ExternalUploadResult — stores the chunks once the wallet has paid.
  • cancel_upload(upload_id) — discard a prepared upload that will not be finalized, freeing its in-memory payload.
  • Progress variants: finalize_upload_with_progress, download_public_to_file / download_private_to_file report live progress via a ProgressListener callback.

New records: PaymentEntry, PreparedUploadInfo, ExternalUploadResult, ProgressUpdate.

Design note

The PreparedUpload payload (chunk content, PreparedChunk/merkle state) can't cross the FFI boundary, so prepare_* stashes it in an in-memory session map keyed by upload_id and returns only the summary; finalize_upload consumes it. Same contract the antd daemon exposes over gRPC/REST.

Breaking changes

  • Removed ClientError.code() and WalletError.code() (previously #[uniffi::export]ed). These numeric error codes are no longer part of the generated Swift/Kotlin surface — consumers should match on the error variant instead. Pre-1.0, but calling it out so it lands in the release notes.

Scope

  • Wave-batch only for now; a merkle-batched prepare returns a clear InvalidInput error (tracked: Linear V2-570).
  • Binding regen + ant-swift/ant-android bumps are follow-ups (the Swift xcframework rides on the dynamic-framework packaging PR).

Verification

cargo build -p ant-ffi clean; cargo clippy -p ant-ffi --all-targets -- -D warnings clean; cargo fmt -p ant-ffi --check clean; uniffi-bindgen generates the new methods + types for both Swift and Kotlin.

Nic and others added 5 commits July 2, 2026 11:12
Adds the WalletConnect/external-signer flow back to ant-ffi (dropped in
#150 during the ant-core refresh). The app builds the on-chain payment,
an external wallet signs it, and no private key is held by the client.

New Client API:
- connect_for_external_signer(peers, rpc_url, token, vault): EVM network
  configured, NO wallet — quotes work, payment is signed off-device.
- prepare_data_upload / prepare_file_upload(visibility) -> PreparedUploadInfo:
  encrypts + collects quotes, returns the payForQuotes payment entries and an
  opaque upload_id. The heavy PreparedUpload state (chunk content) stays in
  Rust in an in-memory session map keyed by upload_id (it can't cross FFI).
- finalize_upload(upload_id, {quote_hash: tx_hash}): stores the chunks after
  the external wallet has paid, returns data map / address.

New records: PaymentEntry, PreparedUploadInfo, ExternalUploadResult.

Wave-batch only for now; merkle prepares return a clear InvalidInput error
(follow-up). Verified: ant-ffi compiles; Swift + Kotlin bindings generate
the new methods + types.

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

build-android.sh generated Kotlin bindings from
target/release/libant_ffi.dylib, but the script never builds that host
cdylib (it only cross-compiles the Android .so's and builds the bindgen
*binary*). So the dylib could be stale, producing bindings that lag the Rust
source — the new external-signer methods were silently missing from the AAR.

Point uniffi-bindgen at the freshly cross-compiled aarch64 Android .so
(UniFFI metadata is arch-independent).

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

- Bump the pinned ant-core rev to latest main (0.2.9) + ant-protocol 2.2.2,
  so the mobile client matches a devnet built from current ant-client.
- Add Client.connect_from_devnet_manifest_external_signer(path): same
  devnet-tuned node config as the wallet variant but attaches only the EVM
  network (no wallet) — required for the Sepolia devnet, whose manifest has an
  empty wallet_private_key ("bring your own wallet").

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The static-library xcframework copied its `module.modulemap` into the
consumer's shared `BUILT_PRODUCTS_DIR/include/`, colliding with any other
static xcframework doing the same — notably Reown AppKit's `yttrium`:
"Multiple commands produce .../include/module.modulemap". This blocked
AntFfi from coexisting with WalletConnect (and likely other native crypto
SDKs) in one iOS app.

`build-swift.sh` now packages each slice as a DYNAMIC `.framework` built
from the cdylib (flat layout for iOS, versioned bundle for macOS): the
modulemap lives in `Modules/`, `install_name` is set to `@rpath`, and a
generated `Info.plist` carries a valid bundle id. Multiple such frameworks
coexist cleanly in one app.

Verified end-to-end: a demo app that links Reown AppKit builds for the
iPhone simulator against the pipeline-produced xcframework — collision gone.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a UniFFI callback interface so consumers get real % progress:
- ProgressUpdate{phase,done,total} record + ProgressListener trait
- Client.finalize_upload_with_progress / download_public_to_file /
  download_private_to_file bridge ant-core's UploadEvent / DownloadEvent
  (tokio mpsc) to the listener; the download variants stream to a file.
- tokio "sync" feature for the mpsc bridge.

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

Copy link
Copy Markdown
Collaborator Author

Self-review notes from a pass over this branch (build confirmed green: cargo build -p ant-ffi at f4a3e14). No blocking issues — the std::sync::Mutex session map (guard never held across .await) and the mpsc→ProgressListener progress bridge both look correct. Four non-blocking observations for the record:

  1. Breaking API removal. #[uniffi::export] ClientError::code() and WalletError::code() are dropped. Fine pre-1.0, but it's a consumer-visible surface change — worth a line in the release notes so any existing binding consumer isn't surprised.

  2. Merkle path is unsupported. stash_prepared returns InvalidInput when ExternalPaymentInfo::Merkle is hit ("wave-batch only"). Documented and intentional, just flagging it as a known follow-up (larger uploads will trip it).

  3. ant-protocol pin coupling. ant-protocol = "2.2.2" must stay in lockstep with ant-core's pinned rev (95a23bee…) to avoid evmlib graph skew. Whenever the ant-core rev is bumped, re-verify this pin — the Cargo.toml comment calls this out, but it's an easy one to miss.

  4. Poison panic. sessions.lock().expect("sessions mutex poisoned") panics on lock poisoning. Unreachable in practice (no panic occurs between lock and unlock), so not worth changing — noting for completeness.

Verified locally; not posting a formal review (leaving the merge to a human).

@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

Requesting changes for one introduced local-check failure. The core FFI build and binding generation are basically there, but this should be fixed before merge.

Blocking

  • cargo clippy -p ant-ffi --all-targets -- -D warnings now fails on the PR head with clippy::doc_lazy_continuation in ffi/rust/ant-ffi/src/lib.rs:107-108. Baseline origin/main passes the same clippy command, so this is introduced by this PR. Indent those continuation doc lines or add a blank doc line before the paragraph.

Warnings / follow-ups

  • cargo fmt --check fails on the PR head. origin/main already has some pre-existing wallet.rs formatting drift, but the PR adds additional formatting diffs in changed files (client.rs, lib.rs, data.rs). Worth running cargo fmt before merge, even if you keep unrelated baseline cleanup separate.
  • finalize_upload removes the prepared upload from the in-memory session map before tx-hash parsing and before the network finalization succeeds (client.rs:750-778). That mirrors the daemon flow, but it means a malformed tx hash or transient store failure consumes the only copy of the prepared chunks. For an external-wallet/mobile flow this is a nasty retry story after the user has paid. At minimum, parse/validate before removing; ideally document or add a retry/cancel contract if the core PreparedUpload ownership model prevents retaining it through a failed finalize.
  • download_private_to_file decodes the caller-provided DataMap hex without the size guard used by data_get_private (client.rs:724-738 vs existing cap at client.rs:522-532). If this FFI surface is callable from app/UI input, reuse the same cap to avoid an avoidable memory spike.

Verified locally

  • cargo build -p ant-ffi
  • cargo test -p ant-ffi --no-fail-fast ✅ (0 tests)
  • cargo build --release -p ant-ffi && cargo build --release --bin uniffi-bindgen
  • Swift and Kotlin UniFFI generation from target/release/libant_ffi.dylib ✅; generated methods include connectForExternalSigner, prepareDataUpload, finalizeUpload, finalizeUploadWithProgress, and the download-to-file/progress APIs.
  • gh pr checks 199 reports no checks on this branch, so there is no CI signal to lean on here.

One small mercy: no private key appears to be introduced in the external-signer constructor path. Quietly behaving as advertised.

@dirvine

dirvine commented Jul 6, 2026

Copy link
Copy Markdown

Follow-up from the parallel review pass: my requested-changes review already covers the destructive finalize_upload retry issue, but two additional points are worth recording before this lands.

  • Prepared-upload sessions need lifecycle controls. Client.sessions stores PreparedUpload values that hold chunk content, and stash_prepared inserts every successful prepare with no TTL, cancellation API, count/byte limit, or cleanup path other than successful finalize (ffi/rust/ant-ffi/src/client.rs:127-130, :846-849). A mobile/FFI caller can repeatedly prepare and never finalize, retaining payload data for the lifetime of the client. Please add a cancel/expiry/limit story, or at least document the lifecycle and memory cost clearly.

  • Progress docs overpromise current upload coverage. ProgressUpdate docs list upload phases encrypting, quoting, and storing (ffi/rust/ant-ffi/src/lib.rs:103-108), but the exported prepare methods do not accept a listener, and finalize_upload_with_progress is documented/implemented as storing-only (ffi/rust/ant-ffi/src/client.rs:654-679, :695-705). Either add prepare-with-progress APIs, or narrow the docs to say external-signer upload progress currently covers finalize/storage only.

I’d keep the destructive finalize behaviour as the merge blocker. These are follow-up quality/resource-safety issues, but they are real enough to avoid surprising mobile consumers later. Tiny foot-gun, large boot.

…t, caps)

Hermes review follow-ups on the external-signer surface:

- finalize_upload: parse & validate ALL tx hashes BEFORE removing the
  prepared upload from the session map. The caller has already paid
  on-chain, so a malformed hash no longer destroys the only in-memory
  copy of the prepared chunks — bad input now leaves the upload intact
  and retryable. Documented the remaining constraint: ant-core consumes
  PreparedUpload by value, so a *network* finalize failure still consumes
  it (re-prepare; the same payment is reusable).
- add cancel_upload(upload_id) so a caller can discard a prepared upload
  that will never be finalized and free its payload buffer; documented
  the sessions map lifecycle + memory cost on the field.
- download_private_to_file: apply the same 20MB hex input cap as
  data_get_private (reachable from app/UI input).
- narrow ProgressUpdate docs: external-signer upload progress currently
  covers the "storing" phase only (finalize); encrypting/quoting are not
  yet surfaced (prepare_* takes no listener).
- fix clippy::doc_lazy_continuation and run cargo fmt on the changed
  files (client.rs, lib.rs, data.rs, wallet.rs).

Verified: cargo clippy -p ant-ffi --all-targets -- -D warnings (clean),
cargo fmt -p ant-ffi --check (clean), cargo build -p ant-ffi (ok).

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

Copy link
Copy Markdown
Collaborator Author

Thanks @dirvine — addressed in aae287b. Rundown against the review:

Blocking

  • clippy doc_lazy_continuation — fixed (lib.rs ProgressUpdate doc restructured; the new sessions doc also kept clippy-clean). cargo clippy -p ant-ffi --all-targets -- -D warnings is green.
  • Destructive finalize_upload — reordered so all tx hashes are parsed & validated before the prepared upload is removed from the session map. Malformed input now errors with the upload left intact and retryable. The residual case is inherent to the core API: finalize_upload_with_progress takes PreparedUpload by value, so a genuine network store failure still consumes it — documented inline that the caller must re-prepare (the same on-chain payment is reusable). Happy to push a finalize(&PreparedUpload) / return-on-error contract into ant-core as a follow-up if you'd prefer lossless network-failure retry.

Warnings / follow-ups

  • cargo fmt — ran on the changed files (client.rs, lib.rs, data.rs, wallet.rs). All four are files this PR already touches, so no unrelated baseline files were swept; cargo fmt -p ant-ffi --check is clean.
  • download_private_to_file cap — now applies the same 20 MB hex-input guard as data_get_private.
  • Session lifecycle — added cancel_upload(upload_id) -> bool to discard a prepared upload and free its payload buffer, and documented the map's lifecycle + memory cost on the field. A bounded/TTL cache is noted as a possible follow-up rather than built now — shout if you want a hard cap in this PR.
  • Progress docs overpromise — narrowed: external-signer upload progress currently covers the storing phase only (during finalize); encrypting/quoting happen inside prepare_*, which takes no listener yet (called out as a possible prepare-with-progress follow-up).

Verified locally: clippy -D warnings clean, fmt --check clean, cargo build -p ant-ffi ok. No CI on this branch yet — a downstream publish-ffi workflow for the Swift side is drafted separately.

@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 re-review after aae287b

Thanks for the update. The local check failures and several API polish points are fixed:

  • cargo build -p ant-ffi
  • cargo test -p ant-ffi --no-fail-fast ✅ (still 0 tests)
  • cargo clippy -p ant-ffi --all-targets -- -D warnings
  • cargo fmt -p ant-ffi --check
  • Swift/Kotlin UniFFI generation from the fresh release dylib ✅, including cancelUpload.

I’m still not comfortable approving yet because the remaining network-failure path is more serious than the inline comment suggests.

Still blocking: network finalize failure still consumes the only reusable proof material

finalize_inner now validates tx hash syntax before removing the prepared upload, which fixes malformed-input retry. But after that it still removes the PreparedUpload and passes it by value into ant-core:

  • ffi/rust/ant-ffi/src/client.rs:806-843

The new inline comment says a network store failure consumes it and “the caller must then re-prepare (safe: the same on-chain payment can be reused).” I don’t think that safety claim is established. In evmlib 0.8.1, PaymentQuote::hash() includes the quote timestamp, node public key, and node signature (data_payments.rs:101-107, with timestamp included in bytes_for_signing at :111-124). A later re-prepare can collect fresh quotes with different quote hashes, so the caller’s old {quote_hash: tx_hash} map may not match the new PreparedUpload. The original prepared quotes/proofs are exactly the material being dropped.

So the bad-input case is fixed, but a transient storage/network failure after the user has paid can still strand the paid attempt unless the original prepared upload/proofs are retained or ant-core returns enough state to retry. At minimum, the public FFI contract should not say or imply that re-prepare safely reuses the same payment. Preferably this PR should either:

  1. keep the prepared state retryable until storage succeeds; or
  2. change ant-core to return the PreparedUpload / retry state on finalize failure; or
  3. explicitly expose/document this as non-retryable-after-payment and provide a follow-up blocker for the core API before depending on it in mobile flows.

The third option is less satisfying, naturally — “pay once, maybe upload once” is not the sort of UX anyone frames in a launch deck unless they enjoy incident reviews.

No other new blockers found in this pass.

The prior comment claimed a network store failure after payment was safe
to recover by re-preparing (same payment reused). That is wrong: ant-core
consumes the PreparedUpload/proofs by value and quote hashes are
timestamp+signature-derived, so a re-prepare yields different quote hashes
that won't match the already-paid tx map — the paid attempt is stranded.

State the honest contract on `finalize_upload` (bad input is retryable;
a post-payment storage failure is currently NOT retryable) and reference
the follow-ups that add a proper retry-state API:
  - WithAutonomi/ant-client#140 (ant-core: return retry state on failure)
  - #201  (this surface: retain session + rev bump)

No behaviour change — docs/comments only.

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

Copy link
Copy Markdown
Collaborator Author

You're right, and thanks for the precise diagnosis — the timestamp+signature in PaymentQuote::hash() is the crux, so re-prepare genuinely can't reuse the payment. My "safe to re-prepare" comment was wrong.

Confirmed the ownership model in ant-core: finalize_upload_with_progress (file.rs:1735) takes PreparedUpload by value, builds the proofs via finalize_batch_payment, and on a store failure returns Error::PartialUpload{…} without handing the prepared state or proofs back (file.rs:1759-1778). PreparedUpload is #[non_exhaustive] and not Clone, so there's no FFI-only way to retain it — the real fix has to live in ant-core. Options 1 and 2 are both core changes; option 3 is what's actionable in this PR.

Going with option 3 (honest contract) here + a tracked two-repo fix, since a proper retry-state API is out of scope for this PR:

So the PR no longer overpromises, and "pay once, maybe upload once" is now a documented, tracked limitation rather than a silent one. Happy to prioritise #140 next if you'd rather the real fix land before this merges — your call on whether the honest-contract version is mergeable in the meantime.

@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 re-review after 63538ef

The new comment and commit address my remaining objection in the form we agreed was acceptable for this PR: it no longer claims post-payment network/storage failure is retryable, and it tracks the real core/API fix separately.

Verified at head 63538ef74ad8b75735f32d5963d56e3bfc51eaae:

  • cargo build -p ant-ffi
  • cargo test -p ant-ffi --no-fail-fast ✅ (still 0 tests)
  • cargo clippy -p ant-ffi --all-targets -- -D warnings
  • cargo fmt -p ant-ffi --check
  • Swift/Kotlin UniFFI generation from the fresh release dylib ✅; generated bindings include cancelUpload and the updated failure-contract docs.
  • gh pr checks 199 still reports no checks on this branch.
  • Follow-up issues exist and match the limitation: WithAutonomi/ant-client#140 for the core retry-state API, #201 for retaining/retrying from ant-ffi once core supports it.

Approved with one explicit caveat: this is mergeable as an honest-contract FFI surface, not as production-ready retry-safe WalletConnect upload UX. Before the mobile external-signer flow depends on this in production, #140/#201 need to land so a post-payment storage failure can be retried without stranding the paid attempt.

@Nic-dorman
Nic-dorman merged commit a6a2b8b into main Jul 6, 2026
Nic-dorman pushed a commit that referenced this pull request Jul 6, 2026
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>
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