Skip to content

fix: strategy and vault hardening (replay of 2df1bb9)#3

Open
antfleet-ops wants to merge 1 commit into
mainfrom
bench/2df1bb9-strategy-vault-hardening
Open

fix: strategy and vault hardening (replay of 2df1bb9)#3
antfleet-ops wants to merge 1 commit into
mainfrom
bench/2df1bb9-strategy-vault-hardening

Conversation

@antfleet-ops
Copy link
Copy Markdown

Benchmark replay

This PR replays upstream commit 2df1bb9 ("fix(security): gate strategy entry points with onlyVault + harden vault") as a diff against its parent.

Scope: strategy access-control hardening, FloatVault reentrancy protection, AgentFloatHook LP accounting, token approval compatibility, deploy script updates, and regression tests.

Not for merge. Purpose: AntFleet two-model PR review evaluation.

See BENCHMARK.md for the policy.

Smart-contract audit found strategy deposit()/withdraw() were callable by
anyone, letting funds be drained directly from a strategy (bypassing the
vault). Fixes:

- AaveStrategy/IdleStrategy/MockYieldStrategy: bind an immutable vault and
  add an onlyVault modifier on deposit()/withdraw(); constructors now take
  the vault address (call sites + deploy scripts updated).
- FloatVault: inherit ReentrancyGuard; park/withdraw/promote are nonReentrant.
- AgentFloatHook: symmetric lpProvider accounting on add/remove (refund
  capped to actual balance); switch to forceApprove for USDT compatibility.
- New regression test test_Revert_DirectStrategyWithdraw; 13/13 pass.

Live mainnet remediation: redeployed the hardened AaveStrategy to
0xB433487F82572FF201A2455BF7a06325a7B8bFEa (strategy ID 4) and migrated
pooled capital via the trustless on-chain promote() path. Docs updated to
the new address and 13/13 test count.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@antfleet
Copy link
Copy Markdown

antfleet Bot commented May 31, 2026

AntFleet · 5 findings

Both reviewers flagged the items below on the changed files. AntFleet posts only what two independent frontier models agree on.


Bug · High — Watcher always uses testnet chain config regardless of CONFIG.chainId
agent/src/watcher.ts:1-22

startWatcher detects mainnet via CONFIG.chainId === 196 and logs accordingly, but unconditionally constructs the viem PublicClient with xLayerTestnetChain. The transport uses http() without an explicit URL, so it falls back to chain.rpcUrls.default. On mainnet this means the watcher polls the testnet chain rather than the configured CONFIG.rpcUrl/mainnet, causing the agent's epoch loop, scoring, and promotions to read state from the wrong chain. README claims the agent runs against the mainnet…

Fix: Pick chain (and transport URL) based on CONFIG.chainId: e.g. chain: isMainnet ? xLayerMainnetChain : xLayerTestnetChain, and pass transport: http(CONFIG.rpcUrl). Add a regression test that verifies the client points at the configured RPC.


Bug · High — FloatVault.withdraw ignores strategy’s actualOut and attempts to transfer requested amount regardless of received funds
contracts/src/FloatVault.sol:91-118

If a strategy cannot return the full requested amount (e.g., a mock or real strategy under stress/rounding), IStrategy.withdraw will return a smaller actualOut. The vault ignores this value, then attempts to transfer ‘amount’ to the caller, which can revert due to insufficient balance or mis-account the vault’s funds.

Fix: Use the return value from IStrategy(activeStrategy).withdraw(amount) and transfer only actualOut to the caller. Consider reverting if actualOut < amount and the vault can’t make up the difference from its own balance. Update accounting consistently with the actualOut value.


Bug · Medium — AgentFloatHook._beforeSwap uses regular approve (not forceApprove) on USDT-style tokens
contracts/src/AgentFloatHook.sol:144-158

On mainnet USDT is the underlying. The README and other code paths consistently use forceApprove (zero-then-set) because USDT rejects non-zero → non-zero approve. The transient-storage branch of _beforeSwap uses plain usdc.approve(address(poolManager), recallAmount) (no SafeERC20 forceApprove), so the second+ JIT recall in a row will revert when the prior allowance to poolManager is non-zero. The fallback branch correctly uses forceApprove, which makes the inconsistency a real bug rather than…

Fix: Use usdc.forceApprove(address(poolManager), recallAmount) in both branches.


Security · Medium — ALLOWED_ORIGIN defaults to '*' but Access-Control-Allow-Credentials is always 'true'
agent/src/api.ts:47-60

When DASHBOARD_ORIGIN is not set, ALLOWED_ORIGIN === '*' but the response also sets access-control-allow-credentials: true. Browsers reject this combination (cookies/auth headers won't be sent), and more importantly, hosting providers/load balancers that allow this header will accept cross-origin credentialed requests from anywhere, expanding attack surface for SIWE replay or CSRF if cookies are added later. The agent admin endpoints (POST /api/mode, /api/proposals/.../reject) could be hit cr…

Fix: Reflect the request Origin only when it matches an allowlist, or refuse to emit Allow-Credentials when Allow-Origin is ''. At minimum, default ALLOWED_ORIGIN to a safe value rather than ''.


Docs-gap · Low — README claims testsPassed=13/13 but API hardcodes testsPassed=8/8
agent/src/api.ts:109-117

The README and pitch deck loudly claim '13/13 Forge tests passing,' but the agent's /api/state response reports testsPassed=8, testsTotal=8 as hardcoded constants. The dashboard reading this state will display the wrong number, contradicting the README narrative. Also, the values are stale (a previous test count), suggesting they were never wired to actual test results.

Fix: Either remove these hardcoded fields or read them from a build artifact / forge JSON output.

Review 57e5c9ae · claude-opus-4-7 + gpt-5 (unanimous) · 184s · ~$0.40

@antfleet
Copy link
Copy Markdown

antfleet Bot commented Jun 1, 2026

AntFleet · finding 57e5c9ae-1 closed in 3c10efc

Bug · Medium — AgentFloatHook._beforeSwap uses regular approve (not forceApprove) on USDT-style tokens
contracts/src/AgentFloatHook.sol:144-158

Originally flagged in the AntFleet review. Receipt automated.

@antfleet
Copy link
Copy Markdown

antfleet Bot commented Jun 1, 2026

AntFleet · finding 57e5c9ae-2 closed in 3c10efc

Bug · High — FloatVault.withdraw ignores strategy’s actualOut and attempts to transfer requested amount regardless of received funds
contracts/src/FloatVault.sol:91-118

Originally flagged in the AntFleet review. Receipt automated.

@antfleet
Copy link
Copy Markdown

antfleet Bot commented Jun 1, 2026

AntFleet · finding 57e5c9ae-3 closed in 3c10efc

Docs-gap · Low — README claims testsPassed=13/13 but API hardcodes testsPassed=8/8
agent/src/api.ts:109-117

Originally flagged in the AntFleet review. Receipt automated.

@antfleet
Copy link
Copy Markdown

antfleet Bot commented Jun 1, 2026

AntFleet · finding 57e5c9ae-4 closed in 3c10efc

Security · Medium — ALLOWED_ORIGIN defaults to '*' but Access-Control-Allow-Credentials is always 'true'
agent/src/api.ts:47-60

Originally flagged in the AntFleet review. Receipt automated.

@antfleet
Copy link
Copy Markdown

antfleet Bot commented Jun 1, 2026

AntFleet · finding 57e5c9ae-0 closed in 3c10efc

Bug · High — Watcher always uses testnet chain config regardless of CONFIG.chainId
agent/src/watcher.ts:1-22

Originally flagged in the AntFleet review. Receipt automated.

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.

1 participant