fix(balancer-v3): require registered pool address, drop zeroAddress fallback - #10
Merged
Conversation
…allback
Background:
BalancerV3Adapter.buildSwap() shipped a `swapSingleTokenExactIn`
call with `zeroAddress` as the pool argument, gated only by a
`// TODO: resolve pool from registry` comment. The factory wires
the adapter via `case "balancer_v3"`, but no chain in the registry
declares `interface = "balancer_v3"`, so the broken path has been
unreachable in practice — yet the unit test was happily decoding
calldata that, if anyone wired it up, would have called the V3
Router with `pool = 0x0` and reverted (or worse, silently routed
through some default).
Fix:
- Constructor reads `entry.contracts["pool"]` if present.
- buildSwap throws DefiError.invalidParam with a clear remediation
message ("register `pool = "0x..."` under [protocol.contracts]")
when the pool is missing — same error class as the existing
amount_out_min guard.
- When the pool is registered, the calldata uses it verbatim;
description string echoes the pool for log clarity.
Test changes:
- Added pool to the existing ENTRY fixture and a parallel
ENTRY_NO_POOL fixture for the new failure case.
- New test: "buildSwap refuses to ship without a registered pool
address".
- Hardened the existing happy-path test to also assert the pool
address survives encoding (previously only checked
minAmountOut).
Out of scope:
- Multi-pool routing / on-chain factory queries / quoter
integration — Balancer V3's BatchRouter is the right surface
for that and is intentionally not pursued here.
- Removing the BalancerV3 export+factory case. Keeping it dormant
so the next consumer can register `pool = "0x..."` and ship
without re-adding the adapter.
Verified:
- pnpm -C ts -r build — clean.
- pnpm -C ts -r lint — 3 packages clean.
- pnpm -C ts -r test — 64/64 (defi-cli) + 39/39 (defi-protocols),
balancer_v3.test.ts now 3 tests (was 2).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BalancerV3Adapter.buildSwap()was encoding aswapSingleTokenExactIncall withpool = 0x0, gated only by a// TODO: resolve pool from registrycomment index/balancer_v3.ts:55. No chain inchains.tomlcurrently usesinterface = "balancer_v3", so this code path is unreachable in production — but if anyone wires up a Balancer V3 entry tomorrow, they'd ship a router call topool = address(0)that reverts (or worse, silently routes through some default).This PR closes the footgun: register a pool, or the adapter refuses to build the call.
Changes
dex/balancer_v3.tsentry.contracts["pool"]and stores it (optional).buildSwapthrowsDefiError.invalidParamwith a clear remediation message when the pool is missing — same error class as the existingamount_out_minguard.descriptionstring includes the pool address for log clarity.dex/balancer_v3.test.tspoolinENTRY.contracts. NewENTRY_NO_POOLfixture mirrors the production "router only" mistake.minAmountOut).Result: balancer_v3 test file goes from 2 → 3 tests, all passing.
What's NOT in this PR
BalancerV3Adapterexport /case "balancer_v3"in factory.ts. Keeping the scaffold dormant so the next consumer can registerpool = "0x..."and ship without re-adding the adapter.Test plan
pnpm -C ts -r build— clean.pnpm -C ts -r lint— 3 packages,tsc --noEmitclean.pnpm -C ts -r test— 64/64 (defi-cli) + 39/39 (defi-protocols).🤖 Generated with Claude Code