diff --git a/contracts/bridge/Bridge.sol b/contracts/bridge/Bridge.sol index 8fe8026..edc4368 100644 --- a/contracts/bridge/Bridge.sol +++ b/contracts/bridge/Bridge.sol @@ -62,14 +62,29 @@ contract Bridge is _disableInitializers(); } + /** + * @notice Initializes the contract. + * @param signers_ The initial signers. Refer to the `Signers` contract for detailed limitations and information. + * + * @param pauseManager_ The address of the initial pause manager, which may be set to the zero address. + * When set to the zero address, the contract can be paused or unpaused by either the owner or the signers, + * depending on the `isSignersMode` flag. + * + * @param signaturesThreshold_ The number of signatures required to withdraw tokens or to execute a protected function. + * A list of all protected functions is available in the `IBridge` interface. + * + * @param isSignersMode_ The flag that enables or disables signers mode. When set to `true`, + * the contract requires signatures from the signers for executing a protected function. + */ function __Bridge_init( address[] calldata signers_, + address pauseManager_, uint256 signaturesThreshold_, bool isSignersMode_ ) external initializer { __Signers_init(signers_, signaturesThreshold_, isSignersMode_); - __PauseManager_init(owner()); + __PauseManager_init(pauseManager_); } /* @@ -81,6 +96,14 @@ contract Bridge is /* * @inheritdoc UUPSSignableUpgradeable + * + * @dev Depending on the `isSignersMode` flag in the `Signers` contract, this function requires + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function _authorizeUpgrade( address newImplementation, @@ -215,6 +238,11 @@ contract Bridge is * * @dev Depending on the `isSignersMode` flag in the `Signers` contract, this function requires * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function addHash(bytes32 txHash_, uint256 txNonce_, bytes[] calldata signatures_) external { bytes32 functionData_ = keccak256( diff --git a/contracts/interfaces/bridge/IBridge.sol b/contracts/interfaces/bridge/IBridge.sol index b8ab875..3fcff3f 100644 --- a/contracts/interfaces/bridge/IBridge.sol +++ b/contracts/interfaces/bridge/IBridge.sol @@ -23,7 +23,7 @@ import {INativeHandler} from "../handlers/INativeHandler.sol"; interface IBridge is IERC20Handler, IERC721Handler, IERC1155Handler, INativeHandler { /** * @notice The enum of protected functions. - * Used as a domain separator for the sign hash when the signatures are required to call a protected function. + * Used as a domain separator for the sign hash when the signatures are required to execute a protected function. */ enum ProtectedFunction { None, diff --git a/contracts/utils/PauseManager.sol b/contracts/utils/PauseManager.sol index d307fec..8166abc 100644 --- a/contracts/utils/PauseManager.sol +++ b/contracts/utils/PauseManager.sol @@ -7,8 +7,8 @@ import {IBridge} from "../interfaces/bridge/IBridge.sol"; /** * @title PauseManager Contract - * @notice Extends PausableUpgradeable from OpenZeppelin, extends existing functionality be allowing delegation of pause - * management to a specified address. + * @notice Extends PausableUpgradeable from OpenZeppelin by allowing the delegation of pause management + * to a specified address. */ abstract contract PauseManager is PausableUpgradeable { address private _pauseManager; @@ -27,7 +27,7 @@ abstract contract PauseManager is PausableUpgradeable { event PauseManagerChanged(address indexed newManager); /** - * @notice Ensures the function is callable only by the pause manager maintainer. + * @notice Ensures the function is callable only by the pause manager maintainer(s). */ modifier onlyPauseManagerMaintainer(bytes32 functionData_, bytes[] calldata signatures_) virtual { @@ -54,10 +54,18 @@ abstract contract PauseManager is PausableUpgradeable { /** * @notice Pauses the contract. - * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’ in the `Signers` contract. + * @param signatures_ The signatures of the signers; this field should be empty + * if the `isSignersMode` flag is set to ‘false’ in the `Signers` contract or if the `pauseManager` is not the zero address. * * @dev Depending on the `isSignersMode` flag in the `Signers` contract, this function requires - * either signatures from the signers or that the transaction be sent by the owner or pause manager. + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | `pauseManager` Address | Callable By | + * |----------------------|------------------------|---------------------------| + * | `false` | `address(0)` | Owner | + * | `false` | Not `address(0)` | Pause Manager | + * | `true` | `address(0)` | Signers | + * | `true` | Not `address(0)` | Pause Manager | */ function pause( bytes[] calldata signatures_ @@ -70,10 +78,18 @@ abstract contract PauseManager is PausableUpgradeable { /** * @notice Unpauses the contract. - * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’ in the `Signers` contract. + * @param signatures_ The signatures of the signers; this field should be empty + * if the `isSignersMode` flag is set to ‘false’ in the `Signers` contract or if the `pauseManager` is not the zero address. * * @dev Depending on the `isSignersMode` flag in the `Signers` contract, this function requires - * either signatures from the signers or that the transaction be sent by the owner or pause manager. + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | `pauseManager` Address | Callable By | + * |----------------------|------------------------|---------------------------| + * | `false` | `address(0)` | Owner | + * | `false` | Not `address(0)` | Pause Manager | + * | `true` | `address(0)` | Signers | + * | `true` | Not `address(0)` | Pause Manager | */ function unpause( bytes[] calldata signatures_ @@ -91,11 +107,19 @@ abstract contract PauseManager is PausableUpgradeable { * @notice Transfers pause management to a new address. * Can only be called by a pause manager maintainer(s). * - * @param newManager_ The address of the new pause manager. Must not be the zero address. + * @param newManager_ The address of the new pause manager, which may be the zero address. + * When set to the zero address, the contract can be paused or unpaused by either the owner or the signers, + * depending on the `isSignersMode` flag. + * * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’ in the `Signers` contract. * * @dev Depending on the `isSignersMode` flag in the `Signers` contract, this function requires * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function setPauseManager( address newManager_, diff --git a/contracts/utils/Signers.sol b/contracts/utils/Signers.sol index b1f8de9..ffffe6e 100644 --- a/contracts/utils/Signers.sol +++ b/contracts/utils/Signers.sol @@ -80,7 +80,13 @@ abstract contract Signers is Hashes, OwnableUpgradeable { * @param signaturesThreshold_ The new signature threshold. * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’. * - * @dev Depending on the `isSignersMode` flag, this function requires either signatures from the signers or that the transaction be sent by the owner. + * @dev Depending on the `isSignersMode` flag, this function requires + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function setSignaturesThreshold( uint256 signaturesThreshold_, @@ -105,7 +111,13 @@ abstract contract Signers is Hashes, OwnableUpgradeable { * @param signers_ The new signers to be added. * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’. * - * @dev Depending on the `isSignersMode` flag, this function requires either signatures from the signers or that the transaction be sent by the owner. + * @dev Depending on the `isSignersMode` flag, this function requires + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function addSigners(address[] calldata signers_, bytes[] calldata signatures_) public { bytes32 functionData_ = keccak256( @@ -122,7 +134,13 @@ abstract contract Signers is Hashes, OwnableUpgradeable { * @param signers_ The signers to remove. * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’. * - * @dev Depending on the `isSignersMode` flag, this function requires either signatures from the signers or that the transaction be sent by the owner. + * @dev Depending on the `isSignersMode` flag, this function requires + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function removeSigners(address[] calldata signers_, bytes[] calldata signatures_) public { bytes32 functionData_ = keccak256( @@ -141,7 +159,13 @@ abstract contract Signers is Hashes, OwnableUpgradeable { * @param isSignersMode_ The new signers mode. * @param signatures_ The signatures of the signers; this field should be empty if the `isSignersMode` flag is set to ‘false’. * - * @dev Depending on the `isSignersMode` flag, this function requires either signatures from the signers or that the transaction be sent by the owner. + * @dev Depending on the `isSignersMode` flag, this function requires + * either signatures from the signers or that the transaction be sent by the owner. + * + * | `isSignersMode` Flag | Callable By | + * |----------------------|---------------------------| + * | `false` | Owner | + * | `true` | Signers | */ function toggleSignersMode(bool isSignersMode_, bytes[] calldata signatures_) public { bytes32 functionData_ = keccak256( diff --git a/test/bridge/Bridge.test.ts b/test/bridge/Bridge.test.ts index 8596ec8..b16540a 100644 --- a/test/bridge/Bridge.test.ts +++ b/test/bridge/Bridge.test.ts @@ -20,6 +20,7 @@ describe("Bridge", () => { const hash = ethers.keccak256(ethers.solidityPacked(["bytes32", "uint256"], [txHash, txNonce])); let OWNER: SignerWithAddress; + let PAUSER: SignerWithAddress; let SECOND: SignerWithAddress; let bridge: Bridge; @@ -28,7 +29,7 @@ describe("Bridge", () => { let erc1155: ERC1155MintableBurnable; before("setup", async () => { - [OWNER, SECOND] = await ethers.getSigners(); + [OWNER, PAUSER, SECOND] = await ethers.getSigners(); const Bridge = await ethers.getContractFactory("Bridge"); @@ -39,7 +40,7 @@ describe("Bridge", () => { bridge = Bridge.attach(await proxy.getAddress()) as Bridge; - await bridge.__Bridge_init([OWNER.address], "1", false); + await bridge.__Bridge_init([OWNER.address], PAUSER.address, "1", false); const ERC20MB = await ethers.getContractFactory("ERC20MintableBurnable"); const ERC721MB = await ethers.getContractFactory("ERC721MintableBurnable"); @@ -68,7 +69,7 @@ describe("Bridge", () => { describe("access", () => { it("should not initialize twice", async () => { - await expect(bridge.__Bridge_init([OWNER.address], "1", false)).to.be.rejectedWith( + await expect(bridge.__Bridge_init([OWNER.address], PAUSER.address, "1", false)).to.be.rejectedWith( "Initializable: contract is already initialized", ); }); @@ -130,7 +131,7 @@ describe("Bridge", () => { }); it("should revert if trying to deposit or withdraw when Bridge is paused", async () => { - await bridge.connect(OWNER).pause([]); + await bridge.connect(PAUSER).pause([]); await expect( bridge.depositERC20(await erc20.getAddress(), baseBalance, "receiver", "kovan", ERC20BridgingType.Wrapped), @@ -183,7 +184,7 @@ describe("Bridge", () => { }); it("should revert if trying to deposit or withdraw when Bridge is paused", async () => { - await bridge.connect(OWNER).pause([]); + await bridge.connect(PAUSER).pause([]); await expect( bridge.depositERC721(await erc721.getAddress(), baseId, "receiver", "kovan", ERC20BridgingType.Wrapped), @@ -246,7 +247,7 @@ describe("Bridge", () => { }); it("should revert if trying to deposit or withdraw when Bridge is paused", async () => { - await bridge.connect(OWNER).pause([]); + await bridge.connect(PAUSER).pause([]); await expect( bridge.depositERC1155( @@ -294,7 +295,7 @@ describe("Bridge", () => { }); it("should revert if trying to deposit or withdraw when Bridge is paused", async () => { - await bridge.connect(OWNER).pause([]); + await bridge.connect(PAUSER).pause([]); await expect(bridge.depositNative("receiver", "kovan", { value: baseBalance })).to.be.rejectedWith( "Bridge: operations are not allowed while paused", diff --git a/test/bridge/Upgradeable.test.ts b/test/bridge/Upgradeable.test.ts index c586fe8..d20228a 100644 --- a/test/bridge/Upgradeable.test.ts +++ b/test/bridge/Upgradeable.test.ts @@ -12,6 +12,7 @@ describe("Upgradeable", () => { const reverter = new Reverter(); let OWNER: SignerWithAddress; + let PAUSER: SignerWithAddress; let SECOND: SignerWithAddress; let bridge: Bridge; @@ -21,7 +22,7 @@ describe("Upgradeable", () => { let proxyBridge: Bridge; before("setup", async () => { - [OWNER, SECOND] = await ethers.getSigners(); + [OWNER, PAUSER, SECOND] = await ethers.getSigners(); const Bridge = await ethers.getContractFactory("Bridge"); const ERC1967Proxy = await ethers.getContractFactory("ERC1967Proxy"); @@ -32,7 +33,7 @@ describe("Upgradeable", () => { proxy = await ERC1967Proxy.deploy(await bridge.getAddress(), "0x"); proxyBridge = Bridge__factory.connect(await proxy.getAddress(), OWNER); - await proxyBridge.__Bridge_init([], "1", false); + await proxyBridge.__Bridge_init([], PAUSER.address, "1", false); await reverter.snapshot(); });