From 22676f3f4a29e960c1df8ae5a42fd577d7443609 Mon Sep 17 00:00:00 2001 From: bweick Date: Fri, 18 Jun 2021 12:35:16 -0700 Subject: [PATCH 1/7] Continuing AirdropModule fixes. --- contracts/protocol/modules/AirdropModule.sol | 92 +++++++++++--------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/contracts/protocol/modules/AirdropModule.sol b/contracts/protocol/modules/AirdropModule.sol index db0f66837..007a29a5d 100644 --- a/contracts/protocol/modules/AirdropModule.sol +++ b/contracts/protocol/modules/AirdropModule.sol @@ -19,7 +19,7 @@ pragma solidity 0.6.10; pragma experimental "ABIEncoderV2"; -import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; import { SafeCast } from "@openzeppelin/contracts/utils/SafeCast.sol"; import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol"; @@ -53,22 +53,28 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { /* ============ Structs ============ */ struct AirdropSettings { - address[] airdrops; // Array of tokens manager is allowing to be absorbed - address feeRecipient; // Address airdrop fees are sent to - uint256 airdropFee; // Percentage in preciseUnits of airdrop sent to feeRecipient (1e16 = 1%) - bool anyoneAbsorb; // Boolean indicating if any address can call absorb or just the manager + address[] airdrops; // Array of tokens manager is allowing to be absorbed + mapping(address => bool) isAirdrop; // Mapping indicating if token is an allowed airdrop + address feeRecipient; // Address airdrop fees are sent to + uint256 airdropFee; // Percentage in preciseUnits of airdrop sent to feeRecipient (1e16 = 1%) + bool anyoneAbsorb; // Boolean indicating if any address can call absorb or just the manager } /* ============ Events ============ */ event ComponentAbsorbed( ISetToken indexed _setToken, - address _absorbedToken, + address indexed _absorbedToken, uint256 _absorbedQuantity, uint256 _managerFee, uint256 _protocolFee ); + event AirdropComponentAdded(ISetToken indexed _setToken, IERC20 indexed _component); + event AirdropComponentRemoved(ISetToken indexed _setToken, IERC20 indexed _component); + event AnyoneAbsorbUpdated(ISetToken indexed _setToken, bool _anyoneAbsorb); + event FeeRecipientUpdated(address indexed _setToken, address _newFeeRecipient); + /* ============ Modifiers ============ */ /** @@ -116,7 +122,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @param _setToken Address of SetToken * @param _token Address of token to absorb */ - function absorb(ISetToken _setToken, address _token) + function absorb(ISetToken _setToken, IERC20 _token) external nonReentrant onlyValidCaller(_setToken) @@ -129,22 +135,26 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * SET MANAGER ONLY. Adds new tokens to be added to positions when absorb is called. * * @param _setToken Address of SetToken - * @param _airdrop List of airdrops to add + * @param _airdrop Component to add to airdrop list */ - function addAirdrop(ISetToken _setToken, address _airdrop) external onlyManagerAndValidSet(_setToken) { + function addAirdrop(ISetToken _setToken, IERC20 _airdrop) external onlyManagerAndValidSet(_setToken) { require(!isAirdropToken(_setToken, _airdrop), "Token already added."); - airdropSettings[_setToken].airdrops.push(_airdrop); + airdropSettings[_setToken].airdrops.push(address(_airdrop)); + airdropSettings[_setToken].isAirdrop[address(_airdrop)] = true; + emit AirdropComponentAdded(_setToken, _airdrop); } /** * SET MANAGER ONLY. Removes tokens from list to be absorbed. * * @param _setToken Address of SetToken - * @param _airdrop List of airdrops to remove + * @param _airdrop Component to remove from airdrop list */ - function removeAirdrop(ISetToken _setToken, address _airdrop) external onlyManagerAndValidSet(_setToken) { + function removeAirdrop(ISetToken _setToken, IERC20 _airdrop) external onlyManagerAndValidSet(_setToken) { require(isAirdropToken(_setToken, _airdrop), "Token not added."); - airdropSettings[_setToken].airdrops = airdropSettings[_setToken].airdrops.remove(_airdrop); + airdropSettings[_setToken].airdrops.removeStorage(address(_airdrop)); + airdropSettings[_setToken].isAirdrop[address(_airdrop)] = false; + emit AirdropComponentRemoved(_setToken, _airdrop); } /** @@ -152,8 +162,9 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * * @param _setToken Address of SetToken */ - function updateAnyoneAbsorb(ISetToken _setToken) external onlyManagerAndValidSet(_setToken) { - airdropSettings[_setToken].anyoneAbsorb = !airdropSettings[_setToken].anyoneAbsorb; + function updateAnyoneAbsorb(ISetToken _setToken, bool _anyoneAbsorb) external onlyManagerAndValidSet(_setToken) { + airdropSettings[_setToken].anyoneAbsorb = _anyoneAbsorb; + emit AnyoneAbsorbUpdated(_setToken, _anyoneAbsorb); } /** @@ -167,11 +178,11 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { address _newFeeRecipient ) external - onlySetManager(_setToken, msg.sender) - onlyValidAndInitializedSet(_setToken) + onlyManagerAndValidSet(_setToken) { require(_newFeeRecipient != address(0), "Passed address must be non-zero"); airdropSettings[_setToken].feeRecipient = _newFeeRecipient; + emit FeeRecipientUpdated(_setToken, _newFeeRecipient); } /** @@ -188,7 +199,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { onlySetManager(_setToken, msg.sender) onlyValidAndInitializedSet(_setToken) { - require(_newFee < PreciseUnitMath.preciseUnit(), "Airdrop fee can't exceed 100%"); + require(_newFee <= PreciseUnitMath.preciseUnit(), "Airdrop fee can't exceed 100%"); // Absorb all outstanding tokens before fee is updated _batchAbsorb(_setToken, airdropSettings[_setToken].airdrops); @@ -212,8 +223,9 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { onlySetManager(_setToken, msg.sender) onlyValidAndPendingSet(_setToken) { - require(_airdropSettings.airdrops.length > 0, "At least one token must be passed."); require(_airdropSettings.airdropFee <= PreciseUnitMath.preciseUnit(), "Fee must be <= 100%."); + require(_airdropSettings.feeRecipient != address(0), "Zero fee address passed"); + require(!_airdropSettings.airdrops.hasDuplicate(), "Duplicate airdrop token passed"); airdropSettings[_setToken] = _airdropSettings; @@ -235,7 +247,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @return Array of tokens approved for airdrops */ function getAirdrops(ISetToken _setToken) external view returns (address[] memory) { - return _airdrops(_setToken); + return airdropSettings[_setToken].airdrops; } /** @@ -244,16 +256,16 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @param _setToken Address of SetToken * @return Boolean indicating approval for airdrops */ - function isAirdropToken(ISetToken _setToken, address _token) public view returns (bool) { - return _airdrops(_setToken).contains(_token); + function isAirdropToken(ISetToken _setToken, IERC20 _token) public view returns (bool) { + return airdropSettings[_setToken].isAirdrop[_token]; } /* ============ Internal Functions ============ */ /** - * Check token approved for airdrops then handle airdropped postion. + * Check token approved for airdrops then handle airdropped position. */ - function _absorb(ISetToken _setToken, address _token) internal { + function _absorb(ISetToken _setToken, IERC20 _token) internal { require(isAirdropToken(_setToken, _token), "Must be approved token."); _handleAirdropPosition(_setToken, _token); @@ -261,7 +273,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { function _batchAbsorb(ISetToken _setToken, address[] memory _tokens) internal { for (uint256 i = 0; i < _tokens.length; i++) { - _absorb(_setToken, _tokens[i]); + _absorb(_setToken, IERC20(_tokens[i])); } } @@ -271,10 +283,9 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @param _setToken Address of SetToken * @param _token Address of airdropped token */ - function _handleAirdropPosition(ISetToken _setToken, address _token) internal { - uint256 preFeeTokenBalance = ERC20(_token).balanceOf(address(_setToken)); - uint256 amountAirdropped = preFeeTokenBalance.sub(_setToken.getDefaultTrackedBalance(_token)); - + function _handleAirdropPosition(ISetToken _setToken, IERC20 _token) internal { + uint256 preFeeTokenBalance = _token.balanceOf(address(_setToken)); + uint256 amountAirdropped = preFeeTokenBalance.sub(_setToken.getDefaultTrackedBalance(address(_token))); if (amountAirdropped > 0) { (uint256 managerTake, uint256 protocolTake, uint256 totalFees) = _handleFees(_setToken, _token, amountAirdropped); @@ -293,30 +304,29 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @param _setToken Address of SetToken * @param _component Address of airdropped component * @param _amountAirdropped Amount of tokens airdropped to the SetToken - * @return Amount of airdropped tokens set aside for manager fees - * @return Amount of airdropped tokens set aside for protocol fees + * @return Amount of airdropped tokens set aside for manager fees net of protocol fees + * @return Amount of airdropped tokens set aside for protocol fees (taken from manager fees) * @return Total fees paid */ function _handleFees( ISetToken _setToken, - address _component, + IERC20 _component, uint256 _amountAirdropped ) internal - returns (uint256, uint256, uint256) + returns (uint256 netManagerTake, uint256 protocolTake, uint256 totalFees) { uint256 airdropFee = airdropSettings[_setToken].airdropFee; if (airdropFee > 0) { - uint256 managerTake = _amountAirdropped.preciseMul(airdropFee); + totalFees = _amountAirdropped.preciseMul(airdropFee); - uint256 protocolTake = ModuleBase.getModuleFee(AIRDROP_MODULE_PROTOCOL_FEE_INDEX, managerTake); - uint256 netManagerTake = managerTake.sub(protocolTake); - uint256 totalFees = netManagerTake.add(protocolTake); + protocolTake = ModuleBase.getModuleFee(AIRDROP_MODULE_PROTOCOL_FEE_INDEX, totalFees); + netManagerTake = totalFees.sub(protocolTake); - _setToken.invokeTransfer(_component, airdropSettings[_setToken].feeRecipient, netManagerTake); + _setToken.strictInvokeTransfer(address(_component), airdropSettings[_setToken].feeRecipient, netManagerTake); - ModuleBase.payProtocolFeeFromSetToken(_setToken, _component, protocolTake); + ModuleBase.payProtocolFeeFromSetToken(_setToken, address(_component), protocolTake); return (netManagerTake, protocolTake, totalFees); } else { @@ -343,8 +353,4 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { function _isValidCaller(ISetToken _setToken) internal view returns(bool) { return airdropSettings[_setToken].anyoneAbsorb || isSetManager(_setToken, msg.sender); } - - function _airdrops(ISetToken _setToken) internal view returns(address[] memory) { - return airdropSettings[_setToken].airdrops; - } } \ No newline at end of file From b0fe513bbd2794c7a3fd34ed89439c7cafb6e06b Mon Sep 17 00:00:00 2001 From: bweick Date: Thu, 1 Jul 2021 19:20:15 -0700 Subject: [PATCH 2/7] Updated data structure, added event, changed some initialization logic. --- contracts/protocol/modules/AirdropModule.sol | 31 +++++--- test/protocol/modules/airdropModule.spec.ts | 79 ++++++++++++++++++-- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/contracts/protocol/modules/AirdropModule.sol b/contracts/protocol/modules/AirdropModule.sol index 007a29a5d..ca1f5dc99 100644 --- a/contracts/protocol/modules/AirdropModule.sol +++ b/contracts/protocol/modules/AirdropModule.sol @@ -54,7 +54,6 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { struct AirdropSettings { address[] airdrops; // Array of tokens manager is allowing to be absorbed - mapping(address => bool) isAirdrop; // Mapping indicating if token is an allowed airdrop address feeRecipient; // Address airdrop fees are sent to uint256 airdropFee; // Percentage in preciseUnits of airdrop sent to feeRecipient (1e16 = 1%) bool anyoneAbsorb; // Boolean indicating if any address can call absorb or just the manager @@ -64,7 +63,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { event ComponentAbsorbed( ISetToken indexed _setToken, - address indexed _absorbedToken, + IERC20 indexed _absorbedToken, uint256 _absorbedQuantity, uint256 _managerFee, uint256 _protocolFee @@ -73,7 +72,8 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { event AirdropComponentAdded(ISetToken indexed _setToken, IERC20 indexed _component); event AirdropComponentRemoved(ISetToken indexed _setToken, IERC20 indexed _component); event AnyoneAbsorbUpdated(ISetToken indexed _setToken, bool _anyoneAbsorb); - event FeeRecipientUpdated(address indexed _setToken, address _newFeeRecipient); + event AirdropFeeUpdated(ISetToken indexed _setToken, uint256 _newFee); + event FeeRecipientUpdated(ISetToken indexed _setToken, address _newFeeRecipient); /* ============ Modifiers ============ */ @@ -92,6 +92,8 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { /* ============ State Variables ============ */ mapping(ISetToken => AirdropSettings) public airdropSettings; + // Mapping indicating if token is an allowed airdrop + mapping(ISetToken => mapping(IERC20 => bool)) public isAirdrop; /* ============ Constructor ============ */ @@ -140,7 +142,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { function addAirdrop(ISetToken _setToken, IERC20 _airdrop) external onlyManagerAndValidSet(_setToken) { require(!isAirdropToken(_setToken, _airdrop), "Token already added."); airdropSettings[_setToken].airdrops.push(address(_airdrop)); - airdropSettings[_setToken].isAirdrop[address(_airdrop)] = true; + isAirdrop[_setToken][_airdrop] = true; emit AirdropComponentAdded(_setToken, _airdrop); } @@ -153,7 +155,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { function removeAirdrop(ISetToken _setToken, IERC20 _airdrop) external onlyManagerAndValidSet(_setToken) { require(isAirdropToken(_setToken, _airdrop), "Token not added."); airdropSettings[_setToken].airdrops.removeStorage(address(_airdrop)); - airdropSettings[_setToken].isAirdrop[address(_airdrop)] = false; + isAirdrop[_setToken][_airdrop] = false; emit AirdropComponentRemoved(_setToken, _airdrop); } @@ -205,6 +207,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { _batchAbsorb(_setToken, airdropSettings[_setToken].airdrops); airdropSettings[_setToken].airdropFee = _newFee; + emit AirdropFeeUpdated(_setToken, _newFee); } /** @@ -225,10 +228,16 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { { require(_airdropSettings.airdropFee <= PreciseUnitMath.preciseUnit(), "Fee must be <= 100%."); require(_airdropSettings.feeRecipient != address(0), "Zero fee address passed"); - require(!_airdropSettings.airdrops.hasDuplicate(), "Duplicate airdrop token passed"); + if (_airdropSettings.airdrops.length > 0) { + require(!_airdropSettings.airdrops.hasDuplicate(), "Duplicate airdrop token passed"); + } airdropSettings[_setToken] = _airdropSettings; + for (uint256 i = 0; i < _airdropSettings.airdrops.length; i++) { + isAirdrop[_setToken][IERC20(_airdropSettings.airdrops[i])] = true; + } + _setToken.initializeModule(); } @@ -257,7 +266,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @return Boolean indicating approval for airdrops */ function isAirdropToken(ISetToken _setToken, IERC20 _token) public view returns (bool) { - return airdropSettings[_setToken].isAirdrop[_token]; + return isAirdrop[_setToken][_token]; } /* ============ Internal Functions ============ */ @@ -292,7 +301,7 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { uint256 newUnit = _getPostAirdropUnit(_setToken, preFeeTokenBalance, totalFees); - _setToken.editDefaultPosition(_token, newUnit); + _setToken.editDefaultPosition(address(_token), newUnit); emit ComponentAbsorbed(_setToken, _token, amountAirdropped, managerTake, protocolTake); } @@ -304,9 +313,9 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * @param _setToken Address of SetToken * @param _component Address of airdropped component * @param _amountAirdropped Amount of tokens airdropped to the SetToken - * @return Amount of airdropped tokens set aside for manager fees net of protocol fees - * @return Amount of airdropped tokens set aside for protocol fees (taken from manager fees) - * @return Total fees paid + * @return netManagerTake Amount of airdropped tokens set aside for manager fees net of protocol fees + * @return protocolTake Amount of airdropped tokens set aside for protocol fees (taken from manager fees) + * @return totalFees Total fees paid */ function _handleFees( ISetToken _setToken, diff --git a/test/protocol/modules/airdropModule.spec.ts b/test/protocol/modules/airdropModule.spec.ts index 16fdd7e5d..761626638 100644 --- a/test/protocol/modules/airdropModule.spec.ts +++ b/test/protocol/modules/airdropModule.spec.ts @@ -23,7 +23,7 @@ import { AirdropSettings } from "@utils/types"; const expect = getWaffleExpect(); -describe("AirdropModule", () => { +describe.only("AirdropModule", () => { let owner: Account; let feeRecipient: Account; let tokenHolder: Account; @@ -98,6 +98,16 @@ describe("AirdropModule", () => { expect(airdropSettings.anyoneAbsorb).to.eq(anyoneAbsorb); }); + it("should set the correct isAirdrop state", async () => { + await subject(); + + const wethIsAirdrop = await airdropModule.isAirdrop(subjectSetToken, setup.weth.address); + const usdcIsAirdrop = await airdropModule.isAirdrop(subjectSetToken, setup.usdc.address); + + expect(wethIsAirdrop).to.be.true; + expect(usdcIsAirdrop).to.be.true; + }); + describe("when the airdrops array is empty", async () => { before(async () => { airdrops = []; @@ -107,8 +117,26 @@ describe("AirdropModule", () => { airdrops = [setup.usdc.address, setup.weth.address]; }); + it("should set the airdrops with an empty array", async () => { + await subject(); + + const airdrops = await airdropModule.getAirdrops(subjectSetToken); + + expect(airdrops).to.be.empty; + }); + }); + + describe("when there are duplicate components in the airdrops array", async () => { + before(async () => { + airdrops = [setup.weth.address, setup.weth.address]; + }); + + after(async () => { + airdrops = [setup.usdc.address, setup.weth.address]; + }); + it("should revert", async () => { - await expect(subject()).to.be.revertedWith("At least one token must be passed."); + await expect(subject()).to.be.revertedWith("Duplicate airdrop token passed"); }); }); @@ -989,7 +1017,16 @@ describe("AirdropModule", () => { await subject(); const airdrops = await airdropModule.getAirdrops(setToken.address); + const isAirdrop = await airdropModule.isAirdrop(subjectSetToken, subjectAirdrop); expect(airdrops[2]).to.eq(subjectAirdrop); + expect(isAirdrop).to.be.true; + }); + + it("should emit the correct AirdropComponentAdded event", async () => { + await expect(subject()).to.emit(airdropModule, "AirdropComponentAdded").withArgs( + subjectSetToken, + subjectAirdrop + ); }); describe("when airdrop has already been added", async () => { @@ -1049,7 +1086,16 @@ describe("AirdropModule", () => { await subject(); const airdrops = await airdropModule.getAirdrops(setToken.address); + const isAirdrop = await airdropModule.isAirdrop(subjectSetToken, subjectAirdrop); expect(airdrops).to.not.contain(subjectAirdrop); + expect(isAirdrop).to.be.false; + }); + + it("should emit the correct AirdropComponentRemoved event", async () => { + await expect(subject()).to.emit(airdropModule, "AirdropComponentRemoved").withArgs( + subjectSetToken, + subjectAirdrop + ); }); describe("when airdrop is not in the airdrops array", async () => { @@ -1170,6 +1216,13 @@ describe("AirdropModule", () => { expect(airdropSettings.airdropFee).to.eq(subjectNewFee); }); + it("should emit the correct AirdropFeeUpdated event", async () => { + await expect(subject()).to.emit(airdropModule, "AirdropFeeUpdated").withArgs( + subjectSetToken, + subjectNewFee + ); + }); + describe("when new fee exceeds 100%", async () => { beforeEach(async () => { subjectNewFee = ether(1.1); @@ -1216,6 +1269,7 @@ describe("AirdropModule", () => { let isInitialized: boolean; let subjectSetToken: Address; + let subjectAnyoneAbsorb: boolean; let subjectCaller: Account; before(async () => { @@ -1231,7 +1285,7 @@ describe("AirdropModule", () => { const airdrops = [setup.usdc.address, setup.weth.address]; const airdropFee = ether(.2); - const anyoneAbsorb = true; + const anyoneAbsorb = false; if (isInitialized) { const airdropSettings = { @@ -1244,19 +1298,27 @@ describe("AirdropModule", () => { } subjectSetToken = setToken.address; + subjectAnyoneAbsorb = true; subjectCaller = owner; }); async function subject(): Promise { airdropModule = airdropModule.connect(subjectCaller.wallet); - return airdropModule.updateAnyoneAbsorb(subjectSetToken); + return airdropModule.updateAnyoneAbsorb(subjectSetToken, subjectAnyoneAbsorb); } it("should flip the anyoneAbsorb indicator", async () => { await subject(); const airdropSettings = await airdropModule.airdropSettings(setToken.address); - expect(airdropSettings.anyoneAbsorb).to.be.false; + expect(airdropSettings.anyoneAbsorb).to.be.true; + }); + + it("should emit the correct AnyoneAbsorbUpdated event", async () => { + await expect(subject()).to.emit(airdropModule, "AnyoneAbsorbUpdated").withArgs( + subjectSetToken, + subjectAnyoneAbsorb + ); }); describe("when module is not initialized", async () => { @@ -1340,6 +1402,13 @@ describe("AirdropModule", () => { expect(airdropSettings.feeRecipient).to.eq(subjectNewFeeRecipient); }); + it("should emit the correct FeeRecipientUpdated event", async () => { + await expect(subject()).to.emit(airdropModule, "FeeRecipientUpdated").withArgs( + subjectSetToken, + subjectNewFeeRecipient + ); + }); + describe("when passed address is zero", async () => { beforeEach(async () => { subjectNewFeeRecipient = ADDRESS_ZERO; From dbbbbe81ac7f17d14c6f06602bee391bd29ef813 Mon Sep 17 00:00:00 2001 From: bweick Date: Thu, 1 Jul 2021 19:33:25 -0700 Subject: [PATCH 3/7] Remove .only --- test/protocol/modules/airdropModule.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/protocol/modules/airdropModule.spec.ts b/test/protocol/modules/airdropModule.spec.ts index 761626638..9b5f02e7b 100644 --- a/test/protocol/modules/airdropModule.spec.ts +++ b/test/protocol/modules/airdropModule.spec.ts @@ -23,7 +23,7 @@ import { AirdropSettings } from "@utils/types"; const expect = getWaffleExpect(); -describe.only("AirdropModule", () => { +describe("AirdropModule", () => { let owner: Account; let feeRecipient: Account; let tokenHolder: Account; From 3d07e540cb652c0f5e14347a5f020c4dbd051f91 Mon Sep 17 00:00:00 2001 From: bweick Date: Thu, 1 Jul 2021 19:46:20 -0700 Subject: [PATCH 4/7] Fixed error in other tests. --- test/integration/curveStakingModule.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/curveStakingModule.spec.ts b/test/integration/curveStakingModule.spec.ts index 1a0d12def..8e2bd329e 100644 --- a/test/integration/curveStakingModule.spec.ts +++ b/test/integration/curveStakingModule.spec.ts @@ -107,7 +107,7 @@ describe("curveStakingModule", () => { airdrops: [curveSetup.poolToken.address], airdropFee: ZERO, anyoneAbsorb: true, - feeRecipient: ADDRESS_ZERO, + feeRecipient: owner.address, }); // Issue some Sets From 0ba3104cd4dacf8e72a19a11f87da1ce5eeee4a8 Mon Sep 17 00:00:00 2001 From: bweick Date: Fri, 2 Jul 2021 09:55:55 -0700 Subject: [PATCH 5/7] Fix coverage. --- test/protocol/modules/airdropModule.spec.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/protocol/modules/airdropModule.spec.ts b/test/protocol/modules/airdropModule.spec.ts index 9b5f02e7b..cbc97202a 100644 --- a/test/protocol/modules/airdropModule.spec.ts +++ b/test/protocol/modules/airdropModule.spec.ts @@ -54,6 +54,7 @@ describe("AirdropModule", () => { let airdrops: Address[]; let airdropFee: BigNumber; let anyoneAbsorb: boolean; + let airdropFeeRecipient: Address; let subjectSetToken: Address; let subjectAirdropSettings: AirdropSettings; @@ -63,6 +64,7 @@ describe("AirdropModule", () => { airdrops = [setup.usdc.address, setup.weth.address]; airdropFee = ether(.2); anyoneAbsorb = true; + airdropFeeRecipient = feeRecipient.address; }); beforeEach(async () => { @@ -75,7 +77,7 @@ describe("AirdropModule", () => { subjectSetToken = setToken.address; subjectAirdropSettings = { airdrops, - feeRecipient: feeRecipient.address, + feeRecipient: airdropFeeRecipient, airdropFee, anyoneAbsorb, } as AirdropSettings; @@ -154,6 +156,20 @@ describe("AirdropModule", () => { }); }); + describe("when the fee recipient is the ZERO_ADDRESS", async () => { + before(async () => { + airdropFeeRecipient = ADDRESS_ZERO; + }); + + after(async () => { + airdropFeeRecipient = feeRecipient.address; + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("Zero fee address passed"); + }); + }); + describe("when the caller is not the SetToken manager", async () => { beforeEach(async () => { subjectCaller = tokenHolder; From 8a00cab67b389232768969721f36179b5c1ae8d3 Mon Sep 17 00:00:00 2001 From: bweick Date: Fri, 2 Jul 2021 11:11:32 -0700 Subject: [PATCH 6/7] Add deletion of isAirdrop from removeModule. --- contracts/protocol/modules/AirdropModule.sol | 6 ++++++ test/protocol/modules/airdropModule.spec.ts | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/contracts/protocol/modules/AirdropModule.sol b/contracts/protocol/modules/AirdropModule.sol index ca1f5dc99..4a3b3973d 100644 --- a/contracts/protocol/modules/AirdropModule.sol +++ b/contracts/protocol/modules/AirdropModule.sol @@ -246,6 +246,12 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { * Airdrops are not absorbed. */ function removeModule() external override { + address[] memory airdrops = airdropSettings[ISetToken(msg.sender)].airdrops; + + for (uint256 i =0; i < airdrops.length; i++) { + isAirdrop[ISetToken(msg.sender)][IERC20(airdrops[i])] = false; + } + delete airdropSettings[ISetToken(msg.sender)]; } diff --git a/test/protocol/modules/airdropModule.spec.ts b/test/protocol/modules/airdropModule.spec.ts index cbc97202a..47a28d780 100644 --- a/test/protocol/modules/airdropModule.spec.ts +++ b/test/protocol/modules/airdropModule.spec.ts @@ -981,6 +981,16 @@ describe("AirdropModule", () => { expect(airdropSettings.airdropFee).to.eq(ZERO); expect(airdropSettings.anyoneAbsorb).to.be.false; }); + + it("should reset the isAirdrop mapping", async () => { + await subject(); + + const wethIsAirdrop = await airdropModule.isAirdrop(subjectModule, setup.weth.address); + const usdcIsAirdrop = await airdropModule.isAirdrop(subjectModule, setup.usdc.address); + + expect(wethIsAirdrop).to.be.false; + expect(usdcIsAirdrop).to.be.false; + }); }); describe("CONTEXT: Airdrop add/remove", async () => { From 7dc2478e080487fc6a7b352047748ec563d14fe4 Mon Sep 17 00:00:00 2001 From: Richard Liang Date: Wed, 25 Aug 2021 16:04:45 -0400 Subject: [PATCH 7/7] Address fixes --- contracts/protocol/modules/AirdropModule.sol | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/contracts/protocol/modules/AirdropModule.sol b/contracts/protocol/modules/AirdropModule.sol index 4a3b3973d..4d6e7a3fd 100644 --- a/contracts/protocol/modules/AirdropModule.sol +++ b/contracts/protocol/modules/AirdropModule.sol @@ -286,6 +286,9 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { _handleAirdropPosition(_setToken, _token); } + /** + * Loop through array of tokens and handle airdropped positions. + */ function _batchAbsorb(ISetToken _setToken, address[] memory _tokens) internal { for (uint256 i = 0; i < _tokens.length; i++) { _absorb(_setToken, IERC20(_tokens[i])); @@ -336,12 +339,12 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { if (airdropFee > 0) { totalFees = _amountAirdropped.preciseMul(airdropFee); - protocolTake = ModuleBase.getModuleFee(AIRDROP_MODULE_PROTOCOL_FEE_INDEX, totalFees); + protocolTake = getModuleFee(AIRDROP_MODULE_PROTOCOL_FEE_INDEX, totalFees); netManagerTake = totalFees.sub(protocolTake); _setToken.strictInvokeTransfer(address(_component), airdropSettings[_setToken].feeRecipient, netManagerTake); - ModuleBase.payProtocolFeeFromSetToken(_setToken, address(_component), protocolTake); + payProtocolFeeFromSetToken(_setToken, address(_component), protocolTake); return (netManagerTake, protocolTake, totalFees); } else { @@ -356,8 +359,11 @@ contract AirdropModule is ModuleBase, ReentrancyGuard { ISetToken _setToken, uint256 _totalComponentBalance, uint256 _totalFeesPaid - - ) internal view returns(uint256) { + ) + internal + view + returns(uint256) + { uint256 totalSupply = _setToken.totalSupply(); return totalSupply.getDefaultPositionUnit(_totalComponentBalance.sub(_totalFeesPaid)); }