Skip to content

Commit

Permalink
feat: make Transfer.sol a library and improve usage (#649)
Browse files Browse the repository at this point in the history 
* feat: make Transfer.sol a library and remove functions

* test: fix tests

* refactor: remove virtual from library function

* fix: custom transfer function

* feat: add back transferAmountFrom and remove SafeTransferLib

* refactor: fix library usage

* fix: library usage and balance check
  • Loading branch information
@ilpepepig
ilpepepig authored Aug 27, 2024
1 parent 5bae660 commit d4ab757
Show file tree
Hide file tree
Showing 34 changed files with 272 additions and 224 deletions.
66 changes: 24 additions & 42 deletions contracts/core/Allo.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import "openzeppelin-contracts-upgradeable/contracts/access/AccessControlUpgrade
import "openzeppelin-contracts-upgradeable/contracts/security/ReentrancyGuardUpgradeable.sol";
// Interfaces
import "./interfaces/IAllo.sol";

// Internal Libraries
import {Clone} from "./libraries/Clone.sol";
import {Errors} from "./libraries/Errors.sol";
import "./libraries/Native.sol";
import {Native} from "./libraries/Native.sol";
import {Transfer} from "./libraries/Transfer.sol";

// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣾⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣷⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
Expand All @@ -35,16 +34,9 @@ import {Transfer} from "./libraries/Transfer.sol";
/// @author @thelostone-mc <[email protected]>, @0xKurt <[email protected]>, @codenamejason <[email protected]>, @0xZakk <[email protected]>, @nfrgosselin <[email protected]>
/// @notice This contract is used to create & manage pools as well as manage the protocol.
/// @dev The contract must be initialized with the 'initialize()' function.
contract Allo is
IAllo,
Native,
Transfer,
Initializable,
Ownable,
AccessControlUpgradeable,
ReentrancyGuardUpgradeable,
Errors
{
contract Allo is IAllo, Native, Initializable, Ownable, AccessControlUpgradeable, ReentrancyGuardUpgradeable, Errors {
using Transfer for address;

// ==========================
// === Storage Variables ====
// ==========================
Expand Down Expand Up @@ -333,7 +325,7 @@ contract Allo is
uint256 amount = _token == NATIVE ? address(this).balance : IERC20Upgradeable(_token).balanceOf(address(this));

// Transfer the amount to the recipient (pool owner)
_transferAmount(_token, _recipient, amount);
_token.transferAmount(_recipient, amount);
}

// ====================================
Expand Down Expand Up @@ -569,10 +561,11 @@ contract Allo is
// To prevent paying the baseFee from the Allo contract's balance
// If _token is NATIVE, then baseFee + _amount should be equal to _msgValue.
// If _token is not NATIVE, then baseFee should be equal to _msgValue.
if ((_token == NATIVE && (baseFee + _amount != _msgValue)) || (_token != NATIVE && baseFee != _msgValue)) {
revert NOT_ENOUGH_FUNDS();
}
_transferAmount(NATIVE, treasury, baseFee);
if (_token == NATIVE && (baseFee + _amount != _msgValue)) revert NOT_ENOUGH_FUNDS();
if (_token != NATIVE && baseFee != _msgValue) revert NOT_ENOUGH_FUNDS();

address(treasury).transferAmountNative(baseFee);

emit BaseFeePaid(poolId, baseFee);
}

Expand Down Expand Up @@ -611,39 +604,28 @@ contract Allo is
/// @param _poolId The 'poolId' for the pool you are funding
/// @param _strategy The address of the strategy
function _fundPool(uint256 _amount, address _funder, uint256 _poolId, IBaseStrategy _strategy) internal virtual {
uint256 feeAmount;
uint256 amountAfterFee = _amount;
uint256 feeAmount = (_amount * percentFee) / getFeeDenominator(); // Can be zero if percentFee is zero
uint256 amountAfterFee = _amount - feeAmount;

Pool storage pool = pools[_poolId];
address _token = pool.token;

if (percentFee > 0) {
feeAmount = (_amount * percentFee) / getFeeDenominator();
amountAfterFee -= feeAmount;

if (feeAmount + amountAfterFee != _amount) revert INVALID();
if (_token == NATIVE && msg.value < _amount) revert ETH_MISMATCH();

if (_token == NATIVE) {
_transferAmountFrom(_token, TransferData({from: _funder, to: treasury, amount: feeAmount}));
} else {
uint256 balanceBeforeFee = _getBalance(_token, treasury);
_transferAmountFrom(_token, TransferData({from: _funder, to: treasury, amount: feeAmount}));
uint256 balanceAfterFee = _getBalance(_token, treasury);
// Track actual fee paid to account for fee on ERC20 token transfers
feeAmount = balanceAfterFee - balanceBeforeFee;
}
}

if (_token == NATIVE) {
_transferAmountFrom(_token, TransferData({from: _funder, to: address(_strategy), amount: amountAfterFee}));
} else {
uint256 balanceBeforeFundingPool = _getBalance(_token, address(_strategy));
_transferAmountFrom(_token, TransferData({from: _funder, to: address(_strategy), amount: amountAfterFee}));
uint256 balanceAfterFundingPool = _getBalance(_token, address(_strategy));
if (feeAmount > 0) {
uint256 balanceBeforeFee = _token.getBalance(treasury);
_token.transferAmountFrom(_funder, treasury, feeAmount);
uint256 balanceAfterFee = _token.getBalance(treasury);
// Track actual fee paid to account for fee on ERC20 token transfers
amountAfterFee = balanceAfterFundingPool - balanceBeforeFundingPool;
feeAmount = balanceAfterFee - balanceBeforeFee;
}

uint256 balanceBeforeFundingPool = _token.getBalance(address(_strategy));
_token.transferAmountFrom(_funder, address(_strategy), amountAfterFee);
uint256 balanceAfterFundingPool = _token.getBalance(address(_strategy));
// Track actual fee paid to account for fee on ERC20 token transfers
amountAfterFee = balanceAfterFundingPool - balanceBeforeFundingPool;

_strategy.increasePoolAmount(amountAfterFee);

emit PoolFunded(_poolId, amountAfterFee, feeAmount);
Expand Down
11 changes: 6 additions & 5 deletions contracts/core/Registry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import "./interfaces/IRegistry.sol";
import {Anchor} from "./Anchor.sol";
import {Errors} from "./libraries/Errors.sol";
import {Metadata} from "./libraries/Metadata.sol";
import "./libraries/Native.sol";
import "./libraries/Transfer.sol";
import {Transfer} from "./libraries/Transfer.sol";

// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣾⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣷⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣿⣿⣿⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣿⣿⣿⡄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
Expand All @@ -36,7 +35,9 @@ import "./libraries/Transfer.sol";
/// It is also used to deploy the anchor contract for each profile which acts as a proxy
/// for the profile and is used to receive funds and execute transactions on behalf of the profile
/// The Registry is also used to add and remove members from a profile and update the profile 'Metadata'
contract Registry is IRegistry, Initializable, Native, AccessControlUpgradeable, Transfer, Errors {
contract Registry is IRegistry, Initializable, AccessControlUpgradeable, Errors {
using Transfer for address;

/// ==========================
/// === Storage Variables ====
/// ==========================
Expand Down Expand Up @@ -392,7 +393,7 @@ contract Registry is IRegistry, Initializable, Native, AccessControlUpgradeable,
function recoverFunds(address _token, address _recipient) external onlyRole(ALLO_OWNER) {
if (_recipient == address(0)) revert ZERO_ADDRESS();

uint256 amount = _token == NATIVE ? address(this).balance : ERC20(_token).balanceOf(address(this));
_transferAmount(_token, _recipient, amount);
uint256 amount = _token.getBalance(address(this));
_token.transferAmount(_recipient, amount);
}
}
88 changes: 27 additions & 61 deletions contracts/core/libraries/Transfer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ pragma solidity 0.8.19;

// External Libraries
import {SafeTransferLib} from "solady/utils/SafeTransferLib.sol";
// Internal Libraries
import "./Native.sol";

// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣾⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣷⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣿⣿⣿⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣿⣿⣿⡄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
Expand All @@ -21,90 +19,58 @@ import "./Native.sol";
// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠈⠙⠙⠋⠛⠙⠋⠛⠙⠋⠛⠙⠋⠃⠀⠀⠀⠀⠀⠀⠀⠀⠠⠿⠻⠟⠿⠃⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠸⠟⠿⠟⠿⠆⠀⠸⠿⠿⠟⠯⠀⠀⠀⠸⠿⠿⠿⠏⠀⠀⠀⠀⠀⠈⠉⠻⠻⡿⣿⢿⡿⡿⠿⠛⠁⠀⠀⠀⠀⠀⠀
// allo.gitcoin.co

/// @title Transfer contract
/// @author @thelostone-mc <[email protected]>, @0xKurt <[email protected]>, @codenamejason <[email protected]>, @0xZakk <[email protected]>, @nfrgosselin <[email protected]>
/// @notice A helper contract to transfer tokens within Allo protocol
/// @title Transfer library
/// @notice A helper library to transfer tokens within Allo protocol
/// @dev Handles the transfer of tokens to an address
contract Transfer is Native {
/// @notice Thrown when the amount of tokens sent does not match the amount of tokens expected
error AMOUNT_MISMATCH();
library Transfer {
using SafeTransferLib for address;

/// @notice This holds the details for a transfer
struct TransferData {
address from;
address to;
uint256 amount;
}

/// @notice Transfer an amount of a token to an array of addresses
/// @param _token The address of the token
/// @param _transferData TransferData[]
/// @return Whether the transfer was successful or not
function _transferAmountsFrom(address _token, TransferData[] memory _transferData)
internal
virtual
returns (bool)
{
uint256 msgValue = msg.value;

for (uint256 i; i < _transferData.length;) {
TransferData memory transferData = _transferData[i];

if (_token == NATIVE) {
msgValue -= transferData.amount;
SafeTransferLib.safeTransferETH(transferData.to, transferData.amount);
} else {
SafeTransferLib.safeTransferFrom(_token, transferData.from, transferData.to, transferData.amount);
}

unchecked {
i++;
}
}

if (msgValue != 0) revert AMOUNT_MISMATCH();

return true;
}
/// @notice Address of the native token
address public constant NATIVE = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

/// @notice Transfer an amount of a token to an address
/// @param _token The address of the token
/// @param _transferData Individual TransferData
/// @return Whether the transfer was successful or not
function _transferAmountFrom(address _token, TransferData memory _transferData) internal virtual returns (bool) {
uint256 amount = _transferData.amount;
/// @dev When this function is used, it must be checked that balances or msg.value is correct for native tokens
/// @param _token The token to transfer
/// @param _from The address to transfer to
/// @param _to The address to transfer to
/// @param _amount The amount to transfer
function transferAmountFrom(address _token, address _from, address _to, uint256 _amount) internal {
if (_token == NATIVE) {
// Native Token
if (msg.value < amount) revert AMOUNT_MISMATCH();

SafeTransferLib.safeTransferETH(_transferData.to, amount);
// '_from' is ignored. The contract's balance is used.
if (_to != address(this)) _to.safeTransferETH(_amount);
} else {
SafeTransferLib.safeTransferFrom(_token, _transferData.from, _transferData.to, amount);
_token.safeTransferFrom(_from, _to, _amount);
}
return true;
}

/// @notice Transfer an amount of a token to an address
/// @param _token The token to transfer
/// @param _to The address to transfer to
/// @param _amount The amount to transfer
function _transferAmount(address _token, address _to, uint256 _amount) internal virtual {
function transferAmount(address _token, address _to, uint256 _amount) internal {
if (_token == NATIVE) {
SafeTransferLib.safeTransferETH(_to, _amount);
_to.safeTransferETH(_amount);
} else {
SafeTransferLib.safeTransfer(_token, _to, _amount);
_token.safeTransfer(_to, _amount);
}
}

/// @notice Transfer an amount of native token to an address
/// @param _to The address to transfer to
/// @param _amount The amount to transfer
function transferAmountNative(address _to, uint256 _amount) internal {
_to.safeTransferETH(_amount);
}

/// @notice Get the balance of a token for an account
/// @param _token The token to get the balance of
/// @param _account The account to get the balance for
/// @return The balance of the token for the account
function _getBalance(address _token, address _account) internal view returns (uint256) {
function getBalance(address _token, address _account) internal view returns (uint256) {
if (_token == NATIVE) {
return payable(_account).balance;
} else {
return SafeTransferLib.balanceOf(_token, _account);
return _token.balanceOf(_account);
}
}
}
7 changes: 3 additions & 4 deletions contracts/strategies/BaseStrategy.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
pragma solidity ^0.8.19;

// Interfaces
import "../core/interfaces/IStrategy.sol";
import "contracts/core/interfaces/IStrategy.sol";

// Libraries
import {Transfer} from "../core/libraries/Transfer.sol";
import {Errors} from "../core/libraries/Errors.sol";
import {Errors} from "contracts/core/libraries/Errors.sol";

// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣾⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣷⣄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
// ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢀⣿⣿⣿⣿⣷⡀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣼⣿⣿⣿⣿⣿⡄⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀
Expand All @@ -27,7 +26,7 @@ import {Errors} from "../core/libraries/Errors.sol";
/// @author @thelostone-mc <[email protected]>, @0xKurt <[email protected]>, @codenamejason <[email protected]>, @0xZakk <[email protected]>, @nfrgosselin <[email protected]>
/// @notice This contract is the base contract for all strategies
/// @dev This contract is implemented by all strategies.
abstract contract BaseStrategy is IStrategy, Transfer, Errors {
abstract contract BaseStrategy is IStrategy, Errors {
/// ==========================
/// === Storage Variables ====
/// ==========================
Expand Down
16 changes: 8 additions & 8 deletions contracts/strategies/CoreBaseStrategy.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@
pragma solidity 0.8.19;

/// Interfaces
import "../core/interfaces/IBaseStrategy.sol";

/// Libraries
import {Transfer} from "./../core/libraries/Transfer.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "contracts/core/interfaces/IBaseStrategy.sol";
// Internal Libraries
import {Transfer} from "contracts/core/libraries/Transfer.sol";

/// @title BaseStrategy Contract
/// @notice This contract is the base contract for all strategies
/// @dev This contract is implemented by all strategies.
abstract contract CoreBaseStrategy is IBaseStrategy, Transfer {
abstract contract CoreBaseStrategy is IBaseStrategy {
using Transfer for address;

/// ==========================
/// === Storage Variables ====
/// ==========================
Expand Down Expand Up @@ -109,10 +109,10 @@ abstract contract CoreBaseStrategy is IBaseStrategy, Transfer {
{
_beforeWithdraw(_token, _amount, _recipient);
// If the token is the pool token, revert if the amount is greater than the pool amount
if (_getBalance(_token, address(this)) - _amount < poolAmount) {
if (_token.getBalance(address(this)) - _amount < poolAmount) {
revert BaseStrategy_WITHDRAW_MORE_THAN_POOL_AMOUNT();
}
_transferAmount(_token, _recipient, _amount);
_token.transferAmount(_recipient, _amount);
_afterWithdraw(_token, _amount, _recipient);

emit Withdrew(_token, _amount, _recipient);
Expand Down
20 changes: 14 additions & 6 deletions contracts/strategies/DirectAllocation.sol
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import {CoreBaseStrategy} from "./CoreBaseStrategy.sol";
// Core Contracts
import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol";
// Internal Libraries
import {Native} from "contracts/core/libraries/Native.sol";
import {Errors} from "contracts/core/libraries/Errors.sol";
import {Transfer} from "contracts/core/libraries/Transfer.sol";

/// @title DirectAllocationStrategy
/// @dev The strategy only implements the allocate logic
/// @notice A strategy that directly allocates funds to a recipient
contract DirectAllocationStrategy is CoreBaseStrategy {
contract DirectAllocationStrategy is CoreBaseStrategy, Native, Errors {
using Transfer for address;

/// ===============================
/// ============ Errors ===========
/// ===============================

/// @notice Error when the function is not implemented
error NOT_IMPLEMENTED();

/// @notice Error when the input is invalid
error INVALID_INPUT();

Expand Down Expand Up @@ -69,15 +73,19 @@ contract DirectAllocationStrategy is CoreBaseStrategy {
revert INVALID_INPUT();
}

uint256 _totalNativeAmount;
for (uint256 _i = 0; _i < _recipientsLength;) {
/// Direct allocate the funds
_transferAmountFrom(_tokens[_i], TransferData({from: _sender, to: _recipients[_i], amount: _amounts[_i]}));
if (_tokens[_i] == NATIVE) _totalNativeAmount += _amounts[_i];
_tokens[_i].transferAmountFrom(_sender, _recipients[_i], _amounts[_i]);

emit DirectAllocated(_recipients[_i], _amounts[_i], _tokens[_i], _sender);
unchecked {
++_i;
}
}

if (msg.value < _totalNativeAmount) revert ETH_MISMATCH();
}

/// @notice Distribute funds to recipients
Expand Down
Loading

0 comments on commit d4ab757

Please sign in to comment.