Repository navigation
feat: add USDT submission deadlines, MAX quotes and senders - #170
Conversation
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Adds send_with_deadline, which stores an optional RFC 3339 submission cutoff with the signed plan and refuses eth_sendUserOperation once it has passed, on the first send and on recovery rebroadcasts. Reviewed 98bf9c6, full tier.
What I checked, and 6 candidates I ruled out
Read in full: the diff in wallet.rs, rpc.rs, backup.rs, transaction.rs, errors.rs, tests/backup.rs; send_with_deadline, recover_pending, broadcast and Rpc::call_with_deadline whole
Call sites traced: broadcast (first send and recover_pending, its only two callers), call_with_deadline (only eth_sendUserOperation passes a deadline), merge_plan (existing-row and seed-discovered restore paths)
Ran here: cargo test --lib modules::usdt: 91 passed, 0 failed, 1 ignored
CI: usdt-tests, SwiftPM manifest and the Cargo/Android manifest check green; iOS and Android bindgen still pending at review time
Ruled out
- Deadline passing during the rate-limit wait: the check runs after
reserve_request()and right beforesend(), andsubmission_deadline_is_checked_after_rate_limit_waitproves no connection is accepted. - Restore widening the cutoff:
merge_plankeepsmin(local, restored), and a restored plan with no deadline leaves the local one alone. With no local deadline, the existing-row path takes the restored one, and the discovered path behaves exactly like the oldCOALESCE(raw, ?4). - Payment left stuck after an expired cutoff: it stays
Pendingand blocks new sends, butrecover_pendingmarks itFailedonce the chain passesplan.expires_at, and the paymaster caps that at 900 s. - Wrong state on the
SubmissionDeadlineExpiredearly return: nothing reached the bundler, so the payment stayingPending(notFailed) only matters if an earlier submission is in flight, and the backup already carries the cutoff, so no other device can rebroadcast it. - Old stored plans:
#[serde(default, skip_serializing_if)]keeps rows and backups without the field byte-identical, so recovery for them is unchanged. - Test clock leaking into release builds:
Rpc::clockand its read are both#[cfg(test)], and no UniFFI export is cfg-gated.
Merge confidence: 4/5, the change is covered by six regressions, but the bindgen jobs had not finished when I reviewed.
|
MAX needs the matching gateway policy from bitkit-usdt-service ada7f6a. The stub gas estimate supplies a narrowly validated USDT balance override; final authorization and submission still use the real balance. An older gateway or fixture can reject that estimate as network unavailable. The service update is pushed, but its production deployment is still pending. Core's MAX tests and the gateway policy tests pass; a read-only quote against the matching local service also passed. This does not close the mobile MAX or end-to-end deadline-expiry checks in this report. Both mobile PRs are being updated to rc4 with a normal MAX button, so the numeric-input test hook will no longer be needed for that path. |
|
Thanks for the rc5 retest. This confirms native MAX affordability and expired-cutoff rejection on both platforms, with zero submissions. The 18.5-second Android MAX wait remains a responsiveness concern, but the report does not isolate its cause; the MAX quoting, paymaster and RPC logic is unchanged from rc4. I’m keeping that latency finding open rather than changing Core without timing evidence. Paykit deadline propagation and the latest mobile integrations remain separate checks, as the report notes. |
Read nonce, authorization and block metadata concurrently, and overlap token quotes with gas pricing. Preserve the shared RPC rate limit and all dependent paymaster, balance and submission checks.
|
QA for e2c768f: The run is still going; the final review follows when it ends. This is a progress note, nothing is approved or requested here. This comment is updated as tests finish and removed when the final result posts. 0 passed so far, 1 not tested. 🟠 Test 1: not tested because the supplied iOS simulator shut down before installation; missing: a working iOS wallet to complete both mobile checks; waits for round 2Not tested because the supplied iOS simulator shut down before installation |
ovitrif
left a comment
There was a problem hiding this comment.
Approving. ovi-reviewer had no findings up to e2c768f, and the rc5 retest showed MAX quotes are affordable and an expired cutoff is refused on both platforms, with nothing sent. The 18.5 s Android MAX wait stays open as you noted; rc6's latency work should help there.
Add the Core APIs needed by the mobile USDT payment screens: exact submission deadlines, a MAX quote that reserves fees, and the sender address for transfer details.
send_with_deadlinepersists the approved RFC 3339 cutoff before backup and enforces it immediately before submission or resubmission. Already-submitted payments and funded bridges continue reconciling.quote_max_transferreturns a normal, affordable quote. It estimates the fee with a restricted simulation balance, then obtains final paymaster terms against the real balance. The retry budget is bounded; actual fees may leave a small remainder.UsdtTransfer.senderretains the verified Arbitrum transfer source. Older records can omit it; a bridge contract is not identified as the original depositor.BridgeAmountTooSmallfor send quotes and receive estimates, without inventing a fixed network minimum.0.8.0-rc6for the mobile integrations: feat: add usdt wallet and paykit payments bitkit-ios#884 and feat: add usdt wallet and paykit payments bitkit-android#1432.MAX requires the companion service's restricted estimation-override support to be deployed. Overrides cannot be used for paymaster signing or transaction submission.
Validation
cargo fmt --checkpassed. Complete Clippy diagnostics inspected: warning-bearing files are unchanged from the base; none in changed USDT files.Acceptance