Skip to content

Conversation

@mgrabina
Copy link
Contributor

@mgrabina mgrabina commented Nov 6, 2025

Summary by CodeRabbit

  • Chores
    • Restructured multi-chain address mappings for broader network compatibility.
    • Expanded contract interfaces with enhanced error handling and new operational functions.
    • Updated test infrastructure for comprehensive multi-network validation.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 7cc2fd0 and ba31822.

⛔ Files ignored due to path filters (1)
  • packages/contracts-ts/src/generated/packageVersion.ts is excluded by !**/generated/**
📒 Files selected for processing (6)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (19 hunks)
  • packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts (3 hunks)
  • packages/flash-loans/src/aave/abi/CollateralSwapAdapterHook.ts (2 hunks)
  • packages/flash-loans/src/aave/abi/DebtSwapAdapter.ts (2 hunks)
  • packages/flash-loans/src/aave/abi/RepayWithCollateralAdapter.ts (2 hunks)
  • packages/flash-loans/src/aave/const.ts (1 hunks)
 _________________________
< Will debug for carrots. >
 -------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

Tip

CodeRabbit can use oxc to improve the quality of JavaScript and TypeScript code reviews.

Add a configuration file to your project to customize how CodeRabbit runs oxc.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@mgrabina mgrabina changed the base branch from main to feat/aave-contract-addresses November 6, 2025 16:07
@carlos-cow carlos-cow merged commit 0e9b3ad into cowprotocol:feat/aave-contract-addresses Nov 6, 2025
8 of 9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 6, 2025
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 7cc2fd0 and ba31822.

⛔ Files ignored due to path filters (1)
  • packages/contracts-ts/src/generated/packageVersion.ts is excluded by !**/generated/**
📒 Files selected for processing (6)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.test.ts (19 hunks)
  • packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts (3 hunks)
  • packages/flash-loans/src/aave/abi/CollateralSwapAdapterHook.ts (2 hunks)
  • packages/flash-loans/src/aave/abi/DebtSwapAdapter.ts (2 hunks)
  • packages/flash-loans/src/aave/abi/RepayWithCollateralAdapter.ts (2 hunks)
  • packages/flash-loans/src/aave/const.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 498
File: packages/sdk/README.md:143-177
Timestamp: 2025-09-16T07:22:55.962Z
Learning: In the CoW Protocol SDK v7 migration guide, the wagmi integration example requires type casting (`as unknown as ViemAdapterOptions`) when creating a ViemAdapter with wagmi's usePublicClient() and useWalletClient() hooks because wagmi's returned types are not directly compatible with ViemAdapterOptions despite being structurally similar. This casting is necessary and used throughout the codebase for similar type compatibility issues.
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 498
File: packages/sdk/README.md:143-177
Timestamp: 2025-09-16T07:22:55.962Z
Learning: In the CoW Protocol SDK v7 migration guide, the wagmi integration example requires type casting (`as unknown as ViemAdapterOptions`) when creating a ViemAdapter with wagmi's usePublicClient() and useWalletClient() hooks because wagmi's returned types are not directly compatible with ViemAdapterOptions despite being structurally similar.
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/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). (3)
  • GitHub Check: Build Package
  • GitHub Check: eslint
  • GitHub Check: test
🔇 Additional comments (1)
packages/flash-loans/src/aave/const.ts (1)

13-25: Fill missing chain entries in AAVE_POOL_ADDRESS.

By removing the mapAddressToSupportedNetworks('') spread the lookup now returns undefined for any SupportedChainId that is not explicitly listed (e.g., SupportedChainId.OPTIMISM, which we still expose elsewhere in the SDK). When downstream code hands that value to viem it will fail with “invalid address: undefined”. Please restore the default map (or add explicit entries for every chain) before overriding the chains we truly support.

 export const AAVE_POOL_ADDRESS: Record<SupportedChainId, string> = {
-  [SupportedChainId.SEPOLIA]: '',
-  [SupportedChainId.LENS]: '',
-  [SupportedChainId.GNOSIS_CHAIN]: '0xb50201558B00496A145fE76f7424749556E326D8',
-  [SupportedChainId.MAINNET]: '0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2',
-  [SupportedChainId.BASE]: '0xA238Dd80C259a72e81d7e4664a9801593F98d1c5',
-  [SupportedChainId.ARBITRUM_ONE]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
-  [SupportedChainId.AVALANCHE]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
-  [SupportedChainId.BNB]: '0x6807dc923806fE8Fd134338EABCA509979a7e0cB',
-  [SupportedChainId.POLYGON]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
-  [SupportedChainId.LINEA]: '0xc47b8C00b0f69a36fa203Ffeac0334874574a8Ac',
-  [SupportedChainId.PLASMA]: '0x925a2A7214Ed92428B5b1B090F80b25700095e12',
-}
+  ...mapAddressToSupportedNetworks(''),
+  [SupportedChainId.GNOSIS_CHAIN]: '0xb50201558B00496A145fE76f7424749556E326D8',
+  [SupportedChainId.MAINNET]: '0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2',
+  [SupportedChainId.BASE]: '0xA238Dd80C259a72e81d7e4664a9801593F98d1c5',
+  [SupportedChainId.ARBITRUM_ONE]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
+  [SupportedChainId.AVALANCHE]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
+  [SupportedChainId.BNB]: '0x6807dc923806fE8Fd134338EABCA509979a7e0cB',
+  [SupportedChainId.POLYGON]: '0x794a61358D6845594F94dc1DB02A252b5b4814aD',
+  [SupportedChainId.LINEA]: '0xc47b8C00b0f69a36fa203Ffeac0334874574a8Ac',
+  [SupportedChainId.PLASMA]: '0x925a2A7214Ed92428B5b1B090F80b25700095e12',
+}

Based on learnings

⛔ Skipped due to 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.

Comment on lines +498 to +560
name: 'order',
type: 'tuple',
internalType: 'struct GPv2Order.Data',
components: [
{
name: 'sellToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'buyToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'receiver',
type: 'address',
internalType: 'address',
},
{
name: 'sellAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'buyAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'validTo',
type: 'uint32',
internalType: 'uint32',
},
{
name: 'appData',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'feeAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'kind',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'partiallyFillable',
type: 'bool',
internalType: 'bool',
},
{
name: 'sellTokenBalance',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'buyTokenBalance',
type: 'bytes32',
internalType: 'bytes32',
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect enum ABI types in GPv2 order tuple

GPv2Order.Data.kind, sellTokenBalance, and buyTokenBalance are Solidity enums compiled as uint8. Encoding them as bytes32 here will produce the wrong calldata layout, so verifyOrderSignature calls will revert or decode garbage. Please align the ABI with the on-chain struct.

-          {
-            name: 'kind',
-            type: 'bytes32',
-            internalType: 'bytes32',
-          },
+          {
+            name: 'kind',
+            type: 'uint8',
+            internalType: 'enum GPv2Order.Kind',
+          },
...
-          {
-            name: 'sellTokenBalance',
-            type: 'bytes32',
-            internalType: 'bytes32',
-          },
-          {
-            name: 'buyTokenBalance',
-            type: 'bytes32',
-            internalType: 'bytes32',
-          },
+          {
+            name: 'sellTokenBalance',
+            type: 'uint8',
+            internalType: 'enum GPv2Order.Balance',
+          },
+          {
+            name: 'buyTokenBalance',
+            type: 'uint8',
+            internalType: 'enum GPv2Order.Balance',
+          },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name: 'order',
type: 'tuple',
internalType: 'struct GPv2Order.Data',
components: [
{
name: 'sellToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'buyToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'receiver',
type: 'address',
internalType: 'address',
},
{
name: 'sellAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'buyAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'validTo',
type: 'uint32',
internalType: 'uint32',
},
{
name: 'appData',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'feeAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'kind',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'partiallyFillable',
type: 'bool',
internalType: 'bool',
},
{
name: 'sellTokenBalance',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'buyTokenBalance',
type: 'bytes32',
internalType: 'bytes32',
name: 'order',
type: 'tuple',
internalType: 'struct GPv2Order.Data',
components: [
{
name: 'sellToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'buyToken',
type: 'address',
internalType: 'contract IERC20',
},
{
name: 'receiver',
type: 'address',
internalType: 'address',
},
{
name: 'sellAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'buyAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'validTo',
type: 'uint32',
internalType: 'uint32',
},
{
name: 'appData',
type: 'bytes32',
internalType: 'bytes32',
},
{
name: 'feeAmount',
type: 'uint256',
internalType: 'uint256',
},
{
name: 'kind',
type: 'uint8',
internalType: 'enum GPv2Order.Kind',
},
{
name: 'partiallyFillable',
type: 'bool',
internalType: 'bool',
},
{
name: 'sellTokenBalance',
type: 'uint8',
internalType: 'enum GPv2Order.Balance',
},
{
name: 'buyTokenBalance',
type: 'uint8',
internalType: 'enum GPv2Order.Balance',
},
🤖 Prompt for AI Agents
In packages/flash-loans/src/aave/abi/AaveAdapterFactory.ts around lines 498 to
560, the tuple components for GPv2Order.Data currently declare kind,
sellTokenBalance, and buyTokenBalance as type/internalType 'bytes32'; these are
Solidity enums compiled as uint8 and must be encoded as uint8 to match on-chain
layout. Change those three components to type: 'uint8' and internalType: 'uint8'
(keeping the same field names and ordering) so the ABI matches the contract
struct and calldata/decoding is correct.

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.

2 participants