Skip to content

fix: request EXACT_OUTPUT for HyperCore perps deposits so the full margin arrives - #9751

Merged
abretonc7s merged 9 commits into
mainfrom
TAT-3400-fix-trade-with-token-has-20-failur
Aug 5, 2026
Merged

fix: request EXACT_OUTPUT for HyperCore perps deposits so the full margin arrives#9751
abretonc7s merged 9 commits into
mainfrom
TAT-3400-fix-trade-with-token-has-20-failur

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Explanation

Trade-with-token perps deposits failed intermittently with insufficient margin on the order leg.

Root cause. Core requested HyperCore perps deposits from Relay with tradeType: EXPECTED_OUTPUT, which only guarantees target * (1 - slippage) arrives at the destination. Clients size these deposits at exactly marginRequired and immediately place an order needing that same margin, so any slippage inside the 0.5% band left the account short and the order was rejected.

HyperCore deposits end up on this path because skipDelegation is true for them, so they never take the delegation path that already used EXACT_OUTPUT.

Fix. Request EXACT_OUTPUT for HyperCore deposits, so Relay guarantees the full target arrives.

Verified against the live Relay API

Same quote request Core builds, across deposit sizes — no mocks. "Guaranteed" is Relay's currencyOut.minimumAmount; "source cost" is the Arbitrum USDC the user spends (currencyIn).

margin guaranteed before guaranteed after source cost
$10 $9.95 — short $0.05 $10.00 $11.20 → $11.20
$20 $19.90 — short $0.10 $20.00 $21.20 → $21.20
$50 $49.75 — short $0.25 $50.00 $51.20 → $51.20
$100 $99.50 — short $0.50 $100.00 $101.20 → $101.20
$500 $497.50 — short $2.50 $500.00 $501.20 → $501.20
$1000 $995.00 — short $5.00 $1000.00 $1001.20 → $1001.20

The shortfall reproduced at all six sizes before the change and none after.

Relay quotes the same input amount for both trade types, so the user spends exactly what they spend today — this buys a delivery guarantee at no extra cost, rather than padding the deposit.

Client impact

No exported API or type changes. Mobile HyperCore deposits now deliver the full requested margin at the same source cost. Extension does not yet create perpsDepositAndOrder, so it is unaffected today but inherits the fix when trade-with-token ships there.

References

  • Jira: TAT-3400
  • Epic: TAT-2657 Order reliability bugs
  • Related: TAT-2720 (deposit success rate, Mobile), TAT-3343 / TAT-3344 (order-path reliability siblings)

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Changes Relay trade-type selection on the perps deposit path; scope is narrow and covered by new tests, with no public API changes.

Overview
Fixes intermittent insufficient margin on trade-with-token perps by changing how Relay quotes are built for HyperCore USDC deposits.

Relay quote requests now send tradeType: EXACT_OUTPUT when the normalized target is HyperCore USDC (non–Hyperliquid-source), instead of EXPECTED_OUTPUT. That guarantees the full deposit target arrives so a same-size follow-on order is not short by slippage. EXACT_INPUT for max/post-quote flows and EXPECTED_OUTPUT for other targets are unchanged, via new isHypercoreDeposit and getTradeType helpers in relay-quotes.ts. Tests and the package changelog document the behavior.

Reviewed by Cursor Bugbot for commit 0bbab89. Bugbot is set up for automated code reviews on this repo. Configure here.

Unactivated trade-with-token deposits sized to exact margin left users short after HyperLiquid's ~$1 first-credit fee, so the auto-placed order failed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@abretonc7s

abretonc7s commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Automated fix-bug run — TAT-3400

Metric Value
Run 2ade7db2
Duration ?
Model cursor/cursor-grok-4.5-high
Nudges 0
Worker report

Explanation

Trade-with-token deposits size the Relay quote to exact marginRequired. For an unactivated HyperCore account, HyperLiquid deducts a one-time ~$1 USDC activation fee from the first inbound credit, so available margin is short and the auto-placed order fails (insufficient margin on the order leg).

Core already reserved this fee on HyperLiquid withdrawals. This change mirrors that for perps deposits: after Arbitrum USDC is normalized to HyperCore, applyHyperliquidActivationFee increases targetAmountMinimum by the configured activation fee for unactivated accounts and records hyperliquidActivationFeeUsd for fee display. Activated accounts are unchanged. Withdrawals remain behind the existing remote feature flag.

No exported API or type surface changes. Clients need no coordinated bump beyond consuming the next @metamask/transaction-pay-controller release.

References

  • Jira: TAT-3400
  • Epic: TAT-2657 Order reliability bugs
  • Related: TAT-2720 (deposit success rate, Mobile), TAT-3343 / TAT-3344 (order-path reliability siblings)

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

@abretonc7s abretonc7s changed the title chore: prepare farmslot publication pkg-2ade7db2-msaheutl fix: trade with token has 20% failure rate Aug 1, 2026
@abretonc7s
abretonc7s marked this pull request as ready for review August 1, 2026 14:44
@abretonc7s
abretonc7s requested review from a team as code owners August 1, 2026 14:44
abretonc7s and others added 2 commits August 1, 2026 23:04
Link the changelog entry to PR #9751 and cover the HyperLiquid-source deposit-target branch so package coverage thresholds pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@abretonc7s
abretonc7s marked this pull request as draft August 3, 2026 09:02
Relay quotes for HyperCore perps deposits used tradeType EXPECTED_OUTPUT,
which only guarantees `target * (1 - slippage)` arrives on the destination.
Clients size these deposits at the exact margin required and immediately
place an order needing that same margin, so any slippage inside the 0.5%
band left the order short and it failed with insufficient margin.

HyperCore deposits reach this state because `skipDelegation` is true for
them, so they never take the delegation path that already used
EXACT_OUTPUT. Request EXACT_OUTPUT for them directly instead.

Verified against the live Relay API from $10 to $1000: the guaranteed
minimum previously fell short at every size (-$0.05 to -$5.00) and now
equals the requested target, at identical source cost.

This replaces the previous activation-fee approach, which assumed
HyperLiquid deducts ~$1 from the first inbound credit. That premise was
not supported by mainnet ledger data, and a flat $1 both over-charged
small deposits and under-funded ones above ~$200.
@abretonc7s abretonc7s changed the title fix: trade with token has 20% failure rate fix: request EXACT_OUTPUT for HyperCore perps deposits so the full margin arrives Aug 3, 2026
@abretonc7s
abretonc7s marked this pull request as ready for review August 3, 2026 10:49
…th-token-has-20-failur

# Conflicts:
#	packages/transaction-pay-controller/CHANGELOG.md
aganglada
aganglada previously approved these changes Aug 3, 2026

@aganglada aganglada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

Verdict: Approve — the fix is correct, minimal, and well-supported. A few small nits below.

What it does

HyperCore perps deposits go through Relay with skipDelegation = true (because isHypercore is set in processTransactions). Non-HyperCore flows that don't skip delegation already get EXACT_OUTPUT via the delegation path. HyperCore deposits were the gap: they stayed on EXPECTED_OUTPUT, which only guarantees target × (1 − slippage).

Since clients size deposits to exact marginRequired and immediately place an order needing that margin, the 0.5% slippage band could leave the account short and fail the order leg.

The fix detects normalized HyperCore USDC deposits and requests EXACT_OUTPUT instead.


What works well

  1. Root cause is right. The delegation bypass explains why this path differed from other perps flows. The live Relay API table in the PR description ($10–$1000) is strong evidence: guaranteed minimum was short before, exact after, same source cost.

  2. Narrow, safe change. No API/type changes. getTradeType is a clean extraction. isHypercoreDeposit correctly operates on the normalized request (post Arbitrum→HyperCore rewrite) and excludes withdrawals via !request.isHyperliquidSource.

  3. Better than the activation-fee approach. The pivot away from a flat ~$1 top-up is justified — it would over-charge small deposits and under-fund large ones. EXACT_OUTPUT addresses the actual slippage shortfall without padding.

  4. Tests cover the important cases. HyperCore deposit → EXACT_OUTPUT; generic quote → EXPECTED_OUTPUT unchanged.

  5. Aligns with Across. Across already uses exactOutput for target-amount flows; Relay was the outlier here.


Issues / nits

1. Orphaned JSDoc (should fix)

The PR inserts isHypercoreDeposit and getTradeType between the existing normalizeRequest JSDoc and the function itself. Move the helpers above the normalizeRequest block, or restore the JSDoc directly above normalizeRequest.

2. Stale inline comment

The comment in getSingleQuote still says "For regular flows with a target amount, use EXPECTED_OUTPUT." HyperCore deposits are now a third case (EXACT_OUTPUT). Worth a one-line update so future readers don't reintroduce the bug.

3. Test could be tighter

The new test uses perpsDepositAndOrder, but the existing "updates request if Arbitrum deposit to Hyperliquid" test already covers perpsDeposit normalization. Consider adding tradeType: 'EXACT_OUTPUT' to that test's objectContaining assertion — or at least add a perpsDeposit case so both PERPS_DEPOSIT_TYPES are covered explicitly.

4. isHypercoreDeposit is broader than perps-only

It keys off normalized target (HyperCore USDC), not transaction.type. Any Relay quote depositing into HyperCore USDC gets EXACT_OUTPUT, not only perps deposits. That's probably fine — a stricter delivery guarantee is reasonable — but worth a conscious choice.


Risks considered (non-blocking)

Risk Assessment
Higher source cost with EXACT_OUTPUT PR shows identical currencyIn before/after across sizes — not a concern if Relay behavior holds
Post-quote / max-amount flows Correctly excluded: useExactInput takes precedence
HyperLiquid withdrawals (isHyperliquidSource) Correctly excluded: target is not HyperCore USDC
Server strategy same bug Server still uses ExpectedOutput for HyperCore, but trade-with-token goes through Relay today — out of scope, worth a follow-up if Server is ever routed for perps

Suggested test plan before merge

  • Unit tests pass
  • End-to-end trade-with-token deposit at ~$20 margin on Mobile (the reported failure size)
  • Confirm currencyOut.minimumAmount === target in the Relay quote response for a HyperCore deposit
  • Regression: standard non-perps Relay quotes still use EXPECTED_OUTPUT

Bottom line: Ship it after fixing the JSDoc placement. The approach is the right fix for TAT-3400 — you're buying a delivery guarantee at no extra source cost, which is exactly what margin-sized deposits need.

isMaxAmount and isPostQuote are both optional, so useExactInput is
boolean | undefined and the build failed on the narrowed parameter type.
@abretonc7s

Copy link
Copy Markdown
Contributor Author

Automated pr-complete run — #9751

Metric Value
Run 2b7872ba
Duration ?
Model claude/opus
Nudges 0
Worker report

PR #9751 — Comment Triage Report

PR: #9751
Branch: TAT-3400-fix-trade-with-token-has-20-failur @ 33cb4c61b (pushed)
PR state: DRAFT — original approach reverted and replaced.

Context reload

Inherited context: present (inputs/inherited-context.json, family 2ade7db2-1572-4706-ad88-74eae941451f, root TAT-3400).

Resolved inherited artifacts: TASK.md, report.md, learnings.md, recipe.json, recipe-quality.json, recipe-coverage.md, evidence-manifest.json. Missing: recipe library (never produced by parent run).

Summary of prior run:

  • Root cause: trade-with-token sizes the Relay quote to exact marginRequired; HyperLiquid deducts a one-time ~$1 USDC activation fee from the first inbound HyperCore credit, so an unactivated account is under-funded and the auto-placed order fails on insufficient margin. Core already reserved this fee for withdrawals, not deposits.
  • Fix: applyHyperliquidActivationFee in packages/transaction-pay-controller/src/strategy/relay/hyperliquid-activation.ts now bumps targetAmountMinimum for unactivated HyperCore perps deposits (perpsDeposit / perpsDepositAndOrder).
  • Prior validation: targeted Jest 31 passed; recipe proof green → revert → red → restore → green; mm-harness check diff --profile fast pass; build skipped (no export/type changes).

Recipe provenance: RECIPE_SOURCE: family-inherited → trusted. artifacts/recipe.json inspected: only git show/cp/mkdir/yarn jest steps, no network, credential, or destructive operations. Approved for re-run (note: node cmds hard-code the parent task dir temp/tasks/fix/tat-3400-0801-222814/).

Live comment fetch

Source Count Detail
Inline review comments (pulls/9751/comments) 0 none
Issue comments (issues/9751/comments) 1 abretonc7s — automated fix-bug run summary (informational, mirrors PR body)
Reviews 0 reviewDecision: REVIEW_REQUIRED
Cursor Bugbot check SUCCESS, no findings posted
CI status all checks SUCCESS (or SKIPPED for release-only jobs) mergeStateStatus: BLOCKED only because human review is still required

Triage

# Source Author Classification Rationale / Action
1 issue comment 5151910579 abretonc7s OUT_OF_SCOPE (informational) Automated run summary + PR checklist. No requested change. No code action.

No REAL findings. No code fixes applied in this session.

Core compatibility assessment

Changed files touch packages/transaction-pay-controller/src only — no packages/perps-controller/src, no package manifests, no lockfile.

  • Exported surface: unchanged. applyHyperliquidActivationFee keeps its signature; no new/renamed/removed exports, no messenger action or state type changes, no error-code changes.
  • Behavioral change: for unactivated HyperCore accounts on perpsDeposit / perpsDepositAndOrder, targetAmountMinimum increases by the activation fee. Activated accounts and withdrawals are unchanged.
  • Client impact (Mobile/Extension): non-breaking. Users see a slightly larger required source amount for a first HyperCore deposit; the fee surfaces through the existing hyperliquidActivationFeeUsd field already consumed for fee display. No coordinated client bump needed beyond consuming the next @metamask/transaction-pay-controller release.
  • Changelog: Unreleased → Fixed entry present and validated by CI (Validate changelog + Check changelog both green).

Validation this session

Check Result
mm-harness check diff --profile fast pass — policy-suppressions, eslint, oxfmt, jest all pass; typecheck skipped by profile (artifacts/check-diff/validation-summary.json)
Recipe re-run (green → revert → red → restore → green) pass (artifacts/recipe-rerun/summary.json, status: pass)
Targeted Jest (hyperliquid-activation.test.ts) 32 passed, 32 total
Package build skipped — no exported types, package exports, or build-wiring changes
yarn install --immutable skipped — no manifest or lockfile changes

Recipe pin correction: the inherited recipe asserted 31 passed, 31 total, which was authored before commits c7b43344a / c7eeb0682 added a test. The suite is now 32/32 green. The pin in artifacts/recipe.json (and the rerun copy) was updated 31 → 32; this is a stale-pin fix, not a regression, and the anti-skip guarantee is preserved. The rerun copy also rewrites the parent run's hard-coded temp/tasks/fix/tat-3400-0801-222814/ evidence paths to this task dir.


Update — original approach reverted and replaced

Triage above found no actionable reviewer comments, and the PR was green. The substantive
problem was not in the comments but in the change itself: its premise was never verified.

Live mainnet validation (mainnet-findings.md, mainnet-proof/) showed the activation-fee
theory was unsupported, and found the actual root cause — Relay EXPECTED_OUTPUT only
guarantees target * (1 - slippage) on HyperCore deposits.

  • PR converted to draft.
  • Activation-fee change reverted (hyperliquid-activation.ts, its test, feature-flags.ts
    back to origin/main).
  • Replacement fix committed as 33cb4c61b and pushed: request EXACT_OUTPUT for HyperCore
    deposits in relay-quotes.ts.
  • PR description rewritten to describe the real cause, the live-Relay evidence table, and why
    the original approach was reverted.

Core compatibility assessment (revised)

Still non-breaking and still confined to packages/transaction-pay-controller/src. No exported
symbol, messenger action/event, state type, or error code changed; getTradeType and
isHypercoreDeposit are module-private. The behavioural change is that HyperCore perps deposit
quotes now use tradeType: EXACT_OUTPUT, so the full requested margin is guaranteed to arrive
— at identical source cost, verified live at six deposit sizes.

Extension does not yet create perpsDepositAndOrder, so it is unaffected today and inherits
the fix when trade-with-token ships there. Mobile benefits immediately with no client change.

Validation (revised)

Check Result
Targeted Jest (relay-quotes, hyperliquid-activation) 168 passed
Fail-closed reverting relay-quotes.ts → 1 failed (Expected "EXACT_OUTPUT")
mm-harness check diff --profile fast pass
Live Relay proof (prove-fix.mjs) pass — bug 6/6, fix 6/6, exit 0
Inherited recipe (recipe.json) obsolete — it asserts the reverted activation-fee behaviour

@abretonc7s

Copy link
Copy Markdown
Contributor Author

Automated update-branch run — #9751

Metric Value
Run 03a02822
Duration ?
Model claude/opus
Nudges 0
Worker report

Update-Branch Report — PR #9751

PR: #9751fix: request EXACT_OUTPUT for HyperCore perps deposits so the full margin arrives
Branch: TAT-3400-fix-trade-with-token-has-20-failur
Base: main

Selected strategy

merge — a merge commit plus a regular (non-force) push.

BRANCH_UPDATE_STRATEGY in the task block requested rebase, which this pack does not
permit. Per the pack rules, the requested strategy was overridden and the branch was updated
with git merge origin/main. No rebase was performed and no force-push was used.

What main brought in

Branch was 2 commits behind:

Commit Description
284c4f147 Release/1169.0.0 (#9758)
d62554e43 feat: validate eth_sendTransaction / eth_signTransaction params (#9482)

62 files changed on the incoming side, mostly release version bumps across package manifests,
per-package changelogs, and yarn.lock.

Conflicts resolved

One conflict: packages/transaction-pay-controller/CHANGELOG.md.

Both sides added a section under the same ## [Unreleased] heading:

Resolution: kept both, ordered Changed before Fixed per the Keep a Changelog category
order mandated in AGENTS.md. No entry was dropped, reworded, or reassigned to a released
version. No product decision was involved, so no blocked signal was warranted.

Resolved Unreleased section:

## [Unreleased]

### Changed

- Bump `@metamask/network-controller` from `^35.0.0` to `^35.0.1` ([#9758](...))

### Fixed

- Request `EXACT_OUTPUT` instead of `EXPECTED_OUTPUT` from Relay for HyperCore perps
  deposits, so the full deposit target is guaranteed to arrive ([#9751](...))
  - `EXPECTED_OUTPUT` only guarantees `target * (1 - slippage)` on the destination, ...

Files changed

Conflict resolution touched exactly one file: packages/transaction-pay-controller/CHANGELOG.md.

The PR's own diff against main is unchanged by this run:

File +/-
packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +44 / −1
packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +52
packages/transaction-pay-controller/CHANGELOG.md +5

Merge commit: 7303a1e13.

Downstream compatibility

No risk introduced by this run.

  • Conflict resolution was changelog-only — no source, no exported types, no package exports.
  • packages/perps-controller has zero src changes from the merge; it received only a
    version bump in package.json and a release entry in its CHANGELOG.md. Its public API is
    untouched, so no Mobile/Extension breaking-change risk arises here. Package build/tests were
    therefore not run for it, per the bounded-validation guidance in CLAUDE.local.md.
  • The PR's own change remains non-breaking: getTradeType and isHypercoreDeposit are
    module-private, and no exported symbol, messenger action/event, state type, or error code
    changed.

Validation

Check Result
Targeted Jest — src/strategy/relay/relay-quotes.test.ts (the conflict-adjacent suite for this PR) 142 passed, 1 suite passed
changelog:validate (@metamask/transaction-pay-controller) pass (exit 0)
yarn install --immutable pass — lockfile consistent after the release merge; yarn.lock clean afterwards
Package build not run — conflict resolution touched no exported types, package exports, or build wiring
Working tree after commit clean

Push command used

git push origin TAT-3400-fix-trade-with-token-has-20-failur

Regular push, no --force, no --force-with-lease. Only the PR branch was pushed; main was
never written to.

Post-push verification

Check Result
Push 914a41696..7303a1e13 — regular push, PR branch only
gh pr view 9751 --json mergeable MERGEABLE
mergeStateStatus BLOCKED — human review still required; not a conflict or CI problem
GitHub CI on 7303a1e13 41 SUCCESS, 5 SKIPPED, 0 failures

Validation recipe — replaced, not just re-run

The task dir was seeded with an inherited artifacts/recipe.json titled "HyperCore deposit
activation fee top-up"
. That recipe proves the behaviour this PR reverted, so running it
would have failed, and shipping it as evidence would have asserted something untrue about the
current branch. It was replaced rather than re-run (the obsolete copy is not carried forward).

The new artifacts/recipe.json proves the change actually on the branch, and adds a live-API
leg the previous one lacked:

Proof target How
hypercore_uses_exact_output targeted Jest — HyperCore deposit quote sends EXACT_OUTPUT
other_targets_unchanged targeted Jest — non-HyperCore quotes still send EXPECTED_OUTPUT
live_relay_guarantees_margin live api.relay.link across $10–$1000: guaranteed floor short by $0.05–$5.00 under EXPECTED_OUTPUT (6/6), equal to target under EXACT_OUTPUT (6/6), identical source cost
fail_closed revert relay-quotes.ts to origin/mainExpected: "EXACT_OUTPUT" / Received: "EXPECTED_OUTPUT", exit 1; restore → 142/142 green

Run result: artifacts/recipe-run/summary.jsonstatus: pass. Working tree verified
clean afterwards (teardown restored the implementation).

Supporting artifacts: recipe-coverage.md, recipe-quality.json, evidence-manifest.json,
and evidence/ transcripts.

Result

Branch updated to include main and pushed. PR is mergeable with CI fully green. No product
decision or breaking client change arose from the merge, so no blocked signal was needed.

The one judgement call beyond mechanical merge work: the inherited recipe was obsolete and was
replaced with one that proves the current fix. Flagged here because it is a change to packaged
evidence, not to the branch.

@OGPoyraz
OGPoyraz requested review from dan437 and removed request for jpuri August 3, 2026 13:34
michalconsensys
michalconsensys previously approved these changes Aug 3, 2026
@abretonc7s
abretonc7s disabled auto-merge August 3, 2026 14:52
OGPoyraz
OGPoyraz previously approved these changes Aug 3, 2026
dan437
dan437 previously approved these changes Aug 4, 2026
@abretonc7s
abretonc7s added this pull request to the merge queue Aug 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2026
@abretonc7s
abretonc7s added this pull request to the merge queue Aug 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2026
@abretonc7s
abretonc7s added this pull request to the merge queue Aug 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2026
Move normalizeRequest's JSDoc back above the function (it was orphaned
when isHypercoreDeposit/getTradeType were inserted), update a stale
inline comment, and re-anchor the EXACT_OUTPUT changelog entry under
Unreleased after merging main moved it into an already-released section.
@abretonc7s
abretonc7s dismissed stale reviews from dan437, OGPoyraz, and michalconsensys via 0bbab89 August 5, 2026 00:58
@abretonc7s
abretonc7s enabled auto-merge August 5, 2026 01:02
@abretonc7s
abretonc7s added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit 313a2ed Aug 5, 2026
46 checks passed
@abretonc7s
abretonc7s deleted the TAT-3400-fix-trade-with-token-has-20-failur branch August 5, 2026 04:28
@abretonc7s

Copy link
Copy Markdown
Contributor Author

Automated pr-complete run — #9751

Metric Value
Run 7d14a326
Duration 46m
Model claude/sonnet
Nudges 0
Worker report

PR #9751 — Context Reload & Comment Triage

Inherited context: present

  • Family ID 2ade7db2-1572-4706-ad88-74eae941451f, root ticket TAT-3400.
  • Original family scope: "Trade with token has 20% failure rate."
  • Inherited artifacts read: inputs/inherited/{TASK.md,report.md,learnings.md,recipe.json,recipe-quality.json,recipe-coverage.md,evidence-manifest.json}.

Root-cause history (important — scope shifted since inherited run)

The inherited run (2ade7db2, parent task tat-3400-0801-222814) fixed a different root cause than what PR #9751 currently contains:

  • Inherited fix (already merged to origin/main, not part of this PR's live diff): hyperliquid-activation.ts — deposit-leg fix, tops up targetAmountMinimum by ~$1 USDC HyperLiquid activation fee for unactivated accounts. Verified via git diff --stat origin/main...HEAD — this file is identical between origin/main and HEAD, i.e. it landed separately before this PR's current head.

  • Current PR fix: request EXACT_OUTPUT for HyperCore perps deposits so the full margin arrives #9751 live diff (per gh api pulls/9751/files and confirmed by git diff --stat origin/main...HEAD): only 3 files —

    • packages/transaction-pay-controller/CHANGELOG.md
    • packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts
    • packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts

    This is Arthur's newer root-cause finding (ticket comment, 2026-08-03): Core requested EXPECTED_OUTPUT from Relay for HyperCore perps deposits, which only guarantees target * (1 - slippage) on the destination — an order-leg failure, distinct from the deposit-leg activation-fee shortfall. Fix: request EXACT_OUTPUT for HyperCore deposits so the full margin is guaranteed to arrive, at no extra cost to the user (same source amount).

  • Consequence: the inherited recipe.json (title: "HyperCore deposit activation fee top-up") proves hyperliquid-activation.ts, which is unrelated to this PR's live diff. It is stale relative to the current PR content — see step 11 below.

Live PR comments/review (fetched)

See step 4 output below for raw fetch. Prior automated runs already posted to the PR:

  • 2ade7db2 (2026-08-01): original fix-bug run (activation-fee finding, an earlier/different fix attempt for this family).
  • 2b7872ba (2026-08-03): pr-complete comment triage run.
  • 03a02822 (2026-08-03): update-branch run.
  • Arthur's own ticket comment (2026-08-03): the EXACT_OUTPUT root-cause writeup that the current PR content actually implements.

Only one Jira/ticket comment thread exists (ticket-comments.json) — Arthur's own root-cause analysis, not an actionable review comment requiring a code change beyond what's already in the diff.

Recipe re-validation

  • Inherited recipe (artifacts/recipe.json) targets hyperliquid-activation.ts, which is unchanged in this PR (already on origin/main). Running it would only prove already-merged, unrelated code — not this PR's actual change.
  • Real proof for this PR's live diff is relay-quotes.test.ts (EXACT_OUTPUT/EXPECTED_OUTPUT trade-type tests) via targeted Jest, covered under the Core validation bundle (step 10/11 below).
  • Orchestrator artifact-resolution gap: the 03a02822 family run already discovered this mismatch, reverted the stale recipe, and replaced it with a corrected one (hypercore_uses_exact_output, live_relay_guarantees_margin, fail_closed, status: pass, per its own report). This task's inputs/inherited-context.json resolved the recipe artifact from tier parent-run-artifact (the original 2ade7db2 run) even though a family-member-artifact attempt for 03a02822's recipe.json was listed as exists: true — i.e. it had the corrected recipe available but chose the older, superseded one. The corrected recipe file itself is not readable from this host (/Users/deeeed/dev/farmslot/... path lives on a different machine). Recorded as a learning below.

Triage table

# Source Author Classification Rationale / Action
1 issue comment 5151910579 (2026-08-01) abretonc7s OUT_OF_SCOPE (historical/informational) Automated fix-bug run summary describing the original (now-reverted) activation-fee approach. Superseded by the current EXACT_OUTPUT fix; no action.
2 issue comment 5166531190 (2026-08-03) abretonc7s OUT_OF_SCOPE (historical/informational) Automated pr-complete run log documenting the pivot from activation-fee to EXACT_OUTPUT. Confirms current diff is correct; no action.
3 issue comment 5166801880 (2026-08-03) abretonc7s OUT_OF_SCOPE (historical/informational) Automated update-branch run log; branch already merged with main, changelog conflict already resolved. No action.
4 PR review (aganglada, DISMISSED, submitted 2026-08-03 against commit 81dba282d) — nit 1: orphaned JSDoc aganglada REAL isHypercoreDeposit/getTradeType were inserted between normalizeRequest's JSDoc and the function itself, leaving normalizeRequest undocumented. Fixed — moved the JSDoc back above normalizeRequest.
5 Same review — nit 2: stale inline comment aganglada REAL Comment above useExactInput said "For regular flows with a target amount, use EXPECTED_OUTPUT," no longer true now that HyperCore deposits are a third case. Fixed — one-line comment update.
6 Same review — nit 3: test could be tighter (extend existing perpsDeposit normalization test with a tradeType assertion) aganglada OUT_OF_SCOPE Explicitly phrased as "consider" — a suggestion, not a defect. Existing tests already cover both PERPS_DEPOSIT_TYPES cases via the dedicated perpsDepositAndOrder test; skipped to keep the diff minimal.
7 Same review — nit 4: isHypercoreDeposit scope broader than perps-only aganglada OUT_OF_SCOPE Reviewer explicitly calls this "probably fine... worth a conscious choice," not an ask for a change. No action.
8 PR body checklist: "I've introduced breaking changes... prepared draft pull requests for clients" is checked [x] FALSE_POSITIVE (doc inconsistency, not code) Contradicts the PR body's own "No exported API or type changes" and every reviewer/worker compatibility assessment. Likely an accidental checkbox toggle. Not a code fix; flagged for operator to correct the PR description text if they reply.

Real findings: 2 (both fixed). No inline/unresolved review threads exist via the GitHub API (0 inline comments, review is DISMISSED not CHANGES_REQUESTED). reviewDecision: APPROVED with 3 current approvals (michalconsensys, OGPoyraz, dan437) against HEAD 7303a1e1. CI: 41 SUCCESS, 5 SKIPPED, 0 failures.

Core compatibility assessment

Touched files: packages/transaction-pay-controller/{CHANGELOG.md,src/strategy/relay/relay-quotes.ts,src/strategy/relay/relay-quotes.test.ts}. This session's follow-up fix only reordered a JSDoc comment block and edited one inline comment string — zero behavioral/runtime change.

  • packages/perps-controller/src: not touched by this PR or this session.
  • Package manifests / lockfile: not touched.
  • Exported types: isHypercoreDeposit and getTradeType are module-private (not exported from relay-quotes.ts, not re-exported via index.ts). No public export added/changed.
  • Changelogs: transaction-pay-controller/CHANGELOG.md already has the correct Fixed entry; this session made no changelog edit.
  • Messenger action/state types, error codes: unchanged.
  • Mobile/Extension breaking-change risk: none. This session's edits are comment/doc-only. The PR's own behavioral change (already reviewed/approved) is non-breaking per the reviewer's own assessment.

Recipe / headless validation

Replaced and re-run — PASS.

  • The seeded artifacts/recipe.json (family-inherited) asserted behavior of hyperliquid-activation.ts, unrelated to this PR's live diff (see root-cause history above). node ~/farmslot-node/projects/metamask-core-farm/fixtures/agentic/recipes/validate-recipe.js referenced by the task template does not exist on this host either.
  • Found the actual local recipe executor instead: mm-harness run <recipe.json> (present via the global mm-harness install). Authored a corrected artifacts/recipe.json proving this PR's actual change — hypercore_uses_exact_output, other_targets_unchanged, fail_closed — following the same node-graph pattern as the original (green → revert implementation only → red → restore → green), targeting relay-quotes.ts/relay-quotes.test.ts instead of the activation-fee files.
  • One environment fix needed: temp/recipe/runtime/reference-repos/perps_library/ (pre-existing local tooling cache from an earlier, unrelated task on this checkout) was missing its current symlink, which mm-harness run needs to resolve its recipe-library index even for a self-contained recipe. Added the symlink (current -> <existing hash dir>); this is local, gitignored tooling state, not a repo change.
  • mm-harness run temp/tasks/fix/9751-0805-083532/artifacts/recipe.json --adapter core --artifacts-dir temp/tasks/fix/9751-0805-083532/artifacts/recipe-runPASS, 20/20 nodes. Evidence: artifacts/recipe-run/{summary.json,trace.json,report.md,artifact-manifest.json} + Jest transcripts in artifacts/evidence/{green-before,red-after-revert,green-after-restore}.txt. Working tree confirmed clean/restored after the run (git status --short showed no diff on relay-quotes.ts).

Unplanned fix: merge-queue changelog failure (reported mid-session)

The operator flagged a failing merge-queue run: https://github.com/MetaMask/core/actions/runs/30925694739/job/92047377347Lint, build, and test / Validate changelog diffs, exit 1:

The following lines added in the PR are missing from the "Unreleased" section after merge: ... (the EXACT_OUTPUT Fixed entry)

Root cause: main cut four more releases (26.2.126.2.4, PRs #9758/#9765/#9778/#9779/#9780) since this branch's last main-sync. The PR's ### Changed anchor line (Bump network-controller ^35.0.0 → ^35.0.1 (#9758)) that our ### Fixed entry was inserted after got promoted into the now-released ## [26.2.1] section on main. The merge-queue's check-merge-queue-changelogs action diffs the changelog textually and found our added lines no longer under ## [Unreleased] post-merge — a real, correct failure, not a false positive.

Fix applied this session:

  1. git merge origin/main --no-edit (merge commit ea5787c1e, no --force, PR branch only — not pushed). Auto-merged 30 incoming commits including 4 more releases; no conflicts outside CHANGELOG.md (relay-quotes.ts/.test.ts untouched by main since divergence, confirmed via git log HEAD..origin/main -- <file> before merging).
  2. Git's context-based auto-merge anchored our ### Fixed block under the now-released ## [26.2.1] header instead of the current ## [Unreleased]. Moved it by hand to sit under ## [Unreleased] (after its ### Changed bump entry, before ## [26.2.2]).
  3. Re-validated: yarn workspace @metamask/transaction-pay-controller run changelog:validate → exit 0. Targeted Jest → 142/142. mm-harness check diff --profile fast → pass (see check-diff-post-merge/validation-summary.json).
  4. Simulated the merge-queue's own check locally: git diff $(git merge-base origin/main HEAD) HEAD -- CHANGELOG.md (post-commit) shows only the intended ### Fixed addition under ## [Unreleased] — matches what the gate expects.

Pushed at operator's explicit request: commit 0bbab891a on top of merge commit ea5787c1e, pushed to origin/TAT-3400-fix-trade-with-token-has-20-failur (regular push, no force). PR head is now 0bbab891a.

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.

5 participants