feat: add Arbitrum USDT wallet with USDT fees - #157
ben-kaufman wants to merge 9 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 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".
| if let Ok(call) = Erc20::transferCall::abi_decode(&data) { | ||
| if payment.is_some() { | ||
| return None; | ||
| } | ||
| payment = Some((call.recipient, token_amount(call.amount).ok()?)); |
There was a problem hiding this comment.
Reject zero-value and self-transfer restored payments
When restoring a supported operation created by another wallet, decode_payment accepts any decoded transfer, including a zero amount or a recipient equal to the wallet address. The raw-log path explicitly discards those transfers, and locally quoted payments prohibit them, but this calldata enrichment path reintroduces them as confirmed outgoing activity, so restoring the same seed can display a spurious payment (and collapse its other raw debits into that entry). Apply the same zero/self-transfer checks before returning the decoded payment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ccdd478 and propagated to #158 and #159. Restore now excludes zero-value and self-directed ERC-20 transfers from decoded payments, preserving the raw fee debits and refunds.
I extended the existing raw-movement restoration test with both cases. It failed before the fix and now passes. All three USDT suites pass locally (48/54/60 tests), along with formatting, clippy and the iOS artifact checks. No signing, storage-schema or public API changes.
There was a problem hiding this comment.
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)
| self.address.to_checksum(None) | ||
| } | ||
|
|
||
| pub fn receive_uri(&self) -> String { |
There was a problem hiding this comment.
The new receive_uri() creates the payment request, but the tests parse a separately formatted string instead of testing this method. A broken QR value could go unnoticed. Could we check that parsing receive_uri() returns this wallet’s address and chain?
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.
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
cargo test --locked --lib modules::usdt,cargo fmt --checkandcargo clippy --locked --lib --tests.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.