Skip to content

Conversation

@shoom3301
Copy link
Contributor

@shoom3301 shoom3301 commented Nov 6, 2025

Network: gnosis
Chain ID: 100
Deployed Contracts:
AaveV3AdapterFactory: 0x7f230F7Cee38ca371b871B74B3C6ded4932A2f93
CollateralSwapAaveV3Adapter: 0x837aA74e8daf7Fd7160f078ca455a67B7F441E4b
DebtSwapAaveV3Adapter: 0xC9A495A17B1eeC18AE0f0865840B8fd3f8a9DE3F
RepayWithCollateralAaveV3Adapter: 0x2890CE372f3a397B6f3BB3a71B1836A6F3F33657

Network: ethereum
Chain ID: 1

Deployed Contracts:
AaveV3AdapterFactory: 0x22E08c56a6799e28e7b05A117D853B9b46abc017
CollateralSwapAaveV3Adapter: 0xFEb471EC22E5572dbb44229301c266f4C305A78a
DebtSwapAaveV3Adapter: 0x238f57A2c3F0696fB20295075a0F9A18EfC67D3a
RepayWithCollateralAaveV3Adapter: 0x9eB0ffd318e862D344792a8e589e8393E8bEd96F

Network: base
Chain ID: 8453

Deployed Contracts:
AaveV3AdapterFactory: 0xc5D68e305e0b5998f895e34d4440954072F285B6
CollateralSwapAaveV3Adapter: 0xBb45A7898A6f06a9c148BfeD0C103140F0079cd9
DebtSwapAaveV3Adapter: 0xaCbd34fAB78BD6C8738eb32dDAFd688df98CD2E3
RepayWithCollateralAaveV3Adapter: 0x1549445700D0Cb2D7Ce85ECd5a7FD7Ba4a3D40A7

Pool addresses are taken from https://aave.com/docs/resources/addresses

Summary by CodeRabbit

  • Tests

    • Collateral-swap tests switched to the Gnosis chain and updated related expectations and verifications.
  • Chores

    • Expanded Aave configuration to explicit per-chain address mappings for pools, factories, and typed adapter hooks.
    • Updated the exported adapter domain identifier to "AaveV3AdapterFactory".
    • Adjusted ABI metadata for DOMAIN_SEPARATOR (structure-only, no behavioral change).

@shoom3301 shoom3301 requested review from a team and carlos-cow November 6, 2025 16:33
@shoom3301 shoom3301 self-assigned this Nov 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

Tests and Aave constants were updated: tests now target GNOSIS_CHAIN instead of SEPOLIA; Aave addresses/hooks were refactored to typed per-chain Records and per-type hook maps; ADAPTER_DOMAIN_NAME was renamed to "AaveV3AdapterFactory"; DOMAIN_SEPARATOR ABI metadata was enriched.

Changes

Cohort / File(s) Summary
Test Chain ID Migration
packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts
Replaced SEPOLIA chainId usages with GNOSIS_CHAIN, updating calls, mock expectations, adapter factory lookups, receiver/verifyingContract references, EIP‑712/EIP‑1271 domain expectations, and chain-dependent test branches.
Constants Type Safety & Per-Chain Support
packages/flash-loans/src/aave/const.ts
Converted AAVE_POOL_ADDRESS and AAVE_ADAPTER_FACTORY into Record<SupportedChainId, string> with per-chain entries; introduced typed per-chain adapter hook constants (AAVE_COLLATERAL_SWAP_ADAPTER_HOOK, AAVE_DEBT_SWAP_ADAPTER_HOOK, AAVE_REPAY_COLLATERAL_ADAPTER_HOOK) and reconstructed AAVE_HOOK_ADAPTER_PER_TYPE; renamed ADAPTER_DOMAIN_NAME to 'AaveV3AdapterFactory'.
ABI Metadata Updates
packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts
Added explicit type and inputs fields for the DOMAIN_SEPARATOR ABI entry and added internalType: 'bytes32' to its outputs metadata.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Check test branches for any remaining SEPOLIA references and ensure GNOSIS_CHAIN expectations (verifyingContract, receiver addresses) are correct.
  • Verify consumers expecting previous untyped maps still work with the new Record<SupportedChainId, string> shape.
  • Confirm ADAPTER_DOMAIN_NAME rename aligns with all EIP‑712 signing and verification usages.

Possibly related PRs

Poem

🐰 I hopped from Sepolia to Gnosis with glee,
Per-chain hooks now mapped neatly for me,
DOMAIN_SEPARATOR got tidy new lore,
Tests point where adapters call out the door,
A little rabbit cheers the code—huzzah! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding support for three new networks (Mainnet, Gnosis, and Base) for AAVE flash loans.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/aave-contract-addresses-1

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e50146a and d1bbc91.

📒 Files selected for processing (1)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (20 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Publish to GitHub Packages
  • GitHub Check: Build Package
  • GitHub Check: test
  • GitHub Check: eslint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

📦 GitHub Packages Published

Last updated: Nov 7, 2025, 06:31:44 AM UTC

The following packages have been published to GitHub Packages with pre-release version pr-657-c001eb93:


Installation

These packages require authentication to install from GitHub Packages. First, create a .npmrc file:

# Create .npmrc file in your project root
echo "@cowprotocol:registry=https://npm.pkg.github.com" > .npmrc
echo "//npm.pkg.github.com/:_authToken=YOUR_GITHUB_TOKEN" >> .npmrc

To get your GitHub token:

  1. Go to https://github.com/settings/tokens
  2. Click "Generate new token (classic)"
  3. Check only the "read:packages" scope
  4. Copy the token and replace YOUR_GITHUB_TOKEN in the .npmrc file

Then install any of the packages above, either by exact version (i.e. @cowprotocol/[email protected]) or more conveniently by using the tag (@cowprotocol/cow-sdk@pr-657):

# Yarn
yarn add npm:@cowprotocol/cow-sdk@pr-657

# pnpm
pnpm install npm:@cowprotocol/cow-sdk@pr-657

# NPM
npm install npm:@cowprotocol/cow-sdk@pr-657

Update to the latest version (only if you used the tag)

Every commit will publish a new package. To upgrade to the latest version, run:

# Yarn
yarn upgrade @cowprotocol/cow-sdk

# pnpm
pnpm update @cowprotocol/cow-sdk

# NPM
npm update @cowprotocol/cow-sdk

View Packages

You can view the published packages at: https://github.com/cowprotocol/cow-sdk/packages

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (1)

162-177: Consider expanding test coverage to new chains.

While the tests now validate GNOSIS_CHAIN functionality, the PR also adds support for MAINNET (chainId 1) and BASE (chainId 8453). Consider adding similar test coverage for these chains to ensure consistent behavior across all newly supported networks.

packages/flash-loans/src/aave/const.ts (1)

26-31: Add runtime validation for empty adapter factory addresses.

The AAVE_ADAPTER_FACTORY mapping contains empty strings for unsupported chains (9 of 12 SupportedChainId values). While TypeScript compiles successfully, all five runtime accesses (lines 224, 226, 354, 489, 512) pass chainId directly to AAVE_ADAPTER_FACTORY[chainId] without validating the result. If an unsupported chain reaches these methods, the empty string will propagate into blockchain interactions, causing silent failures or unclear errors.

Recommend adding explicit validation at SDK entry points (e.g., collateralSwap, getOrderPostingSettings) or before each blockchain interaction to ensure chainId is in the supported set or throw a clear error early.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fac016 and 756daae.

⛔ Files ignored due to path filters (1)
  • packages/contracts-ts/src/generated/packageVersion.ts is excluded by !**/generated/**
📒 Files selected for processing (3)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (19 hunks)
  • packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts (1 hunks)
  • packages/flash-loans/src/aave/const.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alfetopito
Repo: cowprotocol/cow-sdk PR: 391
File: src/trading/getEthFlowTransaction.ts:80-85
Timestamp: 2025-08-12T09:15:28.459Z
Learning: In the CoW SDK codebase, ETH_FLOW_ADDRESSES and BARN_ETH_FLOW_ADDRESSES are typed as Record<SupportedChainId, string>, which requires all SupportedChainId enum values to have corresponding string entries. TypeScript compilation will fail if any chainId is missing from these mappings, making runtime guards for missing addresses unnecessary in these specific cases.
📚 Learning: 2025-08-12T09:15:28.459Z
Learnt from: alfetopito
Repo: cowprotocol/cow-sdk PR: 391
File: src/trading/getEthFlowTransaction.ts:80-85
Timestamp: 2025-08-12T09:15:28.459Z
Learning: In the CoW SDK codebase, ETH_FLOW_ADDRESSES and BARN_ETH_FLOW_ADDRESSES are typed as Record<SupportedChainId, string>, which requires all SupportedChainId enum values to have corresponding string entries. TypeScript compilation will fail if any chainId is missing from these mappings, making runtime guards for missing addresses unnecessary in these specific cases.

Applied to files:

  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts
  • packages/flash-loans/src/aave/const.ts
🧬 Code graph analysis (2)
packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (1)
packages/flash-loans/src/aave/const.ts (1)
  • AAVE_ADAPTER_FACTORY (26-31)
packages/flash-loans/src/aave/const.ts (1)
packages/config/src/chains/const/utils.ts (1)
  • mapAddressToSupportedNetworks (16-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Publish to GitHub Packages
  • GitHub Check: test
  • GitHub Check: Build Package
  • GitHub Check: eslint
🔇 Additional comments (5)
packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts (1)

169-180: LGTM! ABI metadata improvements.

The additions of explicit type, inputs, and internalType fields make the DOMAIN_SEPARATOR ABI entry more complete and consistent with TypeScript typing conventions. No functional changes.

packages/flash-loans/src/aave/const.ts (3)

33-50: LGTM! Well-structured adapter hook constants.

The three adapter hook constants follow a consistent pattern with typed Record<SupportedChainId, string> mappings. The addresses for GNOSIS_CHAIN, MAINNET, and BASE match the PR objectives. The use of mapAddressToSupportedNetworks('') ensures all chain IDs are covered while keeping the supported chains explicit.

Based on learnings: The Record<SupportedChainId, string> typing ensures TypeScript compilation will fail if any chainId is missing, making these mappings type-safe.


12-25: Question: Intentional partial chain support?

I notice that AAVE_POOL_ADDRESS includes addresses for chains like ARBITRUM_ONE, AVALANCHE, BNB, POLYGON, LINEA, and PLASMA, but the corresponding AAVE_ADAPTER_FACTORY and adapter hook constants only support GNOSIS_CHAIN, MAINNET, and BASE. Is this intentional partial support, or are the other chains planned for future PRs?


12-25: AAVE pool addresses verified successfully.

All three pool addresses match official Aave V3 documentation: Gnosis Chain (0xb50201558B00496A145fE76f7424749556E326D8), Ethereum Mainnet (0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2), and Base (0xA238Dd80C259a72e81d7e4664a9801593F98d1c5). The addresses are properly scoped and referenced in tests.

packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (1)

95-106: Addresses verified—chainId migration to GNOSIS_CHAIN is correct.

The test correctly references AAVE_ADAPTER_FACTORY[SupportedChainId.GNOSIS_CHAIN] (address 0x7f230F7Cee38ca371b871B74B3C6ded4932A2f93) at all four locations: lines 215, 250, 301, and 650. The defined address in const.ts matches all test usages, and the migration from SEPOLIA to GNOSIS_CHAIN aligns with the officially supported Aave V3 deployment on Gnosis Chain.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 756daae and e50146a.

📒 Files selected for processing (1)
  • packages/flash-loans/src/aave/const.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alfetopito
Repo: cowprotocol/cow-sdk PR: 391
File: src/trading/getEthFlowTransaction.ts:80-85
Timestamp: 2025-08-12T09:15:28.459Z
Learning: In the CoW SDK codebase, ETH_FLOW_ADDRESSES and BARN_ETH_FLOW_ADDRESSES are typed as Record<SupportedChainId, string>, which requires all SupportedChainId enum values to have corresponding string entries. TypeScript compilation will fail if any chainId is missing from these mappings, making runtime guards for missing addresses unnecessary in these specific cases.
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 642
File: packages/bridging/src/BridgingSdk/BridgingSdk.ts:124-144
Timestamp: 2025-11-06T11:26:21.337Z
Learning: In the cow-sdk bridging module (packages/bridging/), all bridge providers return ChainInfo objects that are singleton instances imported from cowprotocol/sdk-config. The chain objects (mainnet, arbitrumOne, base, optimism, polygon, avalanche, gnosisChain, etc.) are exported as constants from packages/config/src/chains/details/ and are shared across all providers. This singleton pattern makes object reference comparison (e.g., with Array.includes()) safe for deduplicating networks returned by multiple providers, as the same chain will always be represented by the same object reference.
<!--
📚 Learning: 2025-08-12T09:15:28.459Z
Learnt from: alfetopito
Repo: cowprotocol/cow-sdk PR: 391
File: src/trading/getEthFlowTransaction.ts:80-85
Timestamp: 2025-08-12T09:15:28.459Z
Learning: In the CoW SDK codebase, ETH_FLOW_ADDRESSES and BARN_ETH_FLOW_ADDRESSES are typed as Record<SupportedChainId, string>, which requires all SupportedChainId enum values to have corresponding string entries. TypeScript compilation will fail if any chainId is missing from these mappings, making runtime guards for missing addresses unnecessary in these specific cases.

Applied to files:

  • packages/flash-loans/src/aave/const.ts
🧬 Code graph analysis (1)
packages/flash-loans/src/aave/const.ts (1)
packages/config/src/chains/const/utils.ts (1)
  • mapAddressToSupportedNetworks (16-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Publish to GitHub Packages
  • GitHub Check: eslint
🔇 Additional comments (3)
packages/flash-loans/src/aave/const.ts (3)

26-50: LGTM! Type-safe per-chain adapter mappings.

The adapter factory and hook constants are properly structured as Record<SupportedChainId, string>, which ensures compile-time completeness for all supported chains. The pattern of using mapAddressToSupportedNetworks('') with chain-specific overrides is type-safe and consistent across all adapter constants. The deployed addresses match those documented in the PR description.

Based on learnings.


19-22: Addresses verified as correct—no changes needed.

The Aave V3 Pool address (0x794a61358D6845594F94dc1DB02A252b5b4814aD) is correctly deployed identically across Arbitrum One, Avalanche, and Polygon. The code is accurate.


13-25: The original review comment mischaracterizes the code structure and should be disregarded.

The analysis incorrectly states that AAVE_ADAPTER_FACTORY "only provides addresses for GNOSIS_CHAIN, MAINNET, and BASE" implying missing chains. In reality, the mapAddressToSupportedNetworks('') spread operator ensures all 11 SupportedChainId values are present in the Record, per TypeScript's requirement. Only 3 chains have non-empty adapter addresses; the rest have empty strings.

The actual concern (empty string handling) is valid but was misidentified. AAVE_ADAPTER_FACTORY has empty strings for 8 chains (ARBITRUM_ONE, AVALANCHE, BNB, POLYGON, LINEA, PLASMA, SEPOLIA, LENS), and these empty strings are used directly at lines 224–226 without validation.

However, this is a code review issue for the calling code, not the constants file itself—the constants are correctly structured per the codebase patterns.

Likely an incorrect or invalid review comment.

@shoom3301 shoom3301 merged commit c7f2327 into main Nov 7, 2025
9 checks passed
@shoom3301 shoom3301 deleted the feat/aave-contract-addresses-1 branch November 7, 2025 11:30
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants