Skip to content

feat(aave-v3): Pool addresses for Arbitrum/Optimism + multi-market support - #24

Merged
chrisli30 merged 3 commits into
mainfrom
feat/aave-v3-pool-markets
Aug 12, 2026
Merged

feat(aave-v3): Pool addresses for Arbitrum/Optimism + multi-market support#24
chrisli30 merged 3 commits into
mainfrom
feat/aave-v3-pool-markets

Conversation

@chrisli30

Copy link
Copy Markdown
Member

Summary

Closes #23. Extends the Aave V3 address maps so Studio (and other write-path consumers) can drop the hand-written AAVE_V3_MARKETS literal and target every live Pool, not just Core.

  • Add Chains.ArbitrumOne (42161) and Chains.OptimismMainnet (10).
  • Add Arb/OP to aaveV3.pool / oracle / poolAddressesProvider / uiPoolDataProvider / wethGateway.
  • Add aaveV3.markets — every Pool on a chain. Ethereum lists Core / EtherFi / Lido / Horizon; every other covered chain is a single core row. Each row is { key, pool, poolAddressesProvider }.
  • aaveV3.pool[chainId] stays the canonical / Core market so existing callers do not break.
  • Document that the static Pool map is a cache of PoolAddressesProvider.getPool().

Addresses independently confirmed against @aave-dao/aave-address-book (AaveV3Ethereum, AaveV3EthereumEtherFi, AaveV3EthereumLido, AaveV3EthereumHorizon, AaveV3Arbitrum, AaveV3Optimism).

Out of scope: aaveV3.reserves stays 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
  • CI on this PR

…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.
@chrisli30

Copy link
Copy Markdown
Member Author

Audit — addresses verified on-chain, one coverage gap worth closing before merge

Reviewed 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 state

Rather than diffing against the address book, I called PoolAddressesProvider.getPool() (0x026b1d5f) on each listed provider and compared it to the listed pool. That validates both addresses in each row simultaneously, against live chain state:

chain 1      core      MATCH   getPool()=0x87870bca3f3fd6335c3f4ce8392d69350b4fa4e2
chain 1      etherFi   MATCH   getPool()=0x0aa97c284e98396202b6a04024f5e2c65026f3c0
chain 1      lido      MATCH   getPool()=0x4e033931ad43597d96d6bcc25c280717730b58b1
chain 1      horizon   MATCH   getPool()=0xae05cd22df81871bc7cc2a04becfb516bfe332c8
chain 42161  core      MATCH   getPool()=0x794a61358d6845594f94dc1db02a252b5b4814ad
chain 10     core      MATCH   getPool()=0x794a61358d6845594f94dc1db02a252b5b4814ad
chain 8453   core      MATCH   getPool()=0xa238dd80c259a72e81d7e4664a9801593f98d1c5

Including the one that looks like a copy-paste error: Arbitrum and Optimism genuinely share Pool 0x794a6135… — confirmed independently on both chains. Worth knowing so nobody "fixes" it later.

⚠️ Gap: new chains get a Pool but no reserves

scripts/generate-aave-reserves.ts gained RPC entries for Optimism and Arbitrum, but the reserves catalog was not regenerated:

aaveV3.reserves →  1: 66,  56: 8,  8453: 15,  84532: 6,  11155111: 9
                   (no 42161, no 10)

So on Arbitrum a consumer gets:

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-labsaave-dao/aave-address-book (the current canonical org)
  • reserves explicitly 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] is core and equals pool[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.
@chrisli30

Copy link
Copy Markdown
Member Author

Addressed the three review notes:

  1. Reserves gap — regenerated aaveV3.reserves from chain. Arb now has 20 Core reserves, OP 14. Also picked up Ethereum's new PT-srUSDe-22OCT2026 row (66 → 67). A test pins chain-native symbols (ARB / OP) so the two catalogs cannot get swapped despite the shared CREATE2 Pool address.

  2. coreMarket() throw — removed. A missing pool/provider stays undefined and fails the existing lockstep test instead of crashing package import for every consumer.

  3. Closed AaveV3MarketKey — left closed, added a comment that widening it to string is the wrong move.

Non-Core Ethereum markets (EtherFi / Lido / Horizon) still do not have their own reserve lists — that stays a follow-up, as agreed.

@chrisli30

Copy link
Copy Markdown
Member Author

Re-review of c3cfdb2 — both findings addressed, reserves verified on-chain

Both items from my earlier review are resolved, and the new reserve data is correct against live chain state. No blockers from me.

Also: I can run the suite this time (node_modules survived from the earlier attempt), so retracting my previous "reviewed by reading only" disclosure — 69/69 pass.

✅ Finding 1 — reserves now ship, and the data is right

Not just present: I verified all 34 new rows against the Pool, the same way as the addresses. getReservesList() for the set, getReserveData(asset) for each row's aToken / variableDebtToken, and the decimals field decoded from bits 48–55 of the reserve configuration bitmap.

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:148Object.keys(markets)Object.keys(pool)
  • tests/catalog.test.ts:370Object.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.
@chrisli30

Copy link
Copy Markdown
Member Author

Addressed both follow-ups from the re-review:

  1. reserves key-set testObject.keys(reserves) is now asserted equal to Object.keys(pool), same pattern as markets / providers. A new Pool without a regenerate fails CI instead of shipping an empty supply-token list.

  2. Duplicate USDC — bridged token on Arb/OP is now USDC.e; native Circle USDC stays USDC. The generator owns the override (well-known USDC.e addresses) so the next yarn generate:aave-reserves will not revert it. find(r => r.symbol === "USDC") now returns native on both chains. Also added a per-chain symbol-uniqueness test.

71 tests pass.

@chrisli30
chrisli30 merged commit 88ec8f2 into main Aug 12, 2026
6 checks passed
@chrisli30
chrisli30 deleted the feat/aave-v3-pool-markets branch August 12, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant