Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(migration): remove unnecessary network-specific instanciation #328

Closed
wants to merge 4 commits into from

Conversation

Rubilmax
Copy link
Collaborator

So I first went to make migration bundlers abstract and re-instanciate them in ethereum folder but it's simpler to check the address in the constructor or simply don't check anything so I removed AaveV2EthereumMigrationBundler

@Rubilmax Rubilmax marked this pull request as ready for review October 25, 2023 13:41
@Rubilmax Rubilmax linked an issue Oct 25, 2023 that may be closed by this pull request
@MerlinEgalite
Copy link
Contributor

Tests are failing...

@Rubilmax Rubilmax force-pushed the refactor/migration-constructor branch from 2eebb28 to 90ef4bd Compare October 25, 2023 14:15
@MerlinEgalite
Copy link
Contributor

Tests are failing

@Rubilmax Rubilmax self-assigned this Oct 31, 2023
@Rubilmax
Copy link
Collaborator Author

Tests are failing

and i don't know why...

import {MigrationBundler, ERC20} from "./MigrationBundler.sol";

/// @title AaveV2MigrationBundler
/// @author Morpho Labs
/// @custom:contact security@morpho.org
/// @notice Contract allowing to migrate a position from Aave V2 to Morpho Blue easily.
/// If deploying to Ethereum, deploy `AaveV2EthereumMigrationBundler` instead.
contract AaveV2MigrationBundler is MigrationBundler {
contract AaveV2MigrationBundler is MigrationBundler, StEthBundler {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the goal of this PR is to harmonize things: either have all migration bundlers instances defined in ethereum, or have no migration bundler defined in ethereum

The previous state where only the AaveV2 migration bundler was defined with its parameters as constants is inconsistent with other migration bundlers

@MerlinEgalite
Copy link
Contributor

rerunning jobs

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename this file?

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.

AaveV2MigrationBundler & CompoundV2MigrationBundler are not abstract
3 participants