Skip to content

Conversation

@mgrabina
Copy link
Contributor

@mgrabina mgrabina commented Nov 3, 2025

passing hooks dappid already added into registry -> aave://flashloans/v3/repay

Summary by CodeRabbit

  • Refactor
    • Hook payloads for Aave flash loans now include the appropriate dApp identifiers for each flash loan type.
    • Internal wiring updated to propagate these identifiers; no changes to public APIs or exported interfaces and consumer behavior remains unchanged.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds a new public readonly mapping from Aave flash loan types to dApp IDs and updates order hook preparation so getOrderHooks derives the appropriate dappId and injects it into both pre-hook and post-hook call data during order processing.

Changes

Cohort / File(s) Change Summary
Public dApp ID mapping & hook injection
packages/flash-loans/src/aave/AaveCollateralSwapSdk.ts
Added public readonly AAVE_DAPP_ID_PER_TYPE: Record<AaveFlashLoanType, string>; updated getOrderHooks to derive dappId = AAVE_DAPP_ID_PER_TYPE[flashLoanType] and include that dappId in pre-hook and post-hook callData payloads.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant AaveCollateralSwapSdk
  participant PreHook
  participant PostHook

  Note over AaveCollateralSwapSdk: getOrderHooks(flashLoanType, ...)
  Caller->>AaveCollateralSwapSdk: request order hooks
  AaveCollateralSwapSdk->>AaveCollateralSwapSdk: dappId = AAVE_DAPP_ID_PER_TYPE[flashLoanType]
  AaveCollateralSwapSdk->>PreHook: call with callData { ..., dappId }
  PreHook-->>AaveCollateralSwapSdk: pre-hook result
  AaveCollateralSwapSdk->>PostHook: call with callData { ..., dappId }
  PostHook-->>AaveCollateralSwapSdk: post-hook result
  AaveCollateralSwapSdk-->>Caller: assembled hooks with injected dappId
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file change; small, localized logic addition.
  • Review focus: correctness of mapping entries, safe use of public readonly field, and proper inclusion of dappId in pre/post callData.

Poem

🐇 A little map tucked in the code,
dApp IDs ready down the road,
Pre-hook, post-hook — all in line,
Hopping data where it must bind,
Swap and loan, a tidy rhyme. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding an Aave hook dappId into hooks info app data, which directly aligns with the changeset modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 3964b6d and ffb4076.

📒 Files selected for processing (1)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/flash-loans/src/aave/AaveCollateralSwapSdk.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). (3)
  • 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.

@anxolin anxolin requested a review from a team November 4, 2025 13:42
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

Looks good, just the naming convention doesn't match the other ones

Comment on lines +498 to +506
dappId,
},
],
post: [
{
target: expectedInstanceAddress,
callData: postHookCallData,
gasLimit: DEFAULT_HOOK_GAS_LIMIT.post.toString(),
dappId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for the same type of hook to be set for both pre and post hooks at the same time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can u expand?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess he meant, for the same "flow / app" to add both pre and post hook in the same order. And the answer is yes. These orders have the 2 hooks.

])
}

readonly AAVE_DAPP_ID_PER_TYPE: Record<AaveFlashLoanType, string> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: it should be a static field or a const. No reason to make it an instance field

@shoom3301
Copy link
Contributor

Reopened: #645

@shoom3301 shoom3301 closed this Nov 5, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 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