fix: restore swap history order id and surface required recipient entry (OK-59978, OK-58326) - #12850
fix: restore swap history order id and surface required recipient entry (OK-59978, OK-58326)#12850haicongliao wants to merge 16 commits into
Conversation
|
@codex review |
|
@codex security review |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37f9e3dc63
ℹ️ 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".
Review 结论需要修改后复审,7.4/10,当前不建议合入。 已按当前 head 合入前需要处理
历史问题与契约核对
评分与验证边界
本轮核对了 Jira 描述/附件、关联 Slack 讨论、服务端字段来源、当前 diff、review threads 与远端 CI。截图能证明 recipient entry 在 iOS/desktop 出现,但元素存在本身不算完整通过;修复后至少需要验证:作用域切换不残留、外部 recipient 可重复编辑、CoW pending order 可打开正确 explorer,以及所有 |
|
Review 基于 1. [P2] 收款方必填状态跨作用域残留 — 已按 swap type + source account + from/to token network & contract 组成 scope key,作用域变化即重置为 2. [P2] 中等长度 Order ID 缩写重复字符 — 3. [P2] 原生 Yoga 修复施加到 shared component — 绝对定位测量改为仅 关于"pure-function 测试覆盖不到" — 这一条成立,本轮已补。把 scope key 构造与 ref 保持一起抽成 这组测试确实抓到了纯函数测试覆盖不到的缺陷:hook 的 ref 初始值直接取了当轮 相关测试 68 passed。 🤖 Addressed by Claude Code |
Head branch was pushed to by a user without write access
d7516e6 to
f4e5a7f
Compare
|
已 rebase 到最新 唯一冲突点值得说明: #12862 独立修复了同一个原生测量问题,做法与本 PR 早期提交一致——对测量节点无条件加绝对定位。本 PR 在 review 反馈(P1 thread)后已改为仅
这意味着 diff 中会看到本 PR 调整了 #12862 刚合入的这几行,属于按 review 要求收窄平台边界,不是回退其修复。JSDoc 措辞也同步对齐为「native 才脱离文档流」。 其余 upstream 改动无重叠:#12862 触及的 相关测试 68 passed。 🤖 Addressed by Claude Code |
a5d35b5 to
46a89f2
Compare
…l concat(OK-58326 OK-59978)
|
Both issues addressed in P1 — Recipient scope adopting stale quotes: confirmed and fixed. Verified the premise: P2 — 79 focused tests pass; lint/tsc green in an isolated worktree install. 🤖 Addressed by Claude Code |
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 89d052825c98.
- P2 · Pair matching still accepts stale quote events
|
P2 (stale quote events) addressed in 6bfa6f6. Confirmed the premise: pair equality cannot distinguish a same-pair account switch, where the previous account's quote stays selected and its event stays registered as current until the new request is written. Adoption now requires an active-request proof computed from the same primitives the selection layer uses — isSwapQuoteFromCurrentEvent (event-id membership) AND isSwapQuoteRequestForCurrentInput (the request lock matching the current account/pair/amount, which rejects the pre-request transition window that event membership alone cannot). The pair-equality check stays as defense-in-depth. Added the requested hook test: a same-pair previous quote retained across an account switch is not adopted, and the verdict re-establishes once the new account's own round settles. 69 focused tests pass; lint/tsc green. 🤖 Addressed by Claude Code |
originalix
left a comment
There was a problem hiding this comment.
Automated code review found blocking issues
Reviewed commit 6bfa6f6c313a.
- P2 · Request-starting state still proves the retained quote
…verdict(OK-58326)
|
Request-starting interval addressed in fb0c993. Verified the exact sequence in quoteAction: it resets swapQuoteEventTotalCountAtom (clearing the event id) and writes the new lock with actionLock: true before the async runQuoteEvent flips any loading flag, while the previous quote stays selected — so the no-event-id fallback plus the freshly matching lock did prove a stale quote in that window. The proof is now a single composed predicate in the quote-progress layer, isSwapQuoteProvenForCurrentRequest, which treats actionLock-with-no-event-id as unproven before checking event membership and the lock match. Unit tests cover the request-starting interval with a retained same-pair quote (the reported hole), previous-event rejection after the new event reports, streamed current-event adoption while still locked, and lock mismatch. 96 focused tests pass; lint/tsc green. 🤖 Addressed by Claude Code |
|
@cursoragent review |
OK-59978 OK-58326
OK-59978 — Swap history order id
orderSupportUrl(e.g. CoW), so providers without a third-party order explorer stay hidden as OK-57251 intended.ctx(the CoW order uid) instead of the internal service order id, so the explorer link actually resolves for Stock orders — previously the link carried an internal UUID andexplorer.cow.fireturned "No results found".InfoItemgained an optionalcopyContent).OK-58326 — Swap recipient entry
SwapSmoothRevealnever expanding on native: a normal-flow child of the height-0 animated wrapper measures 0 under Yoga, so the measured height stayed 0 forever and the row was permanently hidden on iOS/Android. Content is now measured with absolute positioning. This also unhides the stock trade alert and the cost-savings reveal on native.Verification
swapHistoryIdentity,swapHistoryTransactionIds,useSwapAccount.utils), including new cases for the provider-order-id fallback and the recipient entry conditions.订单编号 0x0d99606c…6a85790cunder渠道商 CoW Swap).Note on CI type check
yarn tsc:stagedreports one error inpackages/kit/src/views/Market/MarketHomeV2/layouts/MobileLayout.native.tsx:873(tabPressAnimationEnabledis not declared on the tab container props). This is pre-existing onx— introduced by #12842 — and unrelated to the files in this PR. Reproduced on a cleanxcheckout with only a whitespace change to trigger the check.