[CodeArena] - Med/High Risk fixes

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;
 
@@ -132,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");
@@ -211,7 +210,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)];

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 ---------------------------- */
 
@@ -104,6 +106,7 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, OwnableProxyDelegatio
             require(operatorsCache[i] != operator, "NF: EXISTENT_OPERATOR");
         }
         operators.push(operator);
+        rebuildCache();
         emit OperatorAdded(operator);
     }
 
@@ -114,6 +117,10 @@ 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
+                }
+                rebuildCache();   
                 emit OperatorRemoved(operator);
                 return;
             }
@@ -443,7 +450,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);
@@ -486,6 +493,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);

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");

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;

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);
@@ -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;

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()", () => {