From 033a83d84d8891a2300acdfcf4bb7ff57bea35ee Mon Sep 17 00:00:00 2001 From: Mick de Graaf Date: Wed, 3 Feb 2021 00:43:12 +0100 Subject: [PATCH 1/5] TokenListUpdater Implementation + Tests --- contracts/callManagers/TokenListUpdater.sol | 31 ++++ .../facets/shared/Access/CallProtection.sol | 1 + test/TokenListUpdater.ts | 164 ++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 contracts/callManagers/TokenListUpdater.sol create mode 100644 test/TokenListUpdater.ts diff --git a/contracts/callManagers/TokenListUpdater.sol b/contracts/callManagers/TokenListUpdater.sol new file mode 100644 index 0000000..31afaa4 --- /dev/null +++ b/contracts/callManagers/TokenListUpdater.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma experimental ABIEncoderV2; +pragma solidity ^0.7.1; + +import "@openzeppelin/contracts/access/Ownable.sol"; +import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import "../interfaces/IExperiPie.sol"; + +contract TokenListUpdater is Ownable, ReentrancyGuard { + + uint256 public constant MIN_AMOUNT = 10**6; + + function update(address _pie, address[] calldata _tokens) onlyOwner external { + IExperiPie pie = IExperiPie(_pie); + + for(uint256 i = 0; i < _tokens.length; i ++) { + uint256 tokenBalance = pie.balance(_tokens[i]); + + if(tokenBalance >= MIN_AMOUNT && !pie.getTokenInPool(_tokens[i])) { + //if min amount reached and not already in pool + bytes memory data = abi.encodeWithSelector(pie.addToken.selector, _tokens[i]); + pie.singleCall(address(pie), data, 0); + } else if(tokenBalance < MIN_AMOUNT && pie.getTokenInPool(_tokens[i])) { + // if smaller than min amount and in pool + bytes memory data = abi.encodeWithSelector(pie.removeToken.selector, _tokens[i]); + pie.singleCall(address(pie), data, 0); + } + } + } + +} \ No newline at end of file diff --git a/contracts/facets/shared/Access/CallProtection.sol b/contracts/facets/shared/Access/CallProtection.sol index 57ff6b8..57a3f23 100644 --- a/contracts/facets/shared/Access/CallProtection.sol +++ b/contracts/facets/shared/Access/CallProtection.sol @@ -8,6 +8,7 @@ contract CallProtection { require( msg.sender == LibDiamond.diamondStorage().contractOwner || msg.sender == address(this), "NOT_ALLOWED" + // TODO consider allowing whitelisted callers from the callFacet ); _; } diff --git a/test/TokenListUpdater.ts b/test/TokenListUpdater.ts new file mode 100644 index 0000000..1bf7f5f --- /dev/null +++ b/test/TokenListUpdater.ts @@ -0,0 +1,164 @@ +import chai, {expect} from "chai"; +import { deployContract, solidity} from "ethereum-waffle"; +import { ethers, run, ethereum, network } from "@nomiclabs/buidler"; +import { Signer, constants, Contract, BytesLike, utils } from "ethers"; +import TimeTraveler from "../utils/TimeTraveler"; +import { IExperiPie } from "../typechain/IExperiPie"; +import { MockToken } from "../typechain/MockToken"; +import { BasketFacet, CallFacet, DiamondFactoryContract, Erc20Facet, TokenListUpdater } from "../typechain"; +import BasketFacetArtifact from "../artifacts/BasketFacet.json"; +import Erc20FacetArtifact from "../artifacts/ERC20Facet.json"; +import TokenListUpdaterArtifact from "../artifacts/TokenListUpdater.json"; +import CallFacetArtifact from "../artifacts/CallFacet.json"; +import { IExperiPieFactory } from "../typechain/IExperiPieFactory"; +import MockTokenArtifact from "../artifacts/MockToken.json"; +import { parseEther } from "ethers/lib/utils"; + +chai.use(solidity); + +const FacetCutAction = { + Add: 0, + Replace: 1, + Remove: 2, +}; + +function getSelectors(contract: Contract) { + const signatures: BytesLike[] = []; + for(const key of Object.keys(contract.functions)) { + signatures.push(utils.keccak256(utils.toUtf8Bytes(key)).substr(0, 10)); + } + + return signatures; +} + +describe("TokenListUpdater", function() { + this.timeout(300000000); + + let experiPie: IExperiPie; + + let account: string; + let account2: string; + let signers: Signer[]; + let timeTraveler: TimeTraveler; + let tokenListUpdater: TokenListUpdater; + const testTokens: MockToken[] = []; + const testTokenAddresses: string[] = []; + let extraToken: MockToken; + + before(async() => { + signers = await ethers.getSigners(); + account = await signers[0].getAddress(); + account2 = await signers[1].getAddress(); + timeTraveler = new TimeTraveler(ethereum); + + const diamondFactory = (await run("deploy-diamond-factory")) as DiamondFactoryContract; + + const basketFacet = (await deployContract(signers[0], BasketFacetArtifact)) as BasketFacet; + const erc20Facet = (await deployContract(signers[0], Erc20FacetArtifact)) as Erc20Facet; + const callFacet = (await deployContract(signers[0], CallFacetArtifact)) as CallFacet; + + + await diamondFactory.deployNewDiamond( + account, + [ + { + action: FacetCutAction.Add, + facetAddress: basketFacet.address, + functionSelectors: getSelectors(basketFacet) + }, + { + action: FacetCutAction.Add, + facetAddress: erc20Facet.address, + functionSelectors: getSelectors(erc20Facet) + }, + { + action: FacetCutAction.Add, + facetAddress: callFacet.address, + functionSelectors: getSelectors(callFacet) + } + ] + ) + + + const experiPieAddress = await diamondFactory.diamonds(0); + experiPie = IExperiPieFactory.connect(experiPieAddress, signers[0]); + + tokenListUpdater = (await deployContract(signers[0], TokenListUpdaterArtifact)) as TokenListUpdater + + for(let i = 0; i < 3; i ++) { + const token = await (deployContract(signers[0], MockTokenArtifact, ["Mock", "Mock"])) as MockToken; + await token.mint(parseEther("1000000"), experiPie.address); + await experiPie.addToken(token.address); + testTokens.push(token); + testTokenAddresses.push(token.address); + } + + extraToken = await (deployContract(signers[0], MockTokenArtifact, ["Mock", "Mock"])) as MockToken; + await extraToken.mint(parseEther("1000000"), account); + + await experiPie.addCaller(tokenListUpdater.address); + + await timeTraveler.snapshot(); + }); + + beforeEach(async() => { + await timeTraveler.revertSnapshot(); + }); + + it("Calling from non owner should fail", async() => { + await tokenListUpdater.renounceOwnership(); + await expect(tokenListUpdater.update(experiPie.address, testTokenAddresses)).to.be.revertedWith("Ownable: caller is not the owner"); + }); + + + it("Removing a token when the balance is too low should work", async() => { + const token = testTokens[testTokens.length - 1] + const transferAmount = (await token.balanceOf(experiPie.address)).sub(1); + + // Send out tokens + const tx = await token.populateTransaction.transfer(account2, transferAmount); + await experiPie.singleCall(tx.to, tx.data, 0); + + await tokenListUpdater.update(experiPie.address, [token.address]); + + const tokens = await experiPie.getTokens(); + const tokenCount = tokens.length; + + expect(tokenCount).to.eq(testTokens.length - 1); + expect(tokens).to.eql(testTokenAddresses.slice(0, -1)); + }); + + it("Adding a token when it was not added before but the balance is sufficient should work", async() => { + await extraToken.transfer(experiPie.address, parseEther("1")); + + await tokenListUpdater.update(experiPie.address, [extraToken.address]); + + const tokens = await experiPie.getTokens(); + const tokenCount = tokens.length; + + expect(tokenCount).to.eq(testTokens.length + 1); + expect(tokens).to.eql([...testTokenAddresses, extraToken.address]); + }); + + it("Updating a token which is not in the list w/o sufficient balance", async() => { + await extraToken.transfer(experiPie.address, "420"); + + await tokenListUpdater.update(experiPie.address, [extraToken.address]); + + const tokens = await experiPie.getTokens(); + const tokenCount = tokens.length; + + expect(tokenCount).to.eq(testTokens.length); + expect(tokens).to.eql(testTokenAddresses); + }); + + it("Updating a token which is in the list with sufficient balance should do nothing", async() => { + await tokenListUpdater.update(experiPie.address, [testTokenAddresses[0]]); + + const tokens = await experiPie.getTokens(); + const tokenCount = tokens.length; + + expect(tokenCount).to.eq(testTokens.length); + expect(tokens).to.eql(testTokenAddresses); + }); +}); \ No newline at end of file From a452e70eadcaad4d2c066ea967b406f9897babbe Mon Sep 17 00:00:00 2001 From: Mick de Graaf Date: Wed, 3 Feb 2021 16:39:20 +0100 Subject: [PATCH 2/5] reentry protection --- contracts/callManagers/TokenListUpdater.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/callManagers/TokenListUpdater.sol b/contracts/callManagers/TokenListUpdater.sol index 31afaa4..333d272 100644 --- a/contracts/callManagers/TokenListUpdater.sol +++ b/contracts/callManagers/TokenListUpdater.sol @@ -10,7 +10,7 @@ contract TokenListUpdater is Ownable, ReentrancyGuard { uint256 public constant MIN_AMOUNT = 10**6; - function update(address _pie, address[] calldata _tokens) onlyOwner external { + function update(address _pie, address[] calldata _tokens) onlyOwner nonReentrant external { IExperiPie pie = IExperiPie(_pie); for(uint256 i = 0; i < _tokens.length; i ++) { From 12883fe1fcc34b40763e1af8ee4e5379a8fb0ae4 Mon Sep 17 00:00:00 2001 From: Mick de Graaf Date: Wed, 3 Feb 2021 16:39:32 +0100 Subject: [PATCH 3/5] tokenListUpdater task --- buidler.config.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/buidler.config.ts b/buidler.config.ts index 2b538ff..d17a68c 100644 --- a/buidler.config.ts +++ b/buidler.config.ts @@ -7,7 +7,7 @@ import {deployContract} from "ethereum-waffle"; import DiamondFactoryArtifact from './artifacts/DiamondFactoryContract.json'; import {DiamondFactoryContract} from "./typechain/DiamondFactoryContract"; -import { BasketFacet, CallFacet, Diamond, DiamondCutFacet, DiamondFactoryContractFactory, DiamondLoupeFacet, Erc20Facet, LendingRegistry, OwnershipFacet, PieFactoryContract, PieFactoryContractFactory, StakingLogicSushiFactory } from "./typechain"; +import { BasketFacet, CallFacet, Diamond, DiamondCutFacet, DiamondFactoryContractFactory, DiamondLoupeFacet, Erc20Facet, LendingRegistry, OwnershipFacet, PieFactoryContract, PieFactoryContractFactory, StakingLogicSushiFactory, TokenListUpdater, TokenListUpdaterFactory } from "./typechain"; import BasketFacetArtifact from "./artifacts/BasketFacet.json"; import Erc20FacetArtifact from "./artifacts/ERC20Facet.json"; import CallFacetArtifact from "./artifacts/CallFacet.json"; @@ -51,7 +51,7 @@ const config = { mainnet: { url: `https://mainnet.infura.io/v3/${process.env.INFURA_PROJECT_ID}`, accounts: [process.env.PRIVATE_KEY], - gasPrice: 70000000000 + gasPrice: 200000000000 }, kovan: { url: `https://kovan.infura.io/v3/${process.env.INFURA_PROJECT_ID}`, @@ -335,4 +335,13 @@ task("deploy-lending-logic-aave") console.log(`Deployed lendingLogicAave at: ${lendingLogicAave.address}`); }); +task("deploy-token-list-updater") + .setAction(async(taskArgs, {ethers}) => { + const signers = await ethers.getSigners(); + + const tokenListUpdater = await (new TokenListUpdaterFactory(signers[0]).deploy()); + + console.log(`Deployed tokenListUpdater: ${tokenListUpdater.address}`); +}); + export default config; \ No newline at end of file From 55c9e505676accc2ed40b0297e604decdd584cfa Mon Sep 17 00:00:00 2001 From: Mick de Graaf Date: Wed, 3 Feb 2021 17:22:19 +0100 Subject: [PATCH 4/5] Allow pie itself to call update for automic transactions --- contracts/callManagers/TokenListUpdater.sol | 8 +++++++- test/TokenListUpdater.ts | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/contracts/callManagers/TokenListUpdater.sol b/contracts/callManagers/TokenListUpdater.sol index 333d272..bc458c6 100644 --- a/contracts/callManagers/TokenListUpdater.sol +++ b/contracts/callManagers/TokenListUpdater.sol @@ -8,9 +8,15 @@ import "../interfaces/IExperiPie.sol"; contract TokenListUpdater is Ownable, ReentrancyGuard { + modifier ownerOrPie(address _pie) { + require(msg.sender == owner() || + msg.sender == _pie, "Not allowed"); + _; + } + uint256 public constant MIN_AMOUNT = 10**6; - function update(address _pie, address[] calldata _tokens) onlyOwner nonReentrant external { + function update(address _pie, address[] calldata _tokens) ownerOrPie(_pie) nonReentrant external { IExperiPie pie = IExperiPie(_pie); for(uint256 i = 0; i < _tokens.length; i ++) { diff --git a/test/TokenListUpdater.ts b/test/TokenListUpdater.ts index 1bf7f5f..b111ad4 100644 --- a/test/TokenListUpdater.ts +++ b/test/TokenListUpdater.ts @@ -107,7 +107,7 @@ describe("TokenListUpdater", function() { it("Calling from non owner should fail", async() => { await tokenListUpdater.renounceOwnership(); - await expect(tokenListUpdater.update(experiPie.address, testTokenAddresses)).to.be.revertedWith("Ownable: caller is not the owner"); + await expect(tokenListUpdater.update(experiPie.address, testTokenAddresses)).to.be.revertedWith("Not allowed"); }); @@ -161,4 +161,14 @@ describe("TokenListUpdater", function() { expect(tokenCount).to.eq(testTokens.length); expect(tokens).to.eql(testTokenAddresses); }); + + it("Updating from the pie itself should work", async() => { + await tokenListUpdater.populateTransaction.update(experiPie.address, [testTokenAddresses[0]]); + + const tokens = await experiPie.getTokens(); + const tokenCount = tokens.length; + + expect(tokenCount).to.eq(testTokens.length); + expect(tokens).to.eql(testTokenAddresses); + }); }); \ No newline at end of file From d1c9e772e49fdea26f5a37996bfb2537f0cef95a Mon Sep 17 00:00:00 2001 From: Mick de Graaf Date: Wed, 3 Feb 2021 18:01:46 +0100 Subject: [PATCH 5/5] Exectute token update from Pie in test --- test/TokenListUpdater.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/TokenListUpdater.ts b/test/TokenListUpdater.ts index b111ad4..dd62430 100644 --- a/test/TokenListUpdater.ts +++ b/test/TokenListUpdater.ts @@ -163,7 +163,9 @@ describe("TokenListUpdater", function() { }); it("Updating from the pie itself should work", async() => { - await tokenListUpdater.populateTransaction.update(experiPie.address, [testTokenAddresses[0]]); + const tx = await tokenListUpdater.populateTransaction.update(experiPie.address, [testTokenAddresses[0]]); + + await experiPie.singleCall(tx.to, tx.data, 0); const tokens = await experiPie.getTokens(); const tokenCount = tokens.length;