Skip to content

feat: add Arbitrum USDT wallet with USDT fees - #157

Merged
ovitrif merged 13 commits into
masterfrom
feat/usdt-arbitrum
Sep 29, 2026
Merged

ovitrif merged 13 commits into
masterfrom
feat/usdt-arbitrum

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Adds USDT0 support on Arbitrum One to Bitkit Core, allowing users to receive and send USDT and pay transaction fees in USDT without holding ETH.

  • Derives an EIP-7702 account from the wallet's existing mnemonic and optional passphrase.
  • Provides balance, receive-address/payment-request, fee-quote, signing and activity APIs through UniFFI for the native apps.
  • Quotes a maximum USDT fee and validates the approved payment before signing. Signed operations are saved before submission so an interrupted or uncertain response does not lose the payment.
  • Recovers pending payments after restart and reconstructs incoming and outgoing activity from chain history after seed restoration. Activity includes payment status and fees where they can be attributed.

This PR covers direct Arbitrum payments. Native UI and the backend that protects provider credentials are maintained separately. Outbound bridging is added in #158; inbound deposits from other networks are added in #159.

QA Notes

  • Run cargo test --locked --lib modules::usdt, cargo fmt --check and cargo clippy --locked --lib --tests.
  • Coverage includes address/signature vectors, payment-request parsing, fee limits, uncertain submission, nonce recovery and restored history. An opt-in Arbitrum fork test exercises deployed contracts and USDT fee collection with zero account ETH.
  • Native-app mainnet testing has exercised send/receive, USDT-paid fees, and restart/seed recovery. Deployment and physical-device release validation remain separate requirements.
  • Chain history and payment outcomes depend on the configured RPC provider. A receipt at the current Arbitrum tip establishes L2 execution, not Ethereum finality.
  • Release packaging targets unpublished v0.6.0. Before remote package consumption, build the selected merged source, record the final SwiftPM checksum in a release-preparation commit, and tag/publish that exact iOS archive and matching Android package. Use matching local artifacts for branch testing; later stack layers must use a new version if an earlier layer has already been released.

@ben-kaufman ben-kaufman changed the title feat: add USDT0 wallet support on Arbitrum feat: add Arbitrum USDT wallet with USDT fees Sep 24, 2026
@ben-kaufman
ben-kaufman marked this pull request as ready for review September 25, 2026 12:45
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T13:16:03.158749Z 1f805f1 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f805f1091

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/modules/usdt/history.rs

@ovi-reviewer ovi-reviewer Bot 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.

Advice: ✅ Approve


Review: diff 33 files.

Findings:
1 inline (non-blocking)

Audit:
Audited - no findings.

Coverage:
QA: the tests under "QA Notes" need a person; no approval follows from this review


Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

Comment thread src/modules/usdt/wallet.rs
Comment thread src/modules/usdt/wallet.rs Outdated
Comment thread src/modules/usdt/wallet.rs
Comment thread src/modules/usdt/history.rs
Comment thread src/modules/usdt/payment_request.rs Outdated

@ovi-reviewer ovi-reviewer Bot 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.

Advice: ✅ Approve


Reaudit: diff 5 files.
No new findings; the rest is in the review.


Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner) · wrong <why> (owner)

@coreyphillips

Copy link
Copy Markdown
Collaborator

Two independent reviews.

needs changing before merge

  • Reorged transfers remain confirmed permanently (src/modules/usdt/store.rs:229). A canonical block change removes cached receipt metadata but leaves the activity row intact, so a transaction that disappears in a reorganization remains Confirmed permanently. I reproduced this with the committed schema: changing the saved block hash removed the receipt row but retained the confirmed transfer. A later scan with no replacement logs never updates or deletes it, and locally settled sends have already discarded the signed plan needed for recovery.

worth doing, does not block

  • check_recent_execution reports lock contention as a network failure (src/modules/usdt/wallet.rs:227). check_recent_execution returns NetworkUnavailable when it is only waiting for another wallet operation to finish. The 5 second RECENT_EXECUTION_BUDGET timeout in src/modules/usdt/wallet.rs:262 wraps the whole check future, and that future starts with self.operation.lock().await (line 227). If sync_history (20 second soft budget, receipts can overrun it) or refresh_transfers holds the lock, every poll during that window fails and the user sees "The network could not be reached". The README tells callers to defer history catch-up during sends, so this only shows up if a caller ignores that. Found by reading the code; I did not reproduce it. Two possible fixes: take the lock before starting the timeout, or map a lock-wait timeout to a result that leaves the payment pending instead of raising a network error.

@ben-kaufman

Copy link
Copy Markdown
Collaborator Author

Thanks, both points are valid. Fixed in 5bd4722.

  • Settled activity now retains its canonical block provenance, and locally signed operations remain available throughout the existing 4096-block history revisit window. History rechecks those blocks even when fresh log results are empty. A proven block-hash change atomically removes orphaned chain-discovered activity or reopens a local payment as Pending, preserving its ID and identical signed operation. Multiple reopened payments recover in nonce order; new sends remain blocked until that recovery is complete. Empty/delayed logs alone never trigger rollback.
  • check_recent_execution now returns the stored activity immediately when another wallet operation owns the lock. The five-second request budget no longer turns local contention into NetworkUnavailable.

Added general coverage for disappearance/re-inclusion, restart and identical rebroadcast, duplicate prevention, multiple reopened payments, and busy polling. The existing resume test also verifies that the revised history walk does not repeat completed block work. Reorg handling remains bounded by the documented revisit window; this does not claim Ethereum finality or arbitrary-depth rollback.

The fixes are also propagated through #158 and #159, including clearing orphaned bridge GUIDs and restoring the destination amount from the signed operation. Validation: 55/55 active USDT tests on #157, 61/61 on #158, and 67/67 on #159; formatting, Clippy (existing unrelated warnings only), generated Swift bindings, and an independent correctness/code-quality review. Live-contract tests remain opt-in; the reorg scenarios use mock RPC data.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovi-reviewer ovi-reviewer Bot 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.

Advice: ✅ Approve

Reaudit: diff 1 file.
No new findings; the rest is in the review.

QA:
The tests under "QA Notes" need a person; no approval follows from this review.


Reviewed by gpt-6-sol-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest (author)

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@ovitrif
ovitrif merged commit 950c4ad into master Sep 29, 2026
5 checks passed
@ovitrif
ovitrif deleted the feat/usdt-arbitrum branch September 29, 2026 23:56
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.

3 participants