diff --git a/contracts/bridge/Bridge.sol b/contracts/bridge/Bridge.sol index f675e8c..ac1ec79 100644 --- a/contracts/bridge/Bridge.sol +++ b/contracts/bridge/Bridge.sol @@ -1,8 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.9; -import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; - import {IBridge} from "../interfaces/bridge/IBridge.sol"; import {ERC20Handler} from "../handlers/ERC20Handler.sol"; @@ -10,18 +8,17 @@ import {ERC721Handler} from "../handlers/ERC721Handler.sol"; import {ERC1155Handler} from "../handlers/ERC1155Handler.sol"; import {NativeHandler} from "../handlers/NativeHandler.sol"; -import {Hashes} from "../utils/Hashes.sol"; import {Signers} from "../utils/Signers.sol"; import {PauseManager} from "../utils/PauseManager.sol"; +import {UUPSSignableUpgradeable} from "../utils/UUPSSignableUpgradeable.sol"; /** * @title Bridge Contract */ contract Bridge is IBridge, - UUPSUpgradeable, + UUPSSignableUpgradeable, Signers, - Hashes, PauseManager, ERC20Handler, ERC721Handler, @@ -31,8 +28,9 @@ contract Bridge is /** * @dev Ensures the function is callable only by the pause manager maintainer. */ - modifier onlyPauseManagerMaintainer() override { - _checkOwner(); + modifier onlyPauseManagerMaintainer(bytes32 functionData_, bytes[] calldata signatures_) + override { + _checkOwnerOrSignatures(functionData_, signatures_); _; } @@ -54,17 +52,28 @@ contract Bridge is function __Bridge_init( address[] calldata signers_, - uint256 signaturesThreshold_ + uint256 signaturesThreshold_, + bool isSignersMode_ ) external initializer { - __Signers_init(signers_, signaturesThreshold_); + __Signers_init(signers_, signaturesThreshold_, isSignersMode_); __PauseManager_init(owner()); } - /** - * @inheritdoc UUPSUpgradeable - */ - function _authorizeUpgrade(address newImplementation) internal override onlyOwner {} + function _authorizeUpgrade(address) internal pure override { + revert("Bridge: this upgrade method is turned off"); + } + + function _authorizeUpgrade( + address newImplementation, + bytes[] calldata signatures_ + ) internal override { + bytes32 functionData_ = keccak256( + abi.encodePacked(IBridge.ProtectedFunction.BridgeUpgrade, newImplementation) + ); + + _checkOwnerOrSignatures(functionData_, signatures_); + } /** * @inheritdoc IBridge @@ -183,12 +192,14 @@ contract Bridge is /** * @notice The function to add a new hash */ - function addHash(bytes32 txHash_, uint256 txNonce_) external onlyOwner { - _checkAndUpdateHashes(txHash_, txNonce_); - } + function addHash(bytes32 txHash_, uint256 txNonce_, bytes[] calldata signatures_) external { + bytes32 functionData_ = keccak256( + abi.encodePacked(IBridge.ProtectedFunction.AddHash, txHash_, txNonce_) + ); + + _checkOwnerOrSignatures(functionData_, signatures_); - function _checkOwner() internal view { - require(owner() == _msgSender(), "Bridge: caller is not the owner"); + _checkAndUpdateHashes(txHash_, txNonce_); } function _checkNotStopped() internal view { diff --git a/contracts/interfaces/bridge/IBridge.sol b/contracts/interfaces/bridge/IBridge.sol index b8a76ac..f18bdb5 100644 --- a/contracts/interfaces/bridge/IBridge.sol +++ b/contracts/interfaces/bridge/IBridge.sol @@ -21,6 +21,17 @@ import {INativeHandler} from "../handlers/INativeHandler.sol"; * All signer addresses must differ in their first (most significant) 8 bits in order to pass a bloom filtering. */ interface IBridge is IERC20Handler, IERC721Handler, IERC1155Handler, INativeHandler { + enum ProtectedFunction { + None, + AddHash, + BridgeUpgrade, + SetPauseManager, + SetSignaturesThreshold, + AddSigners, + RemoveSigners, + ToggleSignersMode + } + /** * @notice Withdraws ERC20 tokens. * @param token_ The address of the token to withdraw. diff --git a/contracts/mocks/utils/PauseManagerMock.sol b/contracts/mocks/utils/PauseManagerMock.sol index b4ed6dc..2faee7c 100644 --- a/contracts/mocks/utils/PauseManagerMock.sol +++ b/contracts/mocks/utils/PauseManagerMock.sol @@ -3,10 +3,13 @@ pragma solidity ^0.8.9; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import {IBridge} from "../../interfaces/bridge/IBridge.sol"; + import {PauseManager} from "../../utils/PauseManager.sol"; contract PauseManagerMock is PauseManager, OwnableUpgradeable { - modifier onlyPauseManagerMaintainer() override { + modifier onlyPauseManagerMaintainer(bytes32 functionData_, bytes[] calldata signatures_) + override { _checkOwner(); _; } @@ -28,8 +31,9 @@ contract PauseManagerMock is PauseManager, OwnableUpgradeable { contract PauseManagerMockCoverage is PauseManager { function __PauseManagerMock_init( - address initialOwner_ - ) public initializer onlyPauseManagerMaintainer { + address initialOwner_, + bytes[] calldata signatures_ + ) public initializer onlyPauseManagerMaintainer(bytes32(0), signatures_) { __PauseManager_init(initialOwner_); } } diff --git a/contracts/mocks/utils/SignersMock.sol b/contracts/mocks/utils/SignersMock.sol index 340857e..d0739a1 100644 --- a/contracts/mocks/utils/SignersMock.sol +++ b/contracts/mocks/utils/SignersMock.sol @@ -6,9 +6,10 @@ import {Signers} from "../../utils/Signers.sol"; contract SignersMock is Signers { function __SignersMock_init( address[] calldata signers_, - uint256 signaturesThreshold_ + uint256 signaturesThreshold_, + bool isSignersMode_ ) public initializer { - __Signers_init(signers_, signaturesThreshold_); + __Signers_init(signers_, signaturesThreshold_, isSignersMode_); } function checkSignatures(bytes32 signHash_, bytes[] calldata signatures_) external view { diff --git a/contracts/utils/PauseManager.sol b/contracts/utils/PauseManager.sol index ab6815f..f07c900 100644 --- a/contracts/utils/PauseManager.sol +++ b/contracts/utils/PauseManager.sol @@ -3,6 +3,8 @@ pragma solidity ^0.8.9; import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol"; +import {IBridge} from "../interfaces/bridge/IBridge.sol"; + /** * @title PauseManager Contract * @notice Extends PausableUpgradeable from OpenZeppelin, extends existing functionality be allowing delegation of pause @@ -27,7 +29,8 @@ abstract contract PauseManager is PausableUpgradeable { /** * @notice Modifier to make a function callable only by the pause manager maintainer. */ - modifier onlyPauseManagerMaintainer() virtual { + modifier onlyPauseManagerMaintainer(bytes32 functionData_, bytes[] calldata signatures_) + virtual { _; } @@ -73,7 +76,16 @@ abstract contract PauseManager is PausableUpgradeable { * * @param newManager_ The address of the new pause manager. Must not be the zero address. */ - function setPauseManager(address newManager_) public onlyPauseManagerMaintainer { + function setPauseManager( + address newManager_, + bytes[] calldata signatures_ + ) + public + onlyPauseManagerMaintainer( + keccak256(abi.encodePacked(IBridge.ProtectedFunction.SetPauseManager, newManager_)), + signatures_ + ) + { require(newManager_ != address(0), "PauseManager: zero address"); _setPauseManager(newManager_); diff --git a/contracts/utils/Signers.sol b/contracts/utils/Signers.sol index c18788c..0308cd5 100644 --- a/contracts/utils/Signers.sol +++ b/contracts/utils/Signers.sol @@ -1,60 +1,121 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.9; +import {Counters} from "@openzeppelin/contracts/utils/Counters.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; -abstract contract Signers is OwnableUpgradeable { +import {Hashes} from "./Hashes.sol"; + +import {IBridge} from "../interfaces/bridge/IBridge.sol"; + +abstract contract Signers is Hashes, OwnableUpgradeable { using ECDSA for bytes32; using EnumerableSet for EnumerableSet.AddressSet; + using Counters for Counters.Counter; + uint256 public signaturesThreshold; + bool public isSignersMode; + EnumerableSet.AddressSet internal _signers; + mapping(bytes32 => Counters.Counter) private _nonces; + /** * @dev This empty reserved space is put in place to allow future versions to add new * variables without shifting down storage in the inheritance chain. * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps */ - uint256[48] private __gap; + uint256[46] private __gap; function __Signers_init( address[] calldata signers_, - uint256 signaturesThreshold_ + uint256 signaturesThreshold_, + bool isSignersMode_ ) public onlyInitializing { __Ownable_init(); - addSigners(signers_); - setSignaturesThreshold(signaturesThreshold_); + _addSigners(signers_); + + signaturesThreshold = signaturesThreshold_; + isSignersMode = isSignersMode_; } - function setSignaturesThreshold(uint256 signaturesThreshold_) public onlyOwner { + function setSignaturesThreshold( + uint256 signaturesThreshold_, + bytes[] calldata signatures_ + ) public { + bytes32 functionData_ = keccak256( + abi.encodePacked( + IBridge.ProtectedFunction.SetSignaturesThreshold, + signaturesThreshold_ + ) + ); + + _checkOwnerOrSignatures(functionData_, signatures_); + require(signaturesThreshold_ > 0, "Signers: invalid threshold"); signaturesThreshold = signaturesThreshold_; } - function addSigners(address[] calldata signers_) public onlyOwner { - for (uint256 i = 0; i < signers_.length; i++) { - require(signers_[i] != address(0), "Signers: zero signer"); + function addSigners(address[] calldata signers_, bytes[] calldata signatures_) public { + bytes32 functionData_ = keccak256( + abi.encodePacked(IBridge.ProtectedFunction.AddSigners, signers_) + ); - _signers.add(signers_[i]); - } + _checkOwnerOrSignatures(functionData_, signatures_); + + _addSigners(signers_); } - function removeSigners(address[] calldata signers_) public onlyOwner { + function removeSigners(address[] calldata signers_, bytes[] calldata signatures_) public { + bytes32 functionData_ = keccak256( + abi.encodePacked(IBridge.ProtectedFunction.RemoveSigners, signers_) + ); + + _checkOwnerOrSignatures(functionData_, signatures_); + for (uint256 i = 0; i < signers_.length; i++) { _signers.remove(signers_[i]); } } + function toggleSignersMode(bool isSignersMode_, bytes[] calldata signatures_) public { + bytes32 functionData_ = keccak256( + abi.encodePacked(IBridge.ProtectedFunction.ToggleSignersMode, isSignersMode_) + ); + + _checkOwnerOrSignatures(functionData_, signatures_); + + isSignersMode = isSignersMode_; + } + function getSigners() external view returns (address[] memory) { return _signers.values(); } - function _checkSignatures(bytes32 signHash_, bytes[] calldata signatures_) internal view { + function getFunctionSignHash( + bytes32 functionData_, + uint256 nonce_, + address contract_, + uint256 chainId_ + ) public pure returns (bytes32) { + return keccak256(abi.encodePacked(functionData_, nonce_, contract_, chainId_)); + } + + function _addSigners(address[] calldata signers_) private { + for (uint256 i = 0; i < signers_.length; i++) { + require(signers_[i] != address(0), "Signers: zero signer"); + + _signers.add(signers_[i]); + } + } + + function _checkSignatures(bytes32 signHash_, bytes[] memory signatures_) internal view { address[] memory signers_ = new address[](signatures_.length); for (uint256 i = 0; i < signatures_.length; i++) { @@ -79,4 +140,39 @@ abstract contract Signers is OwnableUpgradeable { require(signers_.length >= signaturesThreshold, "Signers: threshold is not met"); } + + function _checkOwnerOrSignatures( + bytes32 functionData_, + bytes[] calldata signatures_ + ) internal { + if (isSignersMode) { + bytes32 signHash_ = getFunctionSignHash( + functionData_, + _useNonce(functionData_), + address(this), + block.chainid + ); + + _checkSignatures(signHash_, signatures_); + } else { + require(msg.sender == owner(), "Ownable: caller is not the owner"); + } + } + + /** + * @notice Returns the current nonce for `ProtectedFunction`. This value must be + * included whenever a signature is generated. + */ + function nonces(bytes32 functionData_) public view virtual returns (uint256) { + return _nonces[functionData_].current(); + } + + /** + * @notice "Consume a nonce": return the current value and increment. + */ + function _useNonce(bytes32 functionData_) internal virtual returns (uint256 current) { + Counters.Counter storage nonce = _nonces[functionData_]; + current = nonce.current(); + nonce.increment(); + } } diff --git a/contracts/utils/UUPSSignableUpgradeable.sol b/contracts/utils/UUPSSignableUpgradeable.sol new file mode 100644 index 0000000..b3d2c82 --- /dev/null +++ b/contracts/utils/UUPSSignableUpgradeable.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.9; + +import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; + +abstract contract UUPSSignableUpgradeable is UUPSUpgradeable { + function _authorizeUpgrade( + address newImplementation_, + bytes[] calldata signatures_ + ) internal virtual; + + function upgradeToWithSig( + address newImplementation_, + bytes[] calldata signatures_ + ) external virtual onlyProxy { + _authorizeUpgrade(newImplementation_, signatures_); + _upgradeToAndCallUUPS(newImplementation_, new bytes(0), false); + } +} diff --git a/test/bridge/Bridge.test.ts b/test/bridge/Bridge.test.ts index e28dfc8..bd5d8ed 100644 --- a/test/bridge/Bridge.test.ts +++ b/test/bridge/Bridge.test.ts @@ -4,15 +4,9 @@ import { ethers } from "hardhat"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; import { wei } from "@scripts"; -import { ERC20BridgingType, getSignature, Reverter } from "@test-helpers"; +import { ERC20BridgingType, getSignature, ProtectedFunction, Reverter } from "@test-helpers"; -import { - ERC20MintableBurnable, - Bridge, - ERC721MintableBurnable, - ERC1155MintableBurnable, - Bridge__factory, -} from "@ethers-v6"; +import { ERC20MintableBurnable, Bridge, ERC721MintableBurnable, ERC1155MintableBurnable } from "@ethers-v6"; describe("Bridge", () => { const reverter = new Reverter(); @@ -45,7 +39,7 @@ describe("Bridge", () => { bridge = Bridge.attach(await proxy.getAddress()) as Bridge; - await bridge.__Bridge_init([OWNER.address], "1"); + await bridge.__Bridge_init([OWNER.address], "1", false); const ERC20MB = await ethers.getContractFactory("ERC20MintableBurnable"); const ERC721MB = await ethers.getContractFactory("ERC721MintableBurnable"); @@ -74,7 +68,7 @@ describe("Bridge", () => { describe("access", () => { it("should not initialize twice", async () => { - await expect(bridge.__Bridge_init([OWNER.address], "1")).to.be.rejectedWith( + await expect(bridge.__Bridge_init([OWNER.address], "1", false)).to.be.rejectedWith( "Initializable: contract is already initialized", ); }); @@ -88,17 +82,17 @@ describe("Bridge", () => { await expect(erc721.burnFrom(OWNER.address, 1)).to.be.rejectedWith("Ownable: caller is not the owner"); await expect(erc1155.burnFrom(OWNER.address, 1, 1)).to.be.rejectedWith("Ownable: caller is not the owner"); - await expect(bridge.connect(SECOND).addHash(txHash, txNonce)).to.be.rejectedWith( + await expect(bridge.connect(SECOND).addHash(txHash, txNonce, [])).to.be.rejectedWith( "Ownable: caller is not the owner", ); }); it("should set the pause manager only by the owner", async () => { - await expect(bridge.connect(SECOND).setPauseManager(OWNER.address)).to.be.rejectedWith( - "Bridge: caller is not the owner", + await expect(bridge.connect(SECOND).setPauseManager(OWNER.address, [])).to.be.rejectedWith( + "Ownable: caller is not the owner", ); - await bridge.connect(OWNER).setPauseManager(SECOND.address); + await bridge.connect(OWNER).setPauseManager(SECOND.address, []); }); }); @@ -316,9 +310,59 @@ describe("Bridge", () => { it("should add hash", async () => { expect(await bridge.usedHashes(hash)).to.be.false; - await bridge.addHash(txHash, txNonce); + await bridge.addHash(txHash, txNonce, []); + + expect(await bridge.usedHashes(hash)).to.be.true; + }); + + it("should add hash with signers", async () => { + await bridge.toggleSignersMode(true, []); + + await expect(bridge.addHash(txHash, txNonce, [])).to.be.rejectedWith("Signers: threshold is not met"); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "bytes32", "uint256"], + [ProtectedFunction.AddHash, txHash, txNonce], + ); + + const signHash = await bridge.getFunctionSignHash( + functionData, + await bridge.nonces(functionData), + await bridge.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await bridge.addHash(txHash, txNonce, [signature]); expect(await bridge.usedHashes(hash)).to.be.true; }); }); + + it("should set the pause manager and emit 'PauseManagerChanged' event with signers", async () => { + await bridge.toggleSignersMode(true, []); + + await expect(bridge.setPauseManager(OWNER.address, [])).to.be.rejectedWith("Signers: threshold is not met"); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "address"], + [ProtectedFunction.SetPauseManager, OWNER.address], + ); + + const signHash = await bridge.getFunctionSignHash( + functionData, + await bridge.nonces(functionData), + await bridge.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(bridge.setPauseManager(OWNER.address, [signature])) + .to.emit(bridge, "PauseManagerChanged") + .withArgs(OWNER.address); + + expect(await bridge.pauseManager()).to.equal(OWNER.address); + }); }); diff --git a/test/bridge/Upgradeable.test.ts b/test/bridge/Upgradeable.test.ts index fc9156b..c586fe8 100644 --- a/test/bridge/Upgradeable.test.ts +++ b/test/bridge/Upgradeable.test.ts @@ -4,7 +4,7 @@ import { ethers } from "hardhat"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; import { wei } from "@scripts"; -import { Reverter } from "@test-helpers"; +import { getSignature, ProtectedFunction, Reverter } from "@test-helpers"; import { ERC1967Proxy, Bridge, Bridge__factory } from "@ethers-v6"; @@ -32,19 +32,56 @@ describe("Upgradeable", () => { proxy = await ERC1967Proxy.deploy(await bridge.getAddress(), "0x"); proxyBridge = Bridge__factory.connect(await proxy.getAddress(), OWNER); - await proxyBridge.__Bridge_init([], "1"); + await proxyBridge.__Bridge_init([], "1", false); await reverter.snapshot(); }); afterEach(reverter.revert); + it("should revert if trying to upgrade with turned off method", async () => { + await expect(proxyBridge.upgradeTo(await newBridge.getAddress())).to.be.rejectedWith( + "Bridge: this upgrade method is turned off", + ); + }); + + it("should revert if trying to call `upgradeToWithSig` on implementation", async () => { + await expect(newBridge.upgradeToWithSig(await newBridge.getAddress(), [])).to.be.rejectedWith( + "Function must be called through delegatecall", + ); + }); + it("should upgrade implementation", async () => { - await expect(proxyBridge.upgradeTo(await newBridge.getAddress())).to.be.eventually.fulfilled; + await expect(proxyBridge.upgradeToWithSig(await newBridge.getAddress(), [])).to.be.eventually.fulfilled; + }); + + it("should upgrade implementation with signers", async () => { + await proxyBridge.addSigners([OWNER.address, SECOND.address], []); + await proxyBridge.toggleSignersMode(true, []); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "address"], + [ProtectedFunction.BridgeUpgrade, await newBridge.getAddress()], + ); + + const signHash = await proxyBridge.getFunctionSignHash( + functionData, + await proxyBridge.nonces(functionData), + await proxyBridge.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(proxyBridge.upgradeToWithSig(await proxyBridge.getAddress(), [signature])).to.be.rejectedWith( + "Signers: invalid signer", + ); + + await proxyBridge.upgradeToWithSig(await newBridge.getAddress(), [signature]); }); it("should revert when call from non owner address", async () => { - await expect(proxyBridge.connect(SECOND).upgradeTo(await newBridge.getAddress())).to.be.rejectedWith( + await expect(proxyBridge.connect(SECOND).upgradeToWithSig(await newBridge.getAddress(), [])).to.be.rejectedWith( "Ownable: caller is not the owner", ); }); diff --git a/test/helpers/constants.ts b/test/helpers/constants.ts index f62113d..6e5aabc 100644 --- a/test/helpers/constants.ts +++ b/test/helpers/constants.ts @@ -13,3 +13,14 @@ export enum ERC1155BridgingType { LiquidityPool, Wrapped, } + +export enum ProtectedFunction { + None, + AddHash, + BridgeUpgrade, + SetPauseManager, + SetSignersThreshold, + AddSigners, + RemoveSigners, + ToggleSignersMode, +} diff --git a/test/utils/PauseManager.test.ts b/test/utils/PauseManager.test.ts index f214dde..bf8b232 100644 --- a/test/utils/PauseManager.test.ts +++ b/test/utils/PauseManager.test.ts @@ -67,13 +67,13 @@ describe("PauseManager", () => { describe("setPauseManager", () => { it("should revert if trying to set the pause manager to the zero address", async () => { - await expect(pauseManagerMock.connect(OWNER).setPauseManager(ethers.ZeroAddress)).to.be.rejectedWith( + await expect(pauseManagerMock.connect(OWNER).setPauseManager(ethers.ZeroAddress, [])).to.be.rejectedWith( "PauseManager: zero address", ); }); it("should set the pause manager and emit 'PauseManagerChanged' event", async () => { - await expect(pauseManagerMock.connect(OWNER).setPauseManager(OWNER.address)) + await expect(pauseManagerMock.connect(OWNER).setPauseManager(OWNER.address, [])) .to.emit(pauseManagerMock, "PauseManagerChanged") .withArgs(OWNER.address); @@ -81,7 +81,7 @@ describe("PauseManager", () => { }); it("should revert if trying to set the pause manager not by the owner", async () => { - await expect(pauseManagerMock.connect(PAUSER).setPauseManager(OWNER.address)).to.be.rejectedWith( + await expect(pauseManagerMock.connect(PAUSER).setPauseManager(OWNER.address, [])).to.be.rejectedWith( "PauseManagerMock: caller is not the owner", ); }); @@ -92,7 +92,7 @@ describe("PauseManager", () => { const PauseManagerMock = await ethers.getContractFactory("PauseManagerMockCoverage"); const pauseManagerMock = await PauseManagerMock.deploy(); - await pauseManagerMock.__PauseManagerMock_init(PAUSER.address); + await pauseManagerMock.__PauseManagerMock_init(PAUSER.address, []); }); }); }); diff --git a/test/utils/Signers.test.ts b/test/utils/Signers.test.ts index 1835d95..3e2724b 100644 --- a/test/utils/Signers.test.ts +++ b/test/utils/Signers.test.ts @@ -3,7 +3,7 @@ import { ethers } from "hardhat"; import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers"; -import { getSignature, Reverter } from "@test-helpers"; +import { getSignature, ProtectedFunction, Reverter } from "@test-helpers"; import { SignersMock } from "@ethers-v6"; @@ -23,7 +23,7 @@ describe("Signers", () => { const Signers = await ethers.getContractFactory("SignersMock"); signers = await Signers.deploy(); - await signers.__SignersMock_init([], "1"); + await signers.__SignersMock_init([], "1", false); await reverter.snapshot(); }); @@ -32,21 +32,21 @@ describe("Signers", () => { describe("#access", () => { it("should not initialize twice", async () => { - await expect(signers.__Signers_init([OWNER.address], "1")).to.be.rejectedWith( + await expect(signers.__Signers_init([OWNER.address], "1", false)).to.be.rejectedWith( "Initializable: contract is not initializing", ); }); it("only owner should call these functions", async () => { - await expect(signers.connect(SECOND).setSignaturesThreshold(1)).to.be.rejectedWith( + await expect(signers.connect(SECOND).setSignaturesThreshold(1, [])).to.be.rejectedWith( "Ownable: caller is not the owner", ); - await expect(signers.connect(SECOND).addSigners([OWNER.address])).to.be.rejectedWith( + await expect(signers.connect(SECOND).addSigners([OWNER.address], [])).to.be.rejectedWith( "Ownable: caller is not the owner", ); - await expect(signers.connect(SECOND).removeSigners([OWNER.address])).to.be.rejectedWith( + await expect(signers.connect(SECOND).removeSigners([OWNER.address], [])).to.be.rejectedWith( "Ownable: caller is not the owner", ); }); @@ -56,33 +56,116 @@ describe("Signers", () => { it("should set signaturesThreshold", async () => { let expectedSignaturesThreshold = 5; - await signers.setSignaturesThreshold(expectedSignaturesThreshold); + await signers.setSignaturesThreshold(expectedSignaturesThreshold, []); expect(await signers.signaturesThreshold()).to.equal(expectedSignaturesThreshold); }); + it("should set signaturesThreshold with signers", async () => { + await signers.addSigners([OWNER.address, SECOND.address], []); + await signers.toggleSignersMode(true, []); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "uint256"], + [ProtectedFunction.SetSignersThreshold, 2], + ); + + const signHash = await signers.getFunctionSignHash( + functionData, + await signers.nonces(functionData), + await signers.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(signers.setSignaturesThreshold(3, [signature])).to.be.rejectedWith("Signers: invalid signer"); + + await expect(signers.setSignaturesThreshold(2, [signature])).to.be.fulfilled; + }); + it("should revert when try to set signaturesThreshold to 0", async () => { let expectedSignaturesThreshold = 0; - expect(signers.setSignaturesThreshold(expectedSignaturesThreshold)).to.be.rejectedWith( + expect(signers.setSignaturesThreshold(expectedSignaturesThreshold, [])).to.be.rejectedWith( "Signers: invalid threshold", ); }); }); + describe("#toggleSignersMode", () => { + it("should toggle signers mode", async () => { + await signers.toggleSignersMode(true, []); + + expect(await signers.isSignersMode()).to.be.true; + }); + + it("should toggle signers mode with signers", async () => { + await signers.addSigners([OWNER.address, SECOND.address], []); + await signers.toggleSignersMode(true, []); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "bool"], + [ProtectedFunction.ToggleSignersMode, false], + ); + + const signHash = await signers.getFunctionSignHash( + functionData, + await signers.nonces(functionData), + await signers.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(signers.toggleSignersMode(true, [signature])).to.be.rejectedWith("Signers: invalid signer"); + + await expect(signers.toggleSignersMode(false, [signature])).to.be.fulfilled; + }); + + it("should revert when try to toggle signers mode by not owner", async () => { + await expect(signers.connect(SECOND).toggleSignersMode(true, [])).to.be.rejectedWith( + "Ownable: caller is not the owner", + ); + }); + }); + describe("#addSigners", () => { it("should add signers", async () => { const expectedSigners = [OWNER.address, SECOND.address, THIRD.address]; - await signers.addSigners(expectedSigners); + await signers.addSigners(expectedSigners, []); expect(await signers.getSigners()).to.be.deep.equal(expectedSigners); }); + it("should add signers with signers", async () => { + await signers.addSigners([OWNER.address, SECOND.address], []); + await signers.toggleSignersMode(true, []); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "address[]"], + [ProtectedFunction.AddSigners, [THIRD.address]], + ); + + const signHash = await signers.getFunctionSignHash( + functionData, + await signers.nonces(functionData), + await signers.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(signers.addSigners([OWNER.address], [signature])).to.be.rejectedWith("Signers: invalid signer"); + + await expect(signers.addSigners([THIRD.address], [signature])).to.be.fulfilled; + }); + it("should revert when try add zero address signer", async () => { let expectedSigners = [OWNER.address, SECOND.address, ethers.ZeroAddress]; - expect(signers.addSigners(expectedSigners)).to.be.rejectedWith("Signers: zero signer"); + expect(signers.addSigners(expectedSigners, [])).to.be.rejectedWith("Signers: zero signer"); }); }); @@ -91,11 +174,36 @@ describe("Signers", () => { let signersToAdd = [OWNER.address, SECOND.address, THIRD.address]; let signersToRemove = [OWNER.address, SECOND.address]; - await signers.addSigners(signersToAdd); - await signers.removeSigners(signersToRemove); + await signers.addSigners(signersToAdd, []); + await signers.removeSigners(signersToRemove, []); expect(await signers.getSigners()).to.be.deep.equal([THIRD.address]); }); + + it("should remove signers with signers", async () => { + await signers.addSigners([OWNER.address, SECOND.address, THIRD.address], []); + await signers.toggleSignersMode(true, []); + + const functionData = ethers.solidityPackedKeccak256( + ["uint8", "address[]"], + [ProtectedFunction.RemoveSigners, [OWNER.address]], + ); + + const signHash = await signers.getFunctionSignHash( + functionData, + await signers.nonces(functionData), + await signers.getAddress(), + (await ethers.provider.getNetwork()).chainId, + ); + + const signature = await getSignature(OWNER, signHash); + + await expect(signers.removeSigners([THIRD.address], [signature])).to.be.rejectedWith("Signers: invalid signer"); + + await signers.removeSigners([OWNER.address], [signature]); + + expect(await signers.getSigners()).to.be.deep.equal([THIRD.address, SECOND.address]); + }); }); describe("checkSignatures", () => { @@ -103,7 +211,7 @@ describe("Signers", () => { beforeEach("setup", async () => { signersToAdd = [OWNER.address, SECOND.address, THIRD.address]; - await signers.addSigners(signersToAdd); + await signers.addSigners(signersToAdd, []); }); async function getSigHash() { @@ -119,7 +227,7 @@ describe("Signers", () => { } it("should check signatures", async () => { - await signers.addSigners([FOURTH.address]); + await signers.addSigners([FOURTH.address], []); const signHash = await getSigHash();