Merge pull request #443 from derekpierre/more-testing

More testing of Signing Contracts

Author: derekpierre

contracts/contracts/coordination/SigningCoordinatorDispatcher.sol

@@ -34,6 +34,9 @@ contract SigningCoordinatorDispatcher is Initializable, OwnableUpgradeable {
         require(chainId != 0, "Invalid chain ID");
         if (chainId != block.chainid) {
             require(l1Sender != address(0), "Invalid L1 sender");
+        } else {
+            // same chain so no L1 sender needed
+            require(l1Sender == address(0), "L1 sender not needed for same chain");
         }
         require(signingCoordinatorChild != address(0), "Invalid target");
         dispatchMap[chainId] = DispatchTarget(l1Sender, signingCoordinatorChild);

contracts/test/SigningCoordinatorTestSet.sol

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+pragma solidity ^0.8.0;
+
+import "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol";
+import "../contracts/coordination/SigningCoordinatorDispatcher.sol";
+import "../contracts/coordination/ISigningCoordinatorChild.sol";
+
+/**
+ * @notice Mock contract for testing SigningCoordinatorDispatcher
+ */
+contract SigningCoordinatorMock is Initializable {
+    SigningCoordinatorDispatcher public signingCoordinatorDispatcher;
+
+    function setDispatcher(SigningCoordinatorDispatcher dispatcher) external {
+        require(address(dispatcher).code.length > 0, "Dispatcher must be contract");
+        signingCoordinatorDispatcher = dispatcher;
+    }
+
+    function callDispatch(uint256 chainId, bytes calldata callData) external {
+        signingCoordinatorDispatcher.dispatch(chainId, callData);
+    }
+}
+
+contract SigningCoordinatorChildMock is ISigningCoordinatorChild {
+    function deployCohortMultiSig(uint32 cohortId, address[] calldata, uint16) external {
+        emit CohortMultisigDeployed(cohortId, address(0));
+    }
+
+    function updateMultiSigParameters(
+        uint32 cohortId,
+        address[] calldata signers,
+        uint16 threshold,
+        bool clearSigners
+    ) external {
+        emit CohortMultisigUpdated(cohortId, address(0), signers, threshold, clearSigners);
+    }
+}
+
+contract L1SenderMock {
+    function sendData(address target, bytes calldata data) external {
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, ) = target.call(data);
+        require(success, "Execution failed");
+    }
+}

tests/test_signing_coordinator_child.py

@@ -0,0 +1,197 @@
+import os
+
+import ape
+import pytest
+from eth_utils import to_checksum_address
+
+NUM_SIGNERS = 5
+THRESHOLD = 2
+
+
+@pytest.fixture(scope="module")
+def signers(accounts):
+    _accounts = [acc.address for acc in accounts[:NUM_SIGNERS]]
+    return _accounts
+
+
+@pytest.fixture(scope="module")
+def deployer(accounts):
+    deployer = accounts[NUM_SIGNERS + 1]
+    return deployer
+
+
+@pytest.fixture(scope="module")
+def allowed_caller(accounts):
+    return accounts[NUM_SIGNERS + 2]
+
+
+@pytest.fixture(scope="module")
+def unauthorized_caller(accounts):
+    return accounts[NUM_SIGNERS + 3]
+
+
+@pytest.fixture()
+def signing_coordinator_child(
+    project, oz_dependency, deployer, allowed_caller, threshold_signing_multisig_impl
+):
+    contract = project.SigningCoordinatorChild.deploy(
+        sender=deployer,
+    )
+    proxy = oz_dependency.TransparentUpgradeableProxy.deploy(
+        contract.address,
+        deployer,
+        b"",
+        sender=deployer,
+    )
+    proxy_contract = project.SigningCoordinatorChild.at(proxy.address)
+
+    signing_factory_contract = project.ThresholdSigningMultisigCloneFactory.deploy(
+        threshold_signing_multisig_impl.address,
+        proxy_contract.address,
+        sender=deployer,
+    )
+
+    proxy_contract.initialize(signing_factory_contract.address, allowed_caller, sender=deployer)
+    assert proxy_contract.allowedCaller() == allowed_caller.address
+    assert proxy_contract.signingMultisigFactory() == signing_factory_contract.address
+    assert proxy_contract.owner() == deployer.address
+
+    return proxy_contract
+
+
+@pytest.fixture(scope="module")
+def threshold_signing_multisig_impl(project, deployer):
+    threshold_signing_multisig_impl = project.ThresholdSigningMultisig.deploy(
+        sender=deployer,
+    )
+    return threshold_signing_multisig_impl
+
+
+def test_set_multisig_factory(
+    project,
+    deployer,
+    allowed_caller,
+    unauthorized_caller,
+    signing_coordinator_child,
+    threshold_signing_multisig_impl,
+):
+    new_factory_contract = project.ThresholdSigningMultisigCloneFactory.deploy(
+        threshold_signing_multisig_impl.address,
+        signing_coordinator_child.address,
+        sender=deployer,
+    )
+
+    # must be owner
+    with ape.reverts(f"account={unauthorized_caller.address}"):
+        signing_coordinator_child.setMultisigFactory(
+            new_factory_contract.address, sender=unauthorized_caller
+        )
+
+    with ape.reverts(f"account={allowed_caller.address}"):
+        signing_coordinator_child.setMultisigFactory(
+            new_factory_contract.address, sender=allowed_caller
+        )
+
+    signing_coordinator_child.setMultisigFactory(new_factory_contract.address, sender=deployer)
+    assert signing_coordinator_child.signingMultisigFactory() == new_factory_contract.address
+
+
+def test_set_allowed_caller(
+    deployer, allowed_caller, unauthorized_caller, signing_coordinator_child
+):
+    # must be owner
+    with ape.reverts(f"account={unauthorized_caller.address}"):
+        signing_coordinator_child.setAllowedCaller(
+            unauthorized_caller.address, sender=unauthorized_caller
+        )
+
+    with ape.reverts(f"account={allowed_caller.address}"):
+        signing_coordinator_child.setAllowedCaller(allowed_caller.address, sender=allowed_caller)
+
+    assert signing_coordinator_child.owner() == deployer.address
+    signing_coordinator_child.setAllowedCaller(unauthorized_caller.address, sender=deployer)
+    assert signing_coordinator_child.allowedCaller() == unauthorized_caller.address
+
+
+def test_deploy_cohort_multisig(
+    project, deployer, allowed_caller, signers, signing_coordinator_child
+):
+    cohort_id = 42
+
+    # must be allowed caller
+    with ape.reverts("Unauthorized caller"):
+        signing_coordinator_child.deployCohortMultiSig(
+            cohort_id, signers, THRESHOLD, sender=deployer
+        )
+
+    signing_coordinator_child.deployCohortMultiSig(
+        cohort_id, signers, THRESHOLD, sender=allowed_caller
+    )
+    cohort_42_multisig_address = signing_coordinator_child.cohortMultisigs(cohort_id)
+    cohort_42_multisig_contract = project.ThresholdSigningMultisig.at(cohort_42_multisig_address)
+    assert cohort_42_multisig_contract.getSigners() == signers
+    assert cohort_42_multisig_contract.threshold() == THRESHOLD
+
+    # deploying again for the same cohort should fail
+    with ape.reverts("Multisig already deployed"):
+        signing_coordinator_child.deployCohortMultiSig(
+            cohort_id, signers, THRESHOLD, sender=allowed_caller
+        )
+
+    # deploy for other cohort
+    other_cohort_id = 43
+    cohort_43_threshold = 1
+    cohort_43_signers = [
+        to_checksum_address(os.urandom(20)) for _ in range(cohort_43_threshold + 1)
+    ]
+    signing_coordinator_child.deployCohortMultiSig(
+        other_cohort_id, cohort_43_signers, cohort_43_threshold, sender=allowed_caller
+    )
+    cohort_43_multisig_address = signing_coordinator_child.cohortMultisigs(other_cohort_id)
+    # different address from 1st one
+    assert cohort_43_multisig_address != cohort_42_multisig_address
+
+    cohort_43_multisig_contract = project.ThresholdSigningMultisig.at(cohort_43_multisig_address)
+    assert cohort_43_multisig_contract.getSigners() == cohort_43_signers
+    assert cohort_43_multisig_contract.threshold() == cohort_43_threshold
+
+
+def test_update_multisig_parameters(
+    project, deployer, allowed_caller, unauthorized_caller, signers, signing_coordinator_child
+):
+    cohort_id = 100
+    signing_coordinator_child.deployCohortMultiSig(
+        cohort_id, signers, THRESHOLD, sender=allowed_caller
+    )
+    cohort_multisig_address = signing_coordinator_child.cohortMultisigs(cohort_id)
+    cohort_multisig_contract = project.ThresholdSigningMultisig.at(cohort_multisig_address)
+
+    new_threshold = 3
+    new_signers = [to_checksum_address(os.urandom(20)) for _ in range(NUM_SIGNERS + 2)]
+
+    # must be allowed caller
+    with ape.reverts("Unauthorized caller"):
+        signing_coordinator_child.updateMultiSigParameters(
+            cohort_id, signers, new_threshold, True, sender=deployer
+        )
+
+    # must have already been deployed
+    with ape.reverts("Multisig not deployed"):
+        signing_coordinator_child.updateMultiSigParameters(
+            11, signers, new_threshold, True, sender=allowed_caller
+        )
+
+    # don't replace, bulk add and update
+    signing_coordinator_child.updateMultiSigParameters(
+        cohort_id, new_signers, new_threshold, False, sender=allowed_caller
+    )
+    assert cohort_multisig_contract.threshold() == new_threshold
+    assert len(cohort_multisig_contract.getSigners()) == len(signers) + len(new_signers)
+    assert set(cohort_multisig_contract.getSigners()) == set(signers).union(set(new_signers))
+
+    # totally replace
+    signing_coordinator_child.updateMultiSigParameters(
+        cohort_id, new_signers, new_threshold, True, sender=allowed_caller
+    )
+    assert cohort_multisig_contract.threshold() == new_threshold
+    assert cohort_multisig_contract.getSigners() == new_signers