fix(hooks): [SUP-21367] honour usePrevHookAmount in ApproveERC20Hook; stop value-moving HyperCore leaves forwarding

[supervaulter]

Closes SUP-21367.

Origin: the Slack thread on BatchTransferHook → ApproveERC20Hook → swap producing a 0-amount swap. Covers both halves of the ticket — but with different fixes, because they turned out to be different defects. The approve hook needed its flag honoured; the HyperCore hooks needed their _pipeMode() corrected.

Note on review history: my first comment below argued the HyperCore hooks needed no change at all. That conclusion was wrong and there's a correction further down. The reasoning in it still holds — those hooks genuinely cannot publish an output — I just drew the wrong conclusion from it. Both are left in place so the reasoning is auditable.

The bug, in one line

ApproveERC20Hook._preExecute gated on prevHook != address(0) alone, so mid-chain it always delegated to BaseHook's PASSTHROUGH. usePrevHookAmount therefore only ever reached the allowance in _buildHookExecutions — from a downstream consumer's point of view the flag was ignored.

- if (prevHook != address(0)) {
+ if (prevHook != address(0) && _decodeBool(data, USE_PREV_HOOK_AMOUNT_POSITION)) {

Measured before the change, to show the flag really had no effect on what was reported onward:

chain position	approve's own amount	usePrev	reported onward
mid-chain, prev → 0	1000	false	0
mid-chain, prev → 777	1000	false	777
position 0	1000	false	1000

The rule now

The hook reports whatever allowance it set. usePrevHookAmount governs both halves consistently:

• true → allowance is the previous hook's outAmount, and that same value is forwarded
• false → allowance is this hook's own amount, and that is reported onward

Position 0 already behaved the second way, so this also removes a positional inconsistency: the same hook with the same flag no longer reports different kinds of thing depending on where it sits.

What it fixes

A chain whose source produces no output no longer silently zeroes. A multi-token BatchTransferHook reports 0 — it has no single (amount, token) output, and its decodeAmounts() / amountRoles() are empty by design — and 0 is indistinguishable from a real zero. So batchTransfer → approve(usePrev=false) → swap(usePrev=true) swapped nothing, and setting usePrev=false on the approve did not help, because the flag never reached the pipe.

Read this before approving — the trade-off is real

An allowance is an upper bound, not a quantity held. With usePrev=false, a downstream consumer now inherits a figure the account may not actually have:

swap (receives 777) → approve(1000) → deposit(usePrev=true)
   before: deposit gets 777  ✅ works
   after:  deposit gets 1000 ❌ reverts on the shortfall

Blast radius: any chain with a mid-chain approve at usePrev=false followed by a usePrev=true consumer, where the approve amount ≠ the upstream output. Approving a generous round-number cap is a normal pattern, so this wants grepping in OMS/app templates before it ships. I can't see those templates from the repo.

test_PreExecute_UsePrevFalse_ReportsOwnAmount_AllowanceAboveBalanceIsCallersProblem records the choice deliberately, so a future change back to 777 is a conscious reversal rather than an accident.

Tests

Six added. None existed for this before — the two prior getOutAmount assertions only call postExecute, so the behaviour everyone was debating was untested, and this change would have passed CI silently in either direction.

test/unit/hooks/* 3716 · accounting 1312 · executors 54 · SuperExecutor 11 · validators 59 · script 35 · invariant 29 — all green.

Deployment

Bytecode moved, so all three artifacts are regenerated. Same deploy name: the CREATE2 address is keccak256(0xff ++ deployer ++ salt ++ keccak256(initcode)), so new code lands at a new address under the unchanged salt, and the old address stays live for roots already signed against it — the same pattern as SUP-21143's LOAN hook re-pin.

namespace	new address
PROD1.0.0	0x17DdFE8988cF71BBd672e5daFc4f6D02Cf279fA4
STAGING1.0.0	0x0cEE1a19c3d099f16626678501f4D0F0ACA2F91D

Two operational notes: __deployContractIfNeeded skips a name already recorded in script/output/.../latest.json, so those records need updating or the deploy no-ops; and manifests/hooks.json carries per-chain addresses for this hook, which regenerate from the deployment output afterwards (unchanged by this PR).

Part 2 — HyperCore: the leaves that move value stop forwarding

Same ticket, different defect. Hook1 → HyperCoreSendAssetHook → Hook3(usePrevHookAmount = true) had Hook3 operate on Hook1's amount. Nothing looked wrong — the number was plausible, just stale, because SendAsset had moved the assets onto HyperCore in between.

The cause was not the missing usePrevHookAmount field. It was _pipeMode(). PASSTHROUGH is a claim — "the upstream amount still describes the value in flight" — and BaseHyperCoreWriterHook made it for every leaf and locked it non-virtual ("no leaf may regress to TRANSFORM"). True for a leaf that changes no balance; false for one that moves assets.

So _pipeMode() is virtual again, PASSTHROUGH stays the base default, and only the two value-movers override:

hook	mode	why
HyperCoreSendAssetHook	TRANSFORM	sends a spot balance away from the account
HyperCoreUsdClassTransferHook	TRANSFORM	moves USD between spot and perp classes
HyperCoreAddApiWalletHook	PASSTHROUGH	unchanged — registers an agent, moves nothing
HyperCoreApproveBuilderFeeHook	PASSTHROUGH	unchanged — a fee rate, moves nothing

These two publish nothing of their own, because there is nothing publishable: their amounts (amountWei, ntl) are uint64 HyperCore units offset per token by weiDecimals/evmExtraWeiDecimals, and BaseHyperCoreWriterHook records that CoreWriter "never reverts, never validates the action id, and never reports". A downstream consumer therefore reads 0 and fails — Swap1InchHook, Deposit4626VaultHook, ApproveAndDeposit4626VaultHook, AaveV4LendHook and TransferERC20Hook all guard amount == 0. Failing is the right outcome: no hook after these two can legitimately derive its amount from them.

Cheap, because _pipeMode() is internal pure and inlined — not in the calldata, not in storage, not in the data layout. No new field, no data-length change, no inspector/template/manifest update, no roots invalidated. And adding virtual to the base left the other three leaves byte-identical, so AddApiWallet, ApproveBuilderFee and ApproveAndHyperCoreDeposit keep their live addresses. Only two move:

hook	deployed (HyperEVM 999)	new (PROD1.0.0)
HyperCoreSendAssetHook	0x65aea557e595b9e841Da9F47aae8EacC4650Ea6a	0x541ce604cED66dFb2FE0F2Cc951Bffb3f36eEd7D
HyperCoreUsdClassTransferHook	0x0E4eDd421D0707A454C3c0435DeCF5D708A8b29C	0xa02f963Af997032D9b5bdaF42BC5A1Db15e1c57C

Staging: 0x035799A5871634F7B24562804273B139Aadb5AE6 and 0xDef5387D00384B5a65a50f7B07ed6455b4F0B93B.

One existing test pinned the old behaviour and is updated rather than deleted — test_Passthrough_ForwardsPrevHookOutput made its assertion with classTransfer, which now moves value; it asserts the same thing with addAgent, which doesn't. Three tests added, including test_PipeMode_Chain_DownstreamFailsInsteadOfUsingAStaleAmount, which runs the reported chain with TransferERC20Hook as a real consumer and asserts it reverts AMOUNT_NOT_VALID through SendAsset but still builds through AddApiWallet.

Tooling gap closed on the way: the entire HyperCore family was missing from regenerate_bytecode.sh, so a source change to any of those five left the artifacts stale and a deploy would have shipped the previous bytecode. Added as its own group.

Known gap, deliberately not in scope

Six other hooks share the exact same shape — PASSTHROUGH plus a usePrevHookAmount field — and still ignore the flag for reporting: RecordPurchasePendlePTHook, RecordRedemptionPendlePTHook, and the four *AmortizedOracleHook / V2 variants. They don't even override _preExecute, so they go straight to BaseHook's unconditional forward.

I left them because their usePrev=true path is pinned to one specific predecessor (if (prevHook != APPROVED_PENDLE_PT_HOOK) revert PREV_HOOK_NOT_VALID(), plus operation/market/token checks), so forwarding is correct there by construction; their usePrev=false path is documented as standalone, where there is no predecessor to forward from; and they record rather than move value, so ptAmount mirrors PT that PendlePTHook already moved and already reported.

The honest cost: usePrevHookAmount now governs onward reporting in approve but not in those six. Happy to extend it if we'd rather have uniformity — same one-line pattern, six more hooks re-pinned.

🤖 Generated with Claude Code

https://claude.ai/code/session_0157RUiyLFJjzULEEq6h6biV

[linear-code]

SUP-21367

[supervaulter]

Why the HyperCore half of SUP-21367 is not in this PR

The ticket covers "approve and hypercore hooks". I looked at the HyperCore ones and concluded they need no change — not as a deferral, but because the fix applied here is inapplicable to them and the fix I originally proposed turns out to be impossible. Recording the reasoning so it doesn't get re-litigated.

First, only two of the four move value at all

hook	amount in calldata	moves value	verdict
HyperCoreSendAssetHook	uint64 amountWei @80	yes	see below
HyperCoreUsdClassTransferHook	uint64 ntl @52	yes	see below
HyperCoreAddApiWalletHook	none (agent + name)	no	unconditional passthrough already correct
HyperCoreApproveBuilderFeeHook	none (maxFeeRate is a rate)	no	unconditional passthrough already correct

1. They have no usePrevHookAmount field — by design

This PR's fix is one line that reads that flag. These hooks don't have one, and both say why in their own docblocks:

HyperCoreSendAssetHook: "wei is in HyperCore units for the token, which differ from EVM units per token via weiDecimals and evmExtraWeiDecimals. This hook therefore does NOT support usePrevHookAmount and does not implement ISuperHookContextAware — the caller supplies a converted value."

HyperCoreUsdClassTransferHook: "It is NOT the same scale as an arbitrary EVM token amount: the EVM offset varies per token via weiDecimals and evmExtraWeiDecimals. This hook therefore does NOT support usePrevHookAmount…"

Adding the flag means changing their calldata layout → new data length → new inspectors, templates and manifest entries, and any signed roots for them invalidated.

2. Even with the flag, they would have nothing to report

The flag only lets a hook suppress forwarding. These two would still publish nothing of their own, so a downstream usePrevHookAmount = true would read 0 — exactly the bug this ticket exists to fix. The change would accomplish nothing.

3. And there is no EVM-observable delta to report instead

My first proposal was to reclassify them TRANSFORM and have them report a real balance delta, the way TransferHook and the swap hooks do with a pre/post read. That is not possible. From BaseHyperCoreWriterHook:

"CoreWriter cannot fail. It never reverts, never validates the action id, and never reports"

The value moves on HyperCore, not on the EVM, so there is no pre/post balance to diff. The only number available is the calldata amountWei / ntl, which is in HyperCore units — publishing that into outAmount, which downstream hooks consume as an EVM token quantity, would be a wrong-unit figure. That is worse than 0, because it looks plausible.

What would actually fix the HyperCore case

Not a per-hook change. Two things, in order of how soon they can ship:

1. Off-chain, today, no contract change. Reject at intent-build time any usePrevHookAmount = true that follows a hook declaring no usable amount (amountRoles() empty). The sizing interface from feat: hooks standardization #923 already supplies that metadata — it just isn't being checked. This catches the HyperCore hooks and BatchTransferHook by the same rule.
2. Chain-level, as its own release. Make "no output" a distinct state from 0 in BaseHook and revert when a consumer reads it. Then a swap placed after a HyperCore hook fails loudly in simulation instead of silently moving nothing. This re-pins every hook's bytecode (~120), so it wants to be a deliberate release rather than a rider on this PR.

The practical takeaway

Subhasish was right that the HyperCore hooks are affected — but by a different defect with a different fix. Worth being explicit about, otherwise the natural follow-up ("add usePrevHookAmount to the HyperCore hooks") gets filed and won't help.

Separately: the uint64 HyperCore-unit reasoning above is read off the hooks' own documentation rather than from Hyperliquid experience, so @vik should sanity-check point 3 — specifically whether a spot↔perp class transfer has any EVM-observable effect I've missed.

🤖 Generated with Claude Code

https://claude.ai/code/session_0157RUiyLFJjzULEEq6h6biV

[subhasishgoswami]

This fixes the Bundler incident's approval-output failure: with usePrevHookAmount=false, approval reports its configured amount and token, allowing the following dynamic Kyber swap to consume the adjusted amount.

Verified that the PR's compiled runtime, after immutable substitution, is byte-for-byte identical to the runtime used in the successful historical replay at Base block 52307672. The unchanged control reproduced ZERO_AMOUNT; changing only approval behavior completed both swaps, the combined vault deposit, and the final transfers to the owner. This validates the captured destination build simulation, before later fee-hook setup.

Successful replay: https://dashboard.tenderly.co/superform/v2/simulator/76393756-4600-4eb3-88f8-2c7904845e7c
Control: https://dashboard.tenderly.co/superform/v2/simulator/3dab580b-bcce-471a-9987-fc729647b4f2

No executor scaling or HyperCore change is needed for this incident. Rollout still requires deploying the corrected hook and updating Bundler registry selection/cache while preserving old hook identities. Build and manifest checks have passed; the test job was still running when reviewed.