plexus-money / contracts

Plexus Contracts aims to simplify defi, maintain security of user funds and aggregate other contracts functionality.
12 stars 12 forks source link

Clean up the `wrap` and `unwrap` functions in the `WrapAndUnwrap.sol` contract #78

Closed terminator0x closed 3 years ago

terminator0x commented 3 years ago

Right now the wrap and unwrap functions in the uni contract are poorly optimized and they need to cleaned up as follows.

  1. Seperate out the wrap/unwrap and the token swap logic into 2 functions and call either of the functions based on the length of the destination tokens as is currently the case.
  2. Introduce a new variable called addresses [] paths and pass this variable anywhere the paths are required for conducting a swap.
    • The reason for this is once the new contracts are deployed, all token swap paths will be calculated in real time by the frontend as this is much more dynamic and efficient and it will save massively on gas costs going forward.
  3. Instead of using the zero address to check if the sourceToken is ETH, please import the ERC20 and pass the token address and check if the token symbol is WETH for ETH wrapping i.e.
if (ERC20(sourceToken).symbol() === “WETH”) {
// paste the ETH wrapping logic here
}
  1. Remove all zero address references in the contract which are meant to represent ETH and replace it withe above logic.
  2. Add a new param address lpTokenPairAddress which will be passed from the frontend and use it to initialize the lp token IERC20 lpToken = IERC20(lpTokenPairAddress); as needed in the contract function.
  3. Remove the following variables from the global state as setting them is just wasting gas for no good reason.
uint256 private approvalAmount;
    uint256 private longTimeFromNow;
    uint256 public fee;
    uint256 public maxfee;
    mapping(address => address[]) public lpTokenAddressToPairs;
    mapping(string => address) public stablecoins;
    mapping(address => mapping(address => address[])) public presetPaths;

also remove any functions associated with these global variables and for approvalAmount Just set it locally in the function where it’s being called.

  1. Check the wrap functionality and make sure I’m able to wrap ERC20 other ERC20 tokens if supported i.e. I’m able to wrap from DAI to SUSHI-COMPOUND LP tokens.
    • Also please add a new test for this specific scenario i.e. 11_add_liquidity_sushi.js and make sure you test for both passes and fails.
    • Add a new test too in the same file, that tests the unwrapping of the derived LP tokens back to the original ERC20 token which will be DAI in this case.

Acceptance Criteria

The wrapping and unwrapping functionality works as before but it’s now better optimizer and uses way less gas after the above changes.

ETA

2 days

softskillpro commented 3 years ago

I have a question about fee and maxfee in No.6. in the contract, fee and maxfee is used in following code snippet.

if (fee > 0) {
        uint256 totalFee = (address(this).balance.mul(fee)).div(10000);
        if (totalFee > 0) {
            payable(owner()).transfer(totalFee);
        }
        payable(msg.sender).transfer(address(this).balance);
    } else {
        payable(msg.sender).transfer(address(this).balance);
    }

In order to remove fee and maxfee in global states, we need to pass fee as argument to functions. is it okay?

softskillpro commented 3 years ago

No2, adding addresses [] paths: is it for removing getBestPath()?