Removes butchUpdate, moves additional access in child app to extend…

…ed contract, makes child app upgradeable,

contracts/contracts/TACoApplication.sol

@@ -691,10 +691,13 @@ contract TACoApplication is IApplication, ITACoChildToRoot, OwnableUpgradeable {
             "Only child application allowed to confirm operator"
         );
         address stakingProvider = _stakingProviderFromOperator[_operator];
-        // TODO this case possible only in case of desync
-        require(stakingProvider != address(0), "Operator has no bond with staking provider");
-        StakingProviderInfo storage info = stakingProviderInfo[stakingProvider];
+        // TODO only in case of desync, maybe just exit?
+        // require(stakingProvider != address(0), "Operator has no bond with staking provider");
+        if (stakingProvider == address(0)) {
+            return;
+        }
 
+        StakingProviderInfo storage info = stakingProviderInfo[stakingProvider];
         if (!info.operatorConfirmed) {
             updateRewardInternal(stakingProvider);
             info.operatorConfirmed = true;

contracts/contracts/coordination/Coordinator.sol

@@ -150,15 +150,14 @@ contract Coordinator is AccessControlDefaultAdminRules, FlatRateFeeModel {
     function setProviderPublicKey(BLS12381.G2Point calldata _publicKey) external {
         uint32 lastRitualId = uint32(rituals.length);
         address stakingProvider = application.stakingProviderFromOperator(msg.sender);
-        require(stakingProvider != address(0), "Operator has no bond with staking provider"); // TODO
+        require(stakingProvider != address(0), "Operator has no bond with staking provider");
 
         ParticipantKey memory newRecord = ParticipantKey(lastRitualId, _publicKey);
-        // keysHistory[stakingProvider][-1].publicKey != _publicKey; // TODO it's a question
         keysHistory[stakingProvider].push(newRecord);
 
         emit ParticipantPublicKeySet(lastRitualId, stakingProvider, _publicKey);
         // solhint-disable-next-line avoid-tx-origin
-        require(msg.sender == tx.origin, "Only operator with real address can set public key"); // TODO
+        require(msg.sender == tx.origin, "Only operator with real address can set public key");
         application.confirmOperatorAddress(msg.sender);
     }
 

contracts/contracts/coordination/ITACoRootToChild.sol

@@ -13,6 +13,4 @@ interface ITACoRootToChild {
     function updateOperator(address stakingProvider, address operator) external;
 
     function updateAuthorization(address stakingProvider, uint96 amount) external;
-
-    function batchUpdate(bytes32[] calldata updateInfo) external;
 }

contracts/contracts/coordination/TACoChildApplication.sol

@@ -2,7 +2,8 @@
 
 pragma solidity ^0.8.0;
 
-import "@openzeppelin/contracts/access/AccessControl.sol";
+import "@openzeppelin-upgradeable/contracts/access/AccessControlUpgradeable.sol";
+import "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol";
 import "./ITACoRootToChild.sol";
 import "../../threshold/ITACoChildApplication.sol";
 import "./ITACoChildToRoot.sol";
@@ -11,9 +12,7 @@ import "./ITACoChildToRoot.sol";
  * @title TACoChildApplication
  * @notice TACoChildApplication
  */
-contract TACoChildApplication is AccessControl, ITACoRootToChild, ITACoChildApplication {
-    bytes32 public constant UPDATE_ROLE = keccak256("UPDATE_ROLE");
-
+contract TACoChildApplication is ITACoRootToChild, ITACoChildApplication, Initializable {
     struct StakingProviderInfo {
         address operator;
         bool operatorConfirmed;
@@ -27,27 +26,30 @@ contract TACoChildApplication is AccessControl, ITACoRootToChild, ITACoChildAppl
     mapping(address => StakingProviderInfo) public stakingProviderInfo;
     mapping(address => address) public stakingProviderFromOperator;
 
-    constructor(ITACoChildToRoot _rootApplication, address[] memory updaters) {
+    /**
+     * @dev Checks caller is root application
+     */
+    modifier onlyRootApplication() {
+        require(msg.sender == address(rootApplication), "Caller must be the root application");
+        _;
+    }
+
+    constructor(ITACoChildToRoot _rootApplication) {
         require(
             address(_rootApplication) != address(0),
             "Address for root application must be specified"
         );
         rootApplication = _rootApplication;
-
-        // TODO Issue #112
-        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
-        for (uint256 i = 0; i < updaters.length; i++) {
-            _grantRole(UPDATE_ROLE, updaters[i]);
-        }
     }
 
-    function setCoordinator(address _coordinator) external onlyRole(DEFAULT_ADMIN_ROLE) {
+    /**
+     * @notice Initialize function for using with OpenZeppelin proxy
+     */
+    function initialize(address _coordinator) external initializer {
         require(coordinator == address(0), "Coordinator already set");
         require(_coordinator != address(0), "Coordinator must be specified");
         // require(_coordinator.numberOfRituals() >= 0, "Invalid coordinator");
         coordinator = _coordinator;
-
-        // TODO reset role?
     }
 
     function authorizedStake(address _stakingProvider) external view returns (uint96) {
@@ -57,14 +59,14 @@ contract TACoChildApplication is AccessControl, ITACoRootToChild, ITACoChildAppl
     function updateOperator(
         address stakingProvider,
         address operator
-    ) external override onlyRole(UPDATE_ROLE) {
+    ) external override onlyRootApplication {
         _updateOperator(stakingProvider, operator);
     }
 
     function updateAuthorization(
         address stakingProvider,
         uint96 amount
-    ) external override onlyRole(UPDATE_ROLE) {
+    ) external override onlyRootApplication {
         _updateAuthorization(stakingProvider, amount);
     }
 
@@ -94,29 +96,41 @@ contract TACoChildApplication is AccessControl, ITACoRootToChild, ITACoChildAppl
         }
     }
 
-    function batchUpdate(bytes32[] calldata updateInfo) external override onlyRole(UPDATE_ROLE) {
-        require(updateInfo.length % 2 == 0, "bad length");
-        for (uint256 i = 0; i < updateInfo.length; i += 2) {
-            bytes32 word0 = updateInfo[i];
-            bytes32 word1 = updateInfo[i + 1];
-
-            address provider = address(bytes20(word0));
-            uint96 amount = uint96(uint256(word0));
-            address operator = address(bytes20(word1));
-
-            _updateOperator(provider, operator);
-            _updateAuthorization(provider, amount);
-        }
-    }
-
     function confirmOperatorAddress(address _operator) external override {
         require(msg.sender == coordinator, "Only Coordinator allowed to confirm operator");
         address stakingProvider = stakingProviderFromOperator[_operator];
         StakingProviderInfo storage info = stakingProviderInfo[stakingProvider];
         require(info.authorized > 0, "No stake associated with the operator");
-        // TODO maybe allow second confirmation, just do not send root call
+        // TODO maybe allow second confirmation, just do not send root call?
         require(!info.operatorConfirmed, "Can't confirm same operator twice");
         info.operatorConfirmed = true;
         rootApplication.confirmOperatorAddress(_operator);
     }
 }
+
+contract TestnetTACoChildApplication is AccessControlUpgradeable, TACoChildApplication {
+    bytes32 public constant UPDATE_ROLE = keccak256("UPDATE_ROLE");
+
+    constructor(ITACoChildToRoot _rootApplication) TACoChildApplication(_rootApplication) {}
+
+    function initialize(address _coordinator, address[] memory updaters) external initializer {
+        coordinator = _coordinator;
+        for (uint256 i = 0; i < updaters.length; i++) {
+            _grantRole(UPDATE_ROLE, updaters[i]);
+        }
+    }
+
+    function forceUpdateOperator(
+        address stakingProvider,
+        address operator
+    ) external onlyRole(UPDATE_ROLE) {
+        _updateOperator(stakingProvider, operator);
+    }
+
+    function forceUpdateAuthorization(
+        address stakingProvider,
+        uint96 amount
+    ) external onlyRole(UPDATE_ROLE) {
+        _updateAuthorization(stakingProvider, amount);
+    }
+}

contracts/test/CoordinatorTestSet.sol

@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity ^0.8.0;
+
+import "../threshold/ITACoChildApplication.sol";
+
+/**
+ * @notice Contract for testing Coordinator contract
+ */
+contract ChildApplicationForCoordinatorMock is ITACoChildApplication {
+    mapping(address => uint96) public authorizedStake;
+    mapping(address => address) public operatorFromStakingProvider;
+    mapping(address => address) public stakingProviderFromOperator;
+    mapping(address => bool) public confirmations;
+
+    function updateOperator(address _stakingProvider, address _operator) external {
+        address oldOperator = operatorFromStakingProvider[_stakingProvider];
+        stakingProviderFromOperator[oldOperator] = address(0);
+        operatorFromStakingProvider[_stakingProvider] = _operator;
+        stakingProviderFromOperator[_operator] = _stakingProvider;
+    }
+
+    function updateAuthorization(address _stakingProvider, uint96 _amount) external {
+        authorizedStake[_stakingProvider] = _amount;
+    }
+
+    function confirmOperatorAddress(address _operator) external {
+        confirmations[_operator] = true;
+    }
+}

contracts/test/TACoChildApplicationTestSet.sol

@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity ^0.8.0;
+
+import "../contracts/coordination/ITACoRootToChild.sol";
+
+/**
+ * @notice Contract for testing TACo child application contract
+ */
+contract RootApplicationForTACoChildApplicationMock {
+    ITACoRootToChild public childApplication;
+
+    mapping(address => bool) public confirmations;
+
+    function setChildApplication(ITACoRootToChild _childApplication) external {
+        childApplication = _childApplication;
+    }
+
+    function updateOperator(address _stakingProvider, address _operator) external {
+        childApplication.updateOperator(_stakingProvider, _operator);
+    }
+
+    function updateAuthorization(address _stakingProvider, uint96 _amount) external {
+        childApplication.updateAuthorization(_stakingProvider, _amount);
+    }
+
+    function confirmOperatorAddress(address _operator) external {
+        confirmations[_operator] = true;
+    }
+
+    function resetConfirmation(address _operator) external {
+        confirmations[_operator] = false;
+    }
+}

contracts/xchain/PolygonRoot.sol

@@ -7,8 +7,6 @@ import "../contracts/coordination/ITACoRootToChild.sol";
 contract PolygonRoot is FxBaseRootTunnel, ITACoRootToChild {
     address public immutable rootApplication;
 
-    // bytes public latestData;
-
     constructor(
         address _checkpointManager,
         address _fxRoot,
@@ -29,7 +27,6 @@ contract PolygonRoot is FxBaseRootTunnel, ITACoRootToChild {
     }
 
     function _processMessageFromChild(bytes memory data) internal override {
-        // latestData = data;
         // solhint-disable-next-line avoid-low-level-calls
         rootApplication.call(data);
     }
@@ -57,12 +54,4 @@ contract PolygonRoot is FxBaseRootTunnel, ITACoRootToChild {
         );
         _sendMessageToChild(message);
     }
-
-    function batchUpdate(bytes32[] calldata updateInfo) external override onlyRootApplication {
-        bytes memory message = abi.encodeWithSelector(
-            ITACoRootToChild.batchUpdate.selector,
-            updateInfo
-        );
-        _sendMessageToChild(message);
-    }
 }

tests/application/test_operator.py

@@ -66,8 +66,8 @@ def test_bond_operator(accounts, threshold_staking, taco_application, child_appl
     assert taco_application.stakingProviderFromOperator(staking_provider_4) == ZERO_ADDRESS
 
     # Staking provider can't confirm operator address because there is no operator by default
-    with ape.reverts():
-        child_application.confirmOperatorAddress(staking_provider_1, sender=staking_provider_1)
+    child_application.confirmOperatorAddress(staking_provider_1, sender=staking_provider_1)
+    assert not taco_application.isOperatorConfirmed(staking_provider_1)
 
     # Staking provider can't bond another staking provider as operator
     with ape.reverts():
@@ -195,8 +195,8 @@ def test_bond_operator(accounts, threshold_staking, taco_application, child_appl
     ]
 
     # Now the previous operator can no longer make a confirmation
-    with ape.reverts():
-        child_application.confirmOperatorAddress(operator1, sender=operator1)
+    child_application.confirmOperatorAddress(operator1, sender=operator1)
+    assert not taco_application.isOperatorConfirmed(operator1)
     # Only new operator can
     child_application.confirmOperatorAddress(operator2, sender=operator2)
     assert not taco_application.isOperatorConfirmed(operator1)
@@ -355,9 +355,10 @@ def test_confirm_address(
     min_authorization = MIN_AUTHORIZATION
     min_operator_seconds = MIN_OPERATOR_SECONDS
 
-    # Operator must be associated with staking provider
-    with ape.reverts():
-        child_application.confirmOperatorAddress(staking_provider, sender=staking_provider)
+    # Skips confirmation if operator is not associated with staking provider
+    child_application.confirmOperatorAddress(staking_provider, sender=staking_provider)
+    assert not taco_application.isOperatorConfirmed(staking_provider)
+
     threshold_staking.setRoles(staking_provider, sender=creator)
     threshold_staking.authorizationIncreased(staking_provider, 0, min_authorization, sender=creator)
 

tests/test_coordinator.py

@@ -69,8 +69,7 @@ def treasury(accounts):
 
 @pytest.fixture()
 def application(project, deployer, nodes):
-    dummy = project.Dummy.deploy(sender=deployer)
-    contract = project.TACoChildApplication.deploy(dummy.address, [deployer], sender=deployer)
+    contract = project.ChildApplicationForCoordinatorMock.deploy(sender=deployer)
     for n in nodes:
         contract.updateOperator(n, n, sender=deployer)
         contract.updateAuthorization(n, 42, sender=deployer)
@@ -97,7 +96,6 @@ def coordinator(project, deployer, application, erc20, initiator):
         sender=deployer,
     )
     contract.grantRole(contract.INITIATOR_ROLE(), initiator, sender=admin)
-    application.setCoordinator(contract.address, sender=deployer)
     return contract