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

Add handling for max uint256 amount in withdrawERC20 #333

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions script/migrations/145-upgrade_access_protocol.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ contract DeployScript is Script, MigrationHelper {
registry.allowWorkflow(address(aaveRepayWorkflow));

registry.disallowWorkflow(_getChainDeployment("AaveWithdrawWorkflow"));
AaveWithdrawWorkflow aaveWithdrawWorkflow =
new AaveWithdrawWorkflow(getAavePoolProvider(), _getChainDeployment("Bridger"));
AaveWithdrawWorkflow aaveWithdrawWorkflow = new AaveWithdrawWorkflow(
getAavePoolProvider(), _getChainDeployment("Bridger"), getMamoriSafeByChainId(block.chainid)
);
saveContractAddress("AaveWithdrawWorkflow", address(aaveWithdrawWorkflow));
registry.allowWorkflow(address(aaveWithdrawWorkflow));

Expand Down
15 changes: 12 additions & 3 deletions src/access/workflows/AaveWithdrawWorkflow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,21 @@ contract AaveWithdrawWorkflow {
IPoolAddressesProvider public immutable poolAddressProvider;
/// @notice Address of the Bridger contract
IBridger public immutable bridger;
/// @notice Address of the Safe contract
address public immutable safe;
/// @notice Fee charged upon withdrawal. 10bps.
uint256 public constant FEE = 1e15;

/* ============ Constructor ============ */

/**
* @notice Initializes the contract with Aave's pool address provider
* @param poolAddressProvider_ The address of Aave's pool address provider
*/
constructor(address poolAddressProvider_, address bridger_) {
constructor(address poolAddressProvider_, address bridger_, address safe_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

to change the fee we will just upgrade?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. We can make it a param on a AccessManager.

poolAddressProvider = IPoolAddressesProvider(poolAddressProvider_);
bridger = IBridger(bridger_);
safe = safe_;
}

/* ============ External Functions ============ */
Expand All @@ -42,7 +47,7 @@ contract AaveWithdrawWorkflow {
* @param asset The address of the asset to withdraw
* @param amount The amount to withdraw (use type(uint256).max for max available)
*/
function withdraw(address asset, uint256 amount) public {
function withdraw(address asset, uint256 amount) public returns (uint256) {
address pool = poolAddressProvider.getPool();

// If amount is max uint256, withdraw all available
Expand All @@ -52,6 +57,10 @@ contract AaveWithdrawWorkflow {

// Withdraw from Aave
IAavePool(pool).withdraw(asset, amount, address(this));
// Send the fee to the Safe
uint256 fee = amount * FEE / 1e18;
IERC20(asset).transfer(safe, fee);
return amount - fee;
}

function withdrawAndBridge(
Expand All @@ -60,7 +69,7 @@ contract AaveWithdrawWorkflow {
address kintoWallet,
IBridger.BridgeData calldata bridgeData
) external payable returns (uint256 amountOut) {
withdraw(asset, amount);
amount = withdraw(asset, amount);

// Approve max allowance to save on gas for future transfers
if (IERC20(asset).allowance(address(this), address(bridger)) < amount) {
Expand Down
2 changes: 1 addition & 1 deletion test/artifacts/42161/addresses.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"Viewer": "0x8888886e1d7c1468d7300cF08db89FFE68F29830",
"Viewer-impl": "0x80338A3f75614491c8DC383fFaA663b9a27CD05d",
"WethWorkflow": "0x7F7c594eE170a62d7e7615972831038Cf7d4Fc1A",
"WithdrawWorkflow": "0xbc22c860C1ED7330271eeF19FB47Eb08548f1723",
"WithdrawWorkflow": "0x794E1908A1D41760B8E2b798134c9856E24dCe65",
"AaveLendWorkflow": "0xB47Ed636c8296729E81463109FEbf833CeEa71fb",
"AaveRepayWorkflow": "0x24f71379C39b515Ff5182F4b0cc298793EC5998c",
"AaveWithdrawWorkflow": "0xef4D6687372172c4af1802C208Ab40673b014309",
Expand Down
10 changes: 6 additions & 4 deletions test/fork/workflows/AaveWithdrawWorkflow.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ contract AaveWithdrawWorkflowTest is SignatureHelper, ForkTest, ArtifactsReader,
IAccessPoint internal accessPoint;
AaveWithdrawWorkflow internal aaveWithdrawWorkflow;
IAavePool internal aavePool;
address internal safe = address(0x5afe);
uint256 constant FEE = 1e15;

function setUp() public override {
super.setUp();
Expand All @@ -50,7 +52,7 @@ contract AaveWithdrawWorkflowTest is SignatureHelper, ForkTest, ArtifactsReader,
accessPoint = accessRegistry.deployFor(address(alice0));
vm.label(address(accessPoint), "accessPoint");

aaveWithdrawWorkflow = new AaveWithdrawWorkflow(ARB_AAVE_POOL_PROVIDER, address(bridger));
aaveWithdrawWorkflow = new AaveWithdrawWorkflow(ARB_AAVE_POOL_PROVIDER, address(bridger), safe);
vm.label(address(aaveWithdrawWorkflow), "aaveWithdrawWorkflow");

vm.prank(accessRegistry.owner());
Expand Down Expand Up @@ -89,7 +91,7 @@ contract AaveWithdrawWorkflowTest is SignatureHelper, ForkTest, ArtifactsReader,
// Assert balances changed correctly
assertEq(
IERC20(assetToWithdraw).balanceOf(address(accessPoint)),
initialAccessPointBalance + amountToWithdraw,
initialAccessPointBalance + amountToWithdraw - amountToWithdraw * FEE / 1e18,
"Invalid USDC balance"
);
assertEq(
Expand Down Expand Up @@ -126,7 +128,7 @@ contract AaveWithdrawWorkflowTest is SignatureHelper, ForkTest, ArtifactsReader,
// Assert balances changed correctly
assertEq(
IERC20(assetToWithdraw).balanceOf(address(accessPoint)),
initialAccessPointBalance + amountToSupply,
initialAccessPointBalance + amountToSupply - amountToSupply * FEE / 1e18,
"Invalid USDC balance"
);
assertEq(IERC20(aToken).balanceOf(address(accessPoint)), 0, "Invalid aToken balance");
Expand Down Expand Up @@ -175,7 +177,7 @@ contract AaveWithdrawWorkflowTest is SignatureHelper, ForkTest, ArtifactsReader,
assertEq(IERC20(assetToWithdraw).balanceOf(address(bridger)), initialBridgerBalance, "Invalid bridger balance");
assertEq(
IERC20(assetToWithdraw).balanceOf(address(bridgeData.vault)),
initialVaultBalance + amountToWithdraw,
initialVaultBalance + amountToWithdraw - amountToWithdraw * FEE / 1e18,
"Invalid vault balance"
);
}
Expand Down
Loading