feat(aave-v3): Pool addresses for Arbitrum/Optimism + multi-market support - #24
Conversation
…market Add aaveV3.markets so consumers can enumerate every Pool on a chain (Core/EtherFi/Lido/Horizon on Ethereum) and extend the Core maps to Arbitrum One and Optimism. Addresses from @aave-dao/aave-address-book. aaveV3.pool[chainId] stays Core so existing callers do not break. Closes #23.
Audit — addresses verified on-chain, one coverage gap worth closing before mergeReviewed as the requester of #23. Summary: every address in this PR is correct, the shape matches what was asked for, and there is one gap that would bite the first consumer to use it. ✅ All 7 Pool addresses verified against chain stateRather than diffing against the address book, I called Including the one that looks like a copy-paste error: Arbitrum and Optimism genuinely share Pool
|
aaveV3.pool[42161] |
✅ |
aaveV3.markets[42161] |
✅ |
aaveV3.reserves[42161] |
❌ absent |
You cannot compose a supply/repay with a Pool alone — you need the underlying address, decimals and aToken from reserves. In Studio terms, getAaveV3SupplyTokens(42161) returns [], so the Arbitrum write path would have a target and nothing to send to it.
Since the generator already has the RPC wiring, this is presumably one npm run generate away. Worth doing in this PR so the new chains are actually usable rather than half-shipped — otherwise the first consumer hits a confusing empty list rather than a clear "unsupported chain".
Design questions (not blockers)
1. coreMarket() throws at module evaluation. It runs inside the markets object literal, so an inconsistent catalog (a chain with pool but no poolAddressesProvider) means the entire package fails to import for every consumer, not just Aave users. Fail-fast on malformed static data is defensible, but the same invariant is already covered by the new test (core.poolAddressesProvider === poolAddressesProvider[chain]). A test failure at build time is friendlier than an import-time crash in production. Your call — just naming it as a deliberate choice rather than an accident.
2. AaveV3MarketKey is a closed union. "core" | "etherFi" | "lido" | "horizon" means adding a market (Horizon is recent — they do add them) is a type change that breaks exhaustive consumers. I read that as intentional and good: a new market should be a compile error at every consumer rather than a silently-ignored row. Worth a one-line comment saying so, since the next person may be tempted to widen it to string.
What the PR got right
pool[chainId]preserved as Core → no breaking change for existing consumers, exactly as feat(aave-v3): Pool addresses for Arbitrum/Optimism + multi-market support (Ethereum Core/EtherFi/Lido/Horizon) #23 suggested- Each market carries its own
poolAddressesProvider, so the on-chain escape hatch is per-market rather than per-chain — better than what I asked for - The proxy nuance is captured accurately in the docstring: static map as a deliberate cache of
getPool(), not an immutability claim - Address source updated
bgd-labs→aave-dao/aave-address-book(the current canonical org) reservesexplicitly flagged Core-only, with market-scoping deferred to a separate issue — right call, non-Core markets list different reserves- Tests pin the invariant that matters (
markets[chain][0]iscoreand equalspool[chain]), plus uniqueness and address shape - Changeset included
Disclosure
I could not run the test suite — npm ci exceeded my 10-minute limit, so the tests are reviewed by reading only. Everything above about addresses is from live RPC calls, not from reading the diff.
Related
@avaprotocol/protocols@0.8.0 already ships everything #19 asked for — uiPoolDataProvider, poolAddressesProvider, and both bit-layout maps including the "nice-to-have" reserveConfigurationBits. #19 looks closable, independent of this PR.
Address review on #24: - Regenerate aaveV3.reserves so Arbitrum (20) and Optimism (14) have Core underlyings/aTokens, not just a Pool. Also picks up Ethereum's new PT-srUSDe-22OCT2026 row. - coreMarket() no longer throws at module evaluation — a missing pool/provider is a test failure, not a package-load crash. - Document that AaveV3MarketKey is a closed union on purpose.
|
Addressed the three review notes:
Non-Core Ethereum markets (EtherFi / Lido / Horizon) still do not have their own reserve lists — that stays a follow-up, as agreed. |
Re-review of
|
| chain | catalog | getReservesList() |
underlying set | aToken + vDebt + decimals |
|---|---|---|---|---|
| Arbitrum (42161) | 20 | 20 | exact match | 20/20 OK |
| Optimism (10) | 14 | 14 | exact match | 14/14 OK |
No missing rows, no extra rows. The new Ethereum row picked up in the regen also checks out:
PT-srUSDe-22OCT2026
aToken chain 0x01e69a58…0545 catalog 0x01E69a58…0545 ✅
vDebt chain 0x4af166c6…8083 catalog 0x4Af166c6…8083 ✅
decimals chain 18 catalog 18 ✅
Worth calling out because this was the failure mode most likely to slip through: Arb and OP share aToken addresses across chains (same CREATE2 deployment sequence) but assign them to different reserves — e.g. 0x8Eb270e2… is rETH's aToken on Arbitrum and LUSD's on Optimism. A catalog mix-up would have produced entirely plausible-looking rows. Each row was resolved from its own chain, so the mapping is confirmed per-chain, not by shape. The ARB / OP symbol guard in the new test is a good instinct for exactly this.
✅ Finding 2 — import-time throw removed; the casts are safe
I flagged the replacement as \0x${string}` casts as a possible silent-undefinedswap and went looking for a hole. **There isn't one**, and it's better guarded than I gave it credit for — two *data-derived* key-set assertions pin bothcoreMarketinputs to thepool` key set:
tests/catalog.test.ts:148—Object.keys(markets)≡Object.keys(pool)tests/catalog.test.ts:370—Object.keys(poolAddressesProvider)≡Object.keys(pool)
I tested the worst case rather than assuming: deleted poolAddressesProvider[BnbMainnet] and removed BNB from the hand-written AAVE_V3_CHAINS (simulating a new chain whose test list nobody updated). The :370 assertion still failed. So the casts cannot ship undefined — good trade, crash removed without losing the invariant.
AaveV3MarketKey's closed-union rationale is documented too. 👍
Follow-up (non-blocking): reserves is the one map without a derived key-set test
markets, poolAddressesProvider and uiPoolDataProvider each have a key-set assertion against pool. reserves does not — its only coverage is AAVE_V3_CHAINS, which is hand-maintained in the test file.
Which means the exact bug this commit fixes can recur. I confirmed it empirically — deleted the Arbitrum reserve block, removed ArbitrumOne from AAVE_V3_CHAINS, dropped the new Arb/OP assertion:
Test Files 3 passed (3)
Tests 68 passed (68) ← a chain with a Pool and no reserves, fully green
One line, matching the pattern already at :370, converts "remember to regenerate reserves" into a test failure:
expect(Object.keys(Protocols.aaveV3.reserves).sort())
.toEqual(Object.keys(Protocols.aaveV3.pool).sort());I checked this passes today — all five maps are now on the same key set:
pool / markets / poolAddressesProvider / uiPoolDataProvider / reserves
= 1, 10, 56, 8453, 42161, 84532, 11155111
Follow-up (non-blocking): duplicate USDC symbol on the new chains
Arbitrum and Optimism are the only chains in the catalog with two rows sharing a symbol — bridged USDC.e and native USDC. That is faithful to chain state (I checked symbol() on all four contracts; every one literally returns "USDC"), so this is a consumer-ergonomics note, not a data error.
The catch is that a symbol lookup silently resolves to the bridged token on both chains, since it sorts first:
| chain | find(r => r.symbol === "USDC") returns |
probably wanted |
|---|---|---|
| 42161 | 0xFF970A61… (USDC.e, bridged) |
0xaf88d065… (native) |
| 10 | 0x7F5c764c… (USDC.e, bridged) |
0x0b2C639c… (native) |
Symbol-keyed lookup is an established pattern here (Sepolia LINK reserve… test, and Studio's supply-token picker), so it's worth a deliberate call — disambiguate the symbol (USDC.e), add a bridged?: true flag, or document that consumers must key on underlying. A UI listing two identical "USDC" options is the visible half; picking the wrong one by default is the quiet half.
Both follow-ups are fine as separate issues — neither should hold this PR.
Verification
vitest run → 3 files, 69 passed
on-chain reserve check → 34/34 rows (Arb 20, OP 14) + new Ethereum row
Address the two non-blocking follow-ups on #24: - Assert Object.keys(reserves) === Object.keys(pool) so a new Pool cannot ship without regenerating the reserve catalog. - Label bridged USDC.e on Arbitrum/Optimism as USDC.e. On-chain both tokens report "USDC"; without this, find(r => r.symbol === "USDC") silently returned the bridged token. Generator keeps the override so the next regen does not revert it.
|
Addressed both follow-ups from the re-review:
71 tests pass. |
Summary
Closes #23. Extends the Aave V3 address maps so Studio (and other write-path consumers) can drop the hand-written
AAVE_V3_MARKETSliteral and target every live Pool, not just Core.Chains.ArbitrumOne(42161) andChains.OptimismMainnet(10).aaveV3.pool/oracle/poolAddressesProvider/uiPoolDataProvider/wethGateway.aaveV3.markets— every Pool on a chain. Ethereum lists Core / EtherFi / Lido / Horizon; every other covered chain is a singlecorerow. Each row is{ key, pool, poolAddressesProvider }.aaveV3.pool[chainId]stays the canonical / Core market so existing callers do not break.PoolAddressesProvider.getPool().Addresses independently confirmed against
@aave-dao/aave-address-book(AaveV3Ethereum,AaveV3EthereumEtherFi,AaveV3EthereumLido,AaveV3EthereumHorizon,AaveV3Arbitrum,AaveV3Optimism).Out of scope:
aaveV3.reservesstays Core-only. Non-Core markets list different reserves; that is a follow-up.Also closed #19 separately — that surface already shipped in #20 /
0.8.0.Test plan
yarn test:run(68 tests)yarn typecheck