From da9a817ad3f540f2f11724b265edebc03d1360a3 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 16:09:37 +0100 Subject: [PATCH 1/8] fix: remove ETH constant and misleading comment (#15) --- contracts/FeeSplitter.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/FeeSplitter.sol b/contracts/FeeSplitter.sol index cb616111..d4353305 100644 --- a/contracts/FeeSplitter.sol +++ b/contracts/FeeSplitter.sol @@ -68,8 +68,6 @@ contract FeeSplitter is Ownable, ReentrancyGuard { /* ----------------------------- VARIABLES ----------------------------- */ - address private constant ETH = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; - /// @dev Map of tokens with the tokenRecords mapping(address => TokenRecords) private tokenRecords; @@ -211,7 +209,7 @@ contract FeeSplitter is Ownable, ReentrancyGuard { /// @notice Returns the amount due to an account. Call releaseToken to withdraw the amount. /// @param _account Account address to check the amount due for - /// @param _token ERC20 payment token address (or ETH_ADDR) + /// @param _token ERC20 payment token address /// @return The total amount due for the requested currency function getAmountDue(address _account, IERC20 _token) public view returns (uint256) { TokenRecords storage _tokenRecords = tokenRecords[address(_token)]; From ac472b425cda9e53a130ecd0149657bdc3ce2481 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 16:25:11 +0100 Subject: [PATCH 2/8] fix: accidentally sent ETH funds (#44) --- contracts/NestedFactory.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/NestedFactory.sol b/contracts/NestedFactory.sol index b04c39a0..75dedd2a 100644 --- a/contracts/NestedFactory.sol +++ b/contracts/NestedFactory.sol @@ -68,7 +68,9 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio } /// @dev Receive function - receive() external payable {} + receive() external payable { + require(msg.sender == address(weth), "NF: ETH_SENDER_NOT_WETH"); + } /* ------------------------------ MODIFIERS ---------------------------- */ @@ -486,6 +488,7 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio bool _fromReserve ) private returns (IERC20, uint256) { if (address(_inputToken) == ETH) { + require(!_fromReserve, "NF: NO_ETH_FROM_RESERVE"); require(address(this).balance >= _inputTokenAmount, "NF: INVALID_AMOUNT_IN"); weth.deposit{ value: _inputTokenAmount }(); return (IERC20(address(weth)), _inputTokenAmount); From 2ab79340b2ad21eaf74d6549a4bb7837e3379fd9 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 16:38:29 +0100 Subject: [PATCH 3/8] fix: destroy fees bulking (#27) --- contracts/NestedFactory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/NestedFactory.sol b/contracts/NestedFactory.sol index 75dedd2a..2c66a500 100644 --- a/contracts/NestedFactory.sol +++ b/contracts/NestedFactory.sol @@ -445,7 +445,7 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio if (success) { require(amounts[1] <= _amountToSpend, "NestedFactory::_safeSubmitOrder: Overspent"); if (_amountToSpend > amounts[1]) { - IERC20(_inputToken).safeTransfer(_msgSender(), _amountToSpend - amounts[1]); + _safeTransferWithFees(IERC20(_inputToken), _amountToSpend - amounts[1], _msgSender(), _nftId); } } else { _safeTransferWithFees(IERC20(_inputToken), _amountToSpend, _msgSender(), _nftId); From d2e48a8950659bd176257df26e346bb87a774bc3 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 18:29:38 +0100 Subject: [PATCH 4/8] fix: delete operator from cache (#18) --- contracts/NestedFactory.sol | 3 ++ contracts/abstracts/MixinOperatorResolver.sol | 2 +- test/unit/NestedFactory.unit.ts | 54 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/contracts/NestedFactory.sol b/contracts/NestedFactory.sol index 2c66a500..acf8ad3d 100644 --- a/contracts/NestedFactory.sol +++ b/contracts/NestedFactory.sol @@ -116,6 +116,9 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio if (operators[i] == operator) { operators[i] = operators[operatorsLength - 1]; operators.pop(); + if (operatorCache[operator].implementation != address(0)) { + delete operatorCache[operator]; // remove from cache + } emit OperatorRemoved(operator); return; } diff --git a/contracts/abstracts/MixinOperatorResolver.sol b/contracts/abstracts/MixinOperatorResolver.sol index 465e7655..5b525796 100644 --- a/contracts/abstracts/MixinOperatorResolver.sol +++ b/contracts/abstracts/MixinOperatorResolver.sol @@ -17,7 +17,7 @@ abstract contract MixinOperatorResolver { OperatorResolver public immutable resolver; /// @dev Cache operators map of the name and Operator struct (address/selector) - mapping(bytes32 => IOperatorResolver.Operator) private operatorCache; + mapping(bytes32 => IOperatorResolver.Operator) internal operatorCache; constructor(address _resolver) { resolver = OperatorResolver(_resolver); diff --git a/test/unit/NestedFactory.unit.ts b/test/unit/NestedFactory.unit.ts index 17a3c1a0..f1edb759 100644 --- a/test/unit/NestedFactory.unit.ts +++ b/test/unit/NestedFactory.unit.ts @@ -156,6 +156,7 @@ describe("NestedFactory", () => { await context.nestedFactory.connect(context.masterDeployer).rebuildCache(); await context.nestedFactory.connect(context.masterDeployer).removeOperator(toBytes32("test")); + // Get the operators from the factory let operators = await context.nestedFactory.resolverOperatorsRequired(); @@ -195,6 +196,59 @@ describe("NestedFactory", () => { operators = await context.nestedFactory.resolverOperatorsRequired(); expect(operators[0]).to.be.equal(context.flatOperatorNameBytes32); }); + + it("remove an operator without rebuilding", async () => { + const testAddress = Wallet.createRandom().address; + const operatorResolver = await context.operatorResolver.connect(context.masterDeployer); + // Add the operator named "test" + await importOperatorsWithSigner( + operatorResolver, + [ + { + name: "test", + contract: testAddress, + signature: "function test()", + }, + ], + context.nestedFactory, + context.masterDeployer, + ); + + await context.nestedFactory.connect(context.masterDeployer).removeOperator(toBytes32("test")); + + // Get the operators from the factory + let operators = await context.nestedFactory.resolverOperatorsRequired(); + + // Must have 2 operators ("ZeroEx" from Fixture and "Flat") + expect(operators.length).to.be.equal(2); + expect(operators[0]).to.be.equal(context.zeroExOperatorNameBytes32); + expect(operators[1]).to.be.equal(context.flatOperatorNameBytes32); + + let orders: OrderStruct[] = [ + buildOrderStruct(toBytes32("test"), context.mockUNI.address, [ + ["address", context.mockDAI.address], + ["address", context.mockUNI.address], + [ + "bytes", + ethers.utils.hexConcat([ + dummyRouterSelector, + abiCoder.encode( + ["address", "address", "uint"], + [context.mockDAI.address, context.mockUNI.address, appendDecimals(5)], + ), + ]), + ], + ]), + ]; + + await expect( + context.nestedFactory + .connect(context.user1) + .create(0, [ + { inputToken: context.mockDAI.address, amount: appendDecimals(5), orders, fromReserve: false }, + ]), + ).to.be.revertedWith("MOR: MISSING_OPERATOR: test"); + }); }); describe("setFeeSplitter()", () => { From 720b4502ca781be8575a131b1db6d98d0bf97629 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 18:50:10 +0100 Subject: [PATCH 5/8] fix: areOperatorsImported wrong logic (#17) --- contracts/OperatorResolver.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/OperatorResolver.sol b/contracts/OperatorResolver.sol index 185e772f..922e55ce 100644 --- a/contracts/OperatorResolver.sol +++ b/contracts/OperatorResolver.sol @@ -39,7 +39,7 @@ contract OperatorResolver is IOperatorResolver, Ownable { require(namesLength == destinations.length, "OR: INPUTS_LENGTH_MUST_MATCH"); for (uint256 i = 0; i < namesLength; i++) { if ( - operators[names[i]].implementation != destinations[i].implementation && + operators[names[i]].implementation != destinations[i].implementation || operators[names[i]].selector != destinations[i].selector ) { return false; From e82319e36f32cb8b500c9b732d2b9669a21c0982 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 18:52:57 +0100 Subject: [PATCH 6/8] fix: add weight require (#10) --- contracts/FeeSplitter.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/FeeSplitter.sol b/contracts/FeeSplitter.sol index d4353305..ae13bf9d 100644 --- a/contracts/FeeSplitter.sol +++ b/contracts/FeeSplitter.sol @@ -130,6 +130,7 @@ contract FeeSplitter is Ownable, ReentrancyGuard { /// @param _accountIndex Account to change the weight of /// @param _weight The new weight function updateShareholder(uint256 _accountIndex, uint96 _weight) external onlyOwner { + require(_weight != 0, "FS: INVALID_WEIGHT"); require(_accountIndex < shareholders.length, "FS: INVALID_ACCOUNT_INDEX"); totalWeights = totalWeights + _weight - shareholders[_accountIndex].weight; require(totalWeights != 0, "FS: TOTAL_WEIGHTS_ZERO"); From 0c717742a11fa31f6e8f9ef83545fc43b37e91d8 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 18:55:39 +0100 Subject: [PATCH 7/8] fix: store check _amount (#6) --- contracts/NestedRecords.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/NestedRecords.sol b/contracts/NestedRecords.sol index ee9f59a2..9fd59579 100644 --- a/contracts/NestedRecords.sol +++ b/contracts/NestedRecords.sol @@ -114,6 +114,7 @@ contract NestedRecords is OwnableFactoryHandler { uint256 _amount, address _reserve ) external onlyFactory { + require(_amount != 0, "NRC: INVALID_AMOUNT"); uint256 amount = records[_nftId].holdings[_token]; if (amount != 0) { require(records[_nftId].reserve == _reserve, "NRC: RESERVE_MISMATCH"); From 1a16a5104bdd3cc4fed320deba9d1d6ab6d630c2 Mon Sep 17 00:00:00 2001 From: "axxe.eth" Date: Wed, 16 Feb 2022 19:03:56 +0100 Subject: [PATCH 8/8] fix: rebuildCache when removing and adding operators (#38) --- contracts/NestedFactory.sol | 4 +++- contracts/abstracts/MixinOperatorResolver.sol | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/NestedFactory.sol b/contracts/NestedFactory.sol index acf8ad3d..09ac521a 100644 --- a/contracts/NestedFactory.sol +++ b/contracts/NestedFactory.sol @@ -106,6 +106,7 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio require(operatorsCache[i] != operator, "NF: EXISTENT_OPERATOR"); } operators.push(operator); + rebuildCache(); emit OperatorAdded(operator); } @@ -118,7 +119,8 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio operators.pop(); if (operatorCache[operator].implementation != address(0)) { delete operatorCache[operator]; // remove from cache - } + } + rebuildCache(); emit OperatorRemoved(operator); return; } diff --git a/contracts/abstracts/MixinOperatorResolver.sol b/contracts/abstracts/MixinOperatorResolver.sol index 5b525796..3584c2a8 100644 --- a/contracts/abstracts/MixinOperatorResolver.sol +++ b/contracts/abstracts/MixinOperatorResolver.sol @@ -28,7 +28,7 @@ abstract contract MixinOperatorResolver { function resolverOperatorsRequired() public view virtual returns (bytes32[] memory) {} /// @notice Rebuild the operatorCache - function rebuildCache() external { + function rebuildCache() public { bytes32[] memory requiredOperators = resolverOperatorsRequired(); bytes32 name; IOperatorResolver.Operator memory destination;