Repository navigation
feat: compare USDT0 and Orchestra outbound routes - #167
Conversation
53c6b23 to
c6c987d
Compare
There was a problem hiding this comment.
Verdict: ✅ Approve
Review: diff 14 files.
Findings:
2 inline (1 MEDIUM, 1 LOW)
QA:
No tests ran; the PR description asks for automated cargo checks and leaves funded Orchestra delivery and refund acceptance outstanding.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
@ben-kaufman conflicts and stale unresolved threads |
c6c987d to
45ec0e7
Compare
|
@ovitrif Rebased onto current master and pushed 45ec0e7. GitHub now reports no merge conflicts. Both test gaps are covered and the threads are resolved: destination discovery, and funding-reorg recovery across restart for delivered/refunded Orchestra payments. Validation: 77 USDT tests passed (one optional live-fork test skipped), formatting passed, regenerated Swift bindings typecheck, and Clippy has no new warnings compared with master. CI is running. No live transfers were made for this update. |
There was a problem hiding this comment.
Verdict: ✅ Approve
Reaudit: diff 3 files.
No new findings; the rest is in the review.
QA:
No test ran because the description lists automated cargo checks, asks for no manual test or journey, and leaves funded Orchestra live acceptance outstanding.
Replies:
ben-kaufman: @ovitrif Rebased onto current master and pushed 45ec0e7. GitHub now reports no merge conflicts. Both test gaps are covered and the threads are resolved:… (comment)
I checked the added test assertions against both prior findings; both are addressed in 45ec0e7.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
Two independent reviews, nothing blocking a merge. worth doing, does not block
nits
|
|
Thanks, addressed in 7192be6:
The broader refund-provenance concern remains a trust boundary. The current gateway contract does not pin a refund sender, so I have not assumed refunds must come from the funding address. The gateway validates the provider's order against the saved quote and source payment, but Core cannot independently prove that an otherwise valid incoming transfer belongs to that order. This is now explicit in the README. The duplicate-receipt check conservatively allows one payment per refund transaction, including for batched receipts. Validation: 78 USDT tests passed, one optional live-fork test skipped, formatting passed, both Swift constructor forms typecheck, and all Clippy diagnostics match the previous report in unchanged files. No live transfers were made. |
There was a problem hiding this comment.
Verdict: ✅ Approve
Reaudit: diff 6 files.
No new findings; the rest is in the review.
QA:
No test ran because the QA Notes list only automated cargo checks, ask for no manual test, and leave funded Orchestra live acceptance outstanding.
Replies:
coreyphillips: Two independent reviews, nothing blocking a merge. worth doing, does not block - Refund proof accepts any equal-amount USDT transfer into the wallet… (comment)
I verified the receipt credit and duplicate claim checks, delivery amount bound, shared recipient exclusions, Swift constructor default and unchanged polling fast path. Refund order attribution remains the gateway trust boundary, now stated explicitly in the README.
ben-kaufman: Thanks, addressed in 7192be6: - Refunds require a matching USDT transfer and at least that amount in net credit, so self-transfers and round trips cannot… (comment)
I verified that refund claims persist through restart and pruning, and release on a refund reorg. Your explanation and the README clarify that Core proves receipt of funds while trusting the gateway for order attribution.
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
Description
Adds Orchestra as an alternative for outbound USDT payments from Arbitrum One. Existing USDT0 routes remain available. Where both providers support a destination, Core compares expected received USDT against the maximum total debit, including source fees, and chooses the best usable quote. Ties prefer USDT0.
Orchestra adds Base, BNB Smart Chain, Solana and Tron, and competes on Ethereum, Polygon and Plasma. Stable continues through USDT0. Destinations are enabled explicitly by the gateway; all source funding and fees remain in USDT. Orchestra delivery is an estimate: route costs are deducted from the principal and source network fees are additional.
The chosen provider, funding address and recovery ticket are saved with the signed operation before submission. Recovery follows that same payment, including successor orders and verified Arbitrum refunds; it never starts another provider payment after ambiguous submission. Funding and refund receipts are reconciled independently through the existing reorg window.
The gateway keeps provider credentials server-side and validates the quote's pinned route and exact USDT funding transaction. This PR exposes destination discovery, network-specific address validation and bridge details through matching Swift bindings. No migration or application routing framework is added.
QA Notes
Reviewer checks:
cargo fmt --check cargo test --locked --lib modules::usdt cargo clippy --locked --lib --testscargo fmt --all --checkpasses. Swift bindings were regenerated from the rebased source and pass typechecking.cargo test --locked --lib modules::usdt: 78 passed, one optional live-fork test skipped.cargo clippy --locked --lib --tests: no USDT diagnostics and no new warnings compared withmaster; existing unrelated repository warnings remain.Requires the companion
synonymdev/bitkit-usdt-serviceoutbound endpoint and its durable ticket-signing secret. Funded Orchestra outbound delivery/refund acceptance and deployment remain outstanding; this PR does not claim those live tests passed.