Skip to content

feat(defi): add Aave V3 vault deposits - #9674

Open
ralph-bitgo[bot] wants to merge 1 commit into
masterfrom
DEFI-857-aave-v3-sdk-plumbing
Open

feat(defi): add Aave V3 vault deposits#9674
ralph-bitgo[bot] wants to merge 1 commit into
masterfrom
DEFI-857-aave-v3-sdk-plumbing

Conversation

@ralph-bitgo

@ralph-bitgo ralph-bitgo Bot commented Sep 8, 2026

Copy link
Copy Markdown

What

  • Upgrade @bitgo/public-types to 6.69.0 in sdk-core, which includes
    VaultProtocol.AAVE_V3 and its codec union.
  • Add aave_v3 SDK vault dispatch. Morpho and Aave V3 share the same
    ERC-4626 approve/deposit flow, so both dispatch to a single
    depositToErc4626Vault path that creates the existing defiApprove
    and defiDeposit txRequest pair.
  • Extend DepositResult and add full/lite operation ID, missing ID, and
    failure-propagation coverage.

Why

The SDK rejected Aave StataTokenV2 vaults as an unsupported protocol before
it could create deposit txRequests. Reusing the existing ERC-4626 flow lets
SDK clients deposit into Aave vaults without introducing new calldata or
wallet intent types.

Ticket: DEFI-857

@ralph-bitgo
ralph-bitgo Bot force-pushed the DEFI-857-aave-v3-sdk-plumbing branch from 491f65c to 77c23bd Compare September 8, 2026 07:28
@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

DEFI-857

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

@ralph-bitgo
ralph-bitgo Bot force-pushed the DEFI-857-aave-v3-sdk-plumbing branch 2 times, most recently from ef37c25 to 9fb23bd Compare September 8, 2026 07:52
Add AAVE_V3 vault dispatch to sdk-core. Morpho and Aave V3 share the
same ERC-4626 approve/deposit sequencing, so both protocols dispatch
to a single depositToErc4626Vault path that issues the defiApprove and
defiDeposit sendMany pair and returns the operationId linking them,
with full and lite operation ID extraction coverage.

Upgrade @bitgo/public-types to 6.69.0 in sdk-core, the only module
consuming VaultProtocol.AAVE_V3, so the SDK accepts the published Aave
protocol codec and clients can deposit into StataTokenV2 vaults
instead of receiving an unsupported protocol error.

Ticket: DEFI-857
Session-Id: 133a26bb-c1ee-4f99-886a-f4f1b1b017e5
Task-Id: 3624de8d-630a-454d-ba30230221e2
@sachinBitgo
sachinBitgo force-pushed the DEFI-857-aave-v3-sdk-plumbing branch from a27029a to aaf7b8d Compare September 8, 2026 08:39
@sachinBitgo
sachinBitgo marked this pull request as ready for review September 8, 2026 10:00
@sachinBitgo
sachinBitgo requested review from a team as code owners September 8, 2026 10:00
@sachinBitgo

Copy link
Copy Markdown
Contributor

@claude review

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sachinBitgo's task —— View job


Code Review for Aave V3 Vault Deposits

Todo List:

  • Read and analyze all changed files
  • Review package.json dependency update
  • Review defiVault.ts implementation changes
  • Review iDefiVault.ts interface changes
  • Review test coverage additions
  • Check yarn.lock for dependency conflicts
  • Provide comprehensive review feedback

Overall Assessment ✅

This PR successfully adds Aave V3 vault deposit support by reusing the existing ERC-4626 flow. The implementation is clean, well-tested, and follows established patterns in the codebase.

Detailed Review

1. Dependency Update (package.json)

✅ Good

  • Clean upgrade of @bitgo/public-types from 6.66.0 to 6.69.0
  • This brings in the required VaultProtocol.AAVE_V3 constant and codec support

2. Core Implementation (defiVault.ts)

✅ Excellent Design Decision

  • Smart reuse: Instead of duplicating code, Aave V3 shares the ERC-4626 flow with Morpho via depositToErc4626Vault()
  • Minimal changes: The dispatch logic at defiVault.ts:129 elegantly handles both protocols:
    } else if (protocol === VaultProtocol.MORPHO || protocol === VaultProtocol.AAVE_V3) {
      return this.depositToErc4626Vault(params);
  • Clear refactoring: depositToMorphoVault()depositToErc4626Vault() with updated documentation
  • Backward compatibility: The change maintains the exact same API and behavior

Minor Observations:

  • Removed unused MorphoDepositResult import - good cleanup
  • Updated comments consistently throughout to reflect the new Aave V3 support

3. Interface Extensions (iDefiVault.ts)

✅ Proper Type Safety

  • Added AaveDepositResult interface that matches MorphoDepositResult structure
  • Extended DepositResult union type to include AaveDepositResult
  • This provides proper TypeScript support while maintaining runtime compatibility

4. Test Coverage (test/unit/bitgo/defi/defiVault.ts)

✅ Comprehensive Testing

  • 118 new lines of test coverage for Aave V3 functionality
  • Tests cover all critical paths:
    • Happy path deposit flow (lines 462-503)
    • Operation ID extraction from both API versions (lines 505-525)
    • Error handling for missing operation ID (lines 527-542)
    • Failure propagation without cleanup (lines 544-561)
  • Excellent test structure: Tests mirror existing Morpho tests, ensuring consistency
  • Realistic mocking: Tests use proper makeAaveVault() helper with realistic data

Specific test highlights:

  • lines 467-480: Proper mock setup for full apiVersion responses
  • lines 493-502: Validates correct defiApprove/defiDeposit parameters
  • lines 510-517: Tests lite apiVersion operationId extraction path

5. Dependency Management (yarn.lock)

✅ Clean Update

  • Only adds necessary entries for @bitgo/public-types@6.69.0
  • No unexpected transitive dependency changes

Security & Best Practices Review

✅ Security: No security concerns identified

  • Reuses existing, battle-tested ERC-4626 approve/deposit flow
  • No new external dependencies or attack vectors
  • Proper input validation maintained through existing code paths

✅ Error Handling: Robust error propagation

  • Maintains existing error handling patterns
  • Tests verify proper error scenarios (missing operationId, sendMany failures)

✅ Code Quality:

  • Follows established naming conventions
  • Clear, self-documenting code with updated comments
  • No magic numbers or hardcoded values

Potential Areas for Future Improvement

  1. Type Union Refinement (Optional): The DepositResult union could potentially use discriminated unions for better TypeScript inference, though current approach is perfectly functional.

  2. Test Organization (Minor): Consider extracting shared test helpers between Morpho and Aave test suites to reduce duplication.

Conclusion

Recommendation: ✅ APPROVE

This PR demonstrates excellent software engineering practices:

  • DRY principle: Reuses existing infrastructure rather than duplicating
  • Minimal surface area: Changes are focused and surgical
  • Comprehensive testing: New functionality is thoroughly tested
  • Clear documentation: Comments and naming accurately reflect changes
  • Type safety: Proper TypeScript support added

The implementation correctly addresses the stated goal of allowing Aave StataTokenV2 vault deposits without introducing new complexity. The reuse of the ERC-4626 flow is a smart architectural decision that reduces maintenance burden while providing the required functionality.


Note: The CI failure on Node 26.x is marked as non-blocking since Node 26 is not yet LTS, as mentioned in the GitHub Actions comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant