Skip to content

Commit

Permalink
Updated NatSpec documentation. Updated __Bridge_init function
Browse files Browse the repository at this point in the history
  • Loading branch information
KyrylR committed Mar 20, 2024
1 parent 57796cd commit 18fb04e
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 23 deletions.
30 changes: 29 additions & 1 deletion contracts/bridge/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}

/*
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/bridge/IBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
40 changes: 32 additions & 8 deletions contracts/utils/PauseManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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_
Expand All @@ -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_
Expand All @@ -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_,
Expand Down
32 changes: 28 additions & 4 deletions contracts/utils/Signers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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_,
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
15 changes: 8 additions & 7 deletions test/bridge/Bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");

Expand All @@ -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");
Expand Down Expand Up @@ -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",
);
});
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down
5 changes: 3 additions & 2 deletions test/bridge/Upgradeable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe("Upgradeable", () => {
const reverter = new Reverter();

let OWNER: SignerWithAddress;
let PAUSER: SignerWithAddress;
let SECOND: SignerWithAddress;

let bridge: Bridge;
Expand All @@ -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");
Expand All @@ -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();
});
Expand Down

0 comments on commit 18fb04e

Please sign in to comment.