diff --git a/cli/commands/bridge/to-l2.ts b/cli/commands/bridge/to-l2.ts index 307886e30..ffacee1c8 100644 --- a/cli/commands/bridge/to-l2.ts +++ b/cli/commands/bridge/to-l2.ts @@ -163,7 +163,7 @@ export const sendToL2Command = { description: 'Receiving address in L2. Same to L1 address if empty', }) .positional('calldata', { - description: 'Calldata to pass to the recipient. Must be whitelisted in the bridge', + description: 'Calldata to pass to the recipient. Must be allowlisted in the bridge', }) .coerce({ maxGas: toBN, diff --git a/contracts/curation/Curation.sol b/contracts/curation/Curation.sol index fc59b33d1..565a51a89 100644 --- a/contracts/curation/Curation.sol +++ b/contracts/curation/Curation.sol @@ -2,17 +2,20 @@ pragma solidity ^0.7.6; -import "@openzeppelin/contracts/utils/Address.sol"; -import "@openzeppelin/contracts/math/SafeMath.sol"; -import "@openzeppelin/contracts/proxy/Clones.sol"; - -import "../bancor/BancorFormula.sol"; -import "../upgrades/GraphUpgradeable.sol"; -import "../utils/TokenUtils.sol"; - -import "./CurationStorage.sol"; -import "./ICuration.sol"; -import "./GraphCurationToken.sol"; +import { Address } from "@openzeppelin/contracts/utils/Address.sol"; +import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol"; +import { Clones } from "@openzeppelin/contracts/proxy/Clones.sol"; + +import { BancorFormula } from "../bancor/BancorFormula.sol"; +import { GraphUpgradeable } from "../upgrades/GraphUpgradeable.sol"; +import { TokenUtils } from "../utils/TokenUtils.sol"; +import { IRewardsManager } from "../rewards/IRewardsManager.sol"; +import { Managed } from "../governance/Managed.sol"; +import { IGraphToken } from "../token/IGraphToken.sol"; +import { CurationV1Storage } from "./CurationStorage.sol"; +import { ICuration } from "./ICuration.sol"; +import { IGraphCurationToken } from "./IGraphCurationToken.sol"; +import { GraphCurationToken } from "./GraphCurationToken.sol"; /** * @title Curation contract diff --git a/contracts/curation/CurationStorage.sol b/contracts/curation/CurationStorage.sol index dd2edd18b..a530d9199 100644 --- a/contracts/curation/CurationStorage.sol +++ b/contracts/curation/CurationStorage.sol @@ -2,7 +2,9 @@ pragma solidity ^0.7.6; -import "../governance/Managed.sol"; +import { ICuration } from "./ICuration.sol"; +import { IGraphCurationToken } from "./IGraphCurationToken.sol"; +import { Managed } from "../governance/Managed.sol"; abstract contract CurationV1Storage is Managed, ICuration { // -- Pool -- diff --git a/contracts/gateway/BridgeEscrow.sol b/contracts/gateway/BridgeEscrow.sol index 4d9c11e0d..3c0fa5c1a 100644 --- a/contracts/gateway/BridgeEscrow.sol +++ b/contracts/gateway/BridgeEscrow.sol @@ -4,9 +4,9 @@ pragma solidity ^0.7.6; import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol"; -import "../upgrades/GraphUpgradeable.sol"; -import "../governance/Managed.sol"; -import "../token/IGraphToken.sol"; +import { GraphUpgradeable } from "../upgrades/GraphUpgradeable.sol"; +import { Managed } from "../governance/Managed.sol"; +import { IGraphToken } from "../token/IGraphToken.sol"; /** * @title Bridge Escrow @@ -16,7 +16,7 @@ import "../token/IGraphToken.sol"; */ contract BridgeEscrow is Initializable, GraphUpgradeable, Managed { /** - * @dev Initialize this contract. + * @notice Initialize the BridgeEscrow contract. * @param _controller Address of the Controller that manages this contract */ function initialize(address _controller) external onlyImpl initializer { @@ -24,7 +24,7 @@ contract BridgeEscrow is Initializable, GraphUpgradeable, Managed { } /** - * @dev Approve a spender (i.e. a bridge that manages the GRT funds held by the escrow) + * @notice Approve a spender (i.e. a bridge that manages the GRT funds held by the escrow) * @param _spender Address of the spender that will be approved */ function approveAll(address _spender) external onlyGovernor { @@ -32,7 +32,7 @@ contract BridgeEscrow is Initializable, GraphUpgradeable, Managed { } /** - * @dev Revoke a spender (i.e. a bridge that will no longer manage the GRT funds held by the escrow) + * @notice Revoke a spender (i.e. a bridge that will no longer manage the GRT funds held by the escrow) * @param _spender Address of the spender that will be revoked */ function revokeAll(address _spender) external onlyGovernor { diff --git a/contracts/gateway/GraphTokenGateway.sol b/contracts/gateway/GraphTokenGateway.sol index ed59b149a..ca2ad4c95 100644 --- a/contracts/gateway/GraphTokenGateway.sol +++ b/contracts/gateway/GraphTokenGateway.sol @@ -2,17 +2,17 @@ pragma solidity ^0.7.6; -import "../upgrades/GraphUpgradeable.sol"; -import "../arbitrum/ITokenGateway.sol"; -import "../governance/Pausable.sol"; -import "../governance/Managed.sol"; +import { GraphUpgradeable } from "../upgrades/GraphUpgradeable.sol"; +import { ITokenGateway } from "../arbitrum/ITokenGateway.sol"; +import { Pausable } from "../governance/Pausable.sol"; +import { Managed } from "../governance/Managed.sol"; /** * @title L1/L2 Graph Token Gateway * @dev This includes everything that's shared between the L1 and L2 sides of the bridge. */ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITokenGateway { - // Storage gap added in case we need to add state variables to this contract + /// @dev Storage gap added in case we need to add state variables to this contract uint256[50] private __gap; /** @@ -35,14 +35,6 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok _setPauseGuardian(_newPauseGuardian); } - /** - * @dev Override the default pausing from Managed to allow pausing this - * particular contract instead of pausing from the Controller. - */ - function _notPaused() internal view override { - require(!_paused, "Paused (contract)"); - } - /** * @notice Change the paused state of the contract * @param _newPaused New value for the pause state (true means the transfers will be paused) @@ -56,11 +48,20 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok /** * @notice Getter to access paused state of this contract + * @return True if the contract is paused, false otherwise */ function paused() external view returns (bool) { return _paused; } + /** + * @dev Override the default pausing from Managed to allow pausing this + * particular contract instead of pausing from the Controller. + */ + function _notPaused() internal view override { + require(!_paused, "Paused (contract)"); + } + /** * @dev Runs state validation before unpausing, reverts if * something is not set properly diff --git a/contracts/gateway/ICallhookReceiver.sol b/contracts/gateway/ICallhookReceiver.sol index fb7492bb7..ff0fbfab1 100644 --- a/contracts/gateway/ICallhookReceiver.sol +++ b/contracts/gateway/ICallhookReceiver.sol @@ -3,14 +3,14 @@ /** * @title Interface for contracts that can receive callhooks through the Arbitrum GRT bridge * @dev Any contract that can receive a callhook on L2, sent through the bridge from L1, must - * be whitelisted by the governor, but also implement this interface that contains + * be allowlisted by the governor, but also implement this interface that contains * the function that will actually be called by the L2GraphTokenGateway. */ pragma solidity ^0.7.6; interface ICallhookReceiver { /** - * @dev Receive tokens with a callhook from the bridge + * @notice Receive tokens with a callhook from the bridge * @param _from Token sender in L1 * @param _amount Amount of tokens that were transferred * @param _data ABI-encoded callhook data diff --git a/contracts/gateway/L1GraphTokenGateway.sol b/contracts/gateway/L1GraphTokenGateway.sol index 7688a5e29..06c4b7ea7 100644 --- a/contracts/gateway/L1GraphTokenGateway.sol +++ b/contracts/gateway/L1GraphTokenGateway.sol @@ -4,11 +4,17 @@ pragma solidity ^0.7.6; pragma abicoder v2; import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/Initializable.sol"; -import "@openzeppelin/contracts/utils/Address.sol"; -import "@openzeppelin/contracts/math/SafeMath.sol"; +import { AddressUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol"; +import { SafeMathUpgradeable } from "@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol"; -import "../arbitrum/L1ArbitrumMessenger.sol"; -import "./GraphTokenGateway.sol"; +import { L1ArbitrumMessenger } from "../arbitrum/L1ArbitrumMessenger.sol"; +import { IBridge } from "../arbitrum/IBridge.sol"; +import { IInbox } from "../arbitrum/IInbox.sol"; +import { IOutbox } from "../arbitrum/IOutbox.sol"; +import { ITokenGateway } from "../arbitrum/ITokenGateway.sol"; +import { Managed } from "../governance/Managed.sol"; +import { GraphTokenGateway } from "./GraphTokenGateway.sol"; +import { IGraphToken } from "../token/IGraphToken.sol"; /** * @title L1 Graph Token Gateway Contract @@ -20,22 +26,22 @@ import "./GraphTokenGateway.sol"; * and https://github.com/livepeer/arbitrum-lpt-bridge) */ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMessenger { - using SafeMath for uint256; + using SafeMathUpgradeable for uint256; - // Address of the Graph Token contract on L2 + /// Address of the Graph Token contract on L2 address public l2GRT; - // Address of the Arbitrum Inbox + /// Address of the Arbitrum Inbox address public inbox; - // Address of the Arbitrum Gateway Router on L1 + /// Address of the Arbitrum Gateway Router on L1 address public l1Router; - // Address of the L2GraphTokenGateway on L2 that is the counterpart of this gateway + /// Address of the L2GraphTokenGateway on L2 that is the counterpart of this gateway address public l2Counterpart; - // Address of the BridgeEscrow contract that holds the GRT in the bridge + /// Address of the BridgeEscrow contract that holds the GRT in the bridge address public escrow; - // Addresses for which this mapping is true are allowed to send callhooks in outbound transfers - mapping(address => bool) public callhookWhitelist; + /// Addresses for which this mapping is true are allowed to send callhooks in outbound transfers + mapping(address => bool) public callhookAllowlist; - // Emitted when an outbound transfer is initiated, i.e. tokens are deposited from L1 to L2 + /// Emitted when an outbound transfer is initiated, i.e. tokens are deposited from L1 to L2 event DepositInitiated( address l1Token, address indexed from, @@ -44,7 +50,7 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess uint256 amount ); - // Emitted when an incoming transfer is finalized, i.e tokens are withdrawn from L2 to L1 + /// Emitted when an incoming transfer is finalized, i.e tokens are withdrawn from L2 to L1 event WithdrawalFinalized( address l1Token, address indexed from, @@ -53,18 +59,18 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess uint256 amount ); - // Emitted when the Arbitrum Inbox and Gateway Router addresses have been updated + /// Emitted when the Arbitrum Inbox and Gateway Router addresses have been updated event ArbitrumAddressesSet(address inbox, address l1Router); - // Emitted when the L2 GRT address has been updated + /// Emitted when the L2 GRT address has been updated event L2TokenAddressSet(address l2GRT); - // Emitted when the counterpart L2GraphTokenGateway address has been updated + /// Emitted when the counterpart L2GraphTokenGateway address has been updated event L2CounterpartAddressSet(address l2Counterpart); - // Emitted when the escrow address has been updated + /// Emitted when the escrow address has been updated event EscrowAddressSet(address escrow); - // Emitted when an address is added to the callhook whitelist - event AddedToCallhookWhitelist(address newWhitelisted); - // Emitted when an address is removed from the callhook whitelist - event RemovedFromCallhookWhitelist(address notWhitelisted); + /// Emitted when an address is added to the callhook allowlist + event AddedToCallhookAllowlist(address newAllowlisted); + /// Emitted when an address is removed from the callhook allowlist + event RemovedFromCallhookAllowlist(address notAllowlisted); /** * @dev Allows a function to be called only by the gateway's L2 counterpart. @@ -86,15 +92,15 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess } /** - * @dev Initialize this contract. - * The contract will be paused. + * @notice Initialize the L1GraphTokenGateway contract. + * @dev The contract will be paused. * Note some parameters have to be set separately as they are generally * not expected to be available at initialization time: * - inbox and l1Router using setArbitrumAddresses * - l2GRT using setL2TokenAddress * - l2Counterpart using setL2CounterpartAddress * - escrow using setEscrowAddress - * - whitelisted callhook callers using addToCallhookWhitelist + * - allowlisted callhook callers using addToCallhookAllowlist * - pauseGuardian using setPauseGuardian * @param _controller Address of the Controller that manages this contract */ @@ -104,22 +110,23 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess } /** - * @dev Sets the addresses for L1 contracts provided by Arbitrum + * @notice Sets the addresses for L1 contracts provided by Arbitrum * @param _inbox Address of the Inbox that is part of the Arbitrum Bridge * @param _l1Router Address of the Gateway Router */ function setArbitrumAddresses(address _inbox, address _l1Router) external onlyGovernor { require(_inbox != address(0), "INVALID_INBOX"); require(_l1Router != address(0), "INVALID_L1_ROUTER"); - require(Address.isContract(_inbox), "INBOX_MUST_BE_CONTRACT"); - require(Address.isContract(_l1Router), "ROUTER_MUST_BE_CONTRACT"); + require(!callhookAllowlist[_l1Router], "ROUTER_CANT_BE_ALLOWLISTED"); + require(AddressUpgradeable.isContract(_inbox), "INBOX_MUST_BE_CONTRACT"); + require(AddressUpgradeable.isContract(_l1Router), "ROUTER_MUST_BE_CONTRACT"); inbox = _inbox; l1Router = _l1Router; emit ArbitrumAddressesSet(_inbox, _l1Router); } /** - * @dev Sets the address of the L2 Graph Token + * @notice Sets the address of the L2 Graph Token * @param _l2GRT Address of the GRT contract on L2 */ function setL2TokenAddress(address _l2GRT) external onlyGovernor { @@ -129,7 +136,7 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess } /** - * @dev Sets the address of the counterpart gateway on L2 + * @notice Sets the address of the counterpart gateway on L2 * @param _l2Counterpart Address of the corresponding L2GraphTokenGateway on Arbitrum */ function setL2CounterpartAddress(address _l2Counterpart) external onlyGovernor { @@ -139,39 +146,40 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess } /** - * @dev Sets the address of the escrow contract on L1 + * @notice Sets the address of the escrow contract on L1 * @param _escrow Address of the BridgeEscrow */ function setEscrowAddress(address _escrow) external onlyGovernor { require(_escrow != address(0), "INVALID_ESCROW"); - require(Address.isContract(_escrow), "MUST_BE_CONTRACT"); + require(AddressUpgradeable.isContract(_escrow), "MUST_BE_CONTRACT"); escrow = _escrow; emit EscrowAddressSet(_escrow); } /** - * @dev Adds an address to the callhook whitelist. + * @notice Adds an address to the callhook allowlist. * This address will be allowed to include callhooks when transferring tokens. - * @param _newWhitelisted Address to add to the whitelist + * @param _newAllowlisted Address to add to the allowlist */ - function addToCallhookWhitelist(address _newWhitelisted) external onlyGovernor { - require(_newWhitelisted != address(0), "INVALID_ADDRESS"); - require(Address.isContract(_newWhitelisted), "MUST_BE_CONTRACT"); - require(!callhookWhitelist[_newWhitelisted], "ALREADY_WHITELISTED"); - callhookWhitelist[_newWhitelisted] = true; - emit AddedToCallhookWhitelist(_newWhitelisted); + function addToCallhookAllowlist(address _newAllowlisted) external onlyGovernor { + require(_newAllowlisted != address(0), "INVALID_ADDRESS"); + require(_newAllowlisted != l1Router, "CANT_ALLOW_ROUTER"); + require(AddressUpgradeable.isContract(_newAllowlisted), "MUST_BE_CONTRACT"); + require(!callhookAllowlist[_newAllowlisted], "ALREADY_ALLOWLISTED"); + callhookAllowlist[_newAllowlisted] = true; + emit AddedToCallhookAllowlist(_newAllowlisted); } /** - * @dev Removes an address from the callhook whitelist. + * @notice Removes an address from the callhook allowlist. * This address will no longer be allowed to include callhooks when transferring tokens. - * @param _notWhitelisted Address to remove from the whitelist + * @param _notAllowlisted Address to remove from the allowlist */ - function removeFromCallhookWhitelist(address _notWhitelisted) external onlyGovernor { - require(_notWhitelisted != address(0), "INVALID_ADDRESS"); - require(callhookWhitelist[_notWhitelisted], "NOT_WHITELISTED"); - callhookWhitelist[_notWhitelisted] = false; - emit RemovedFromCallhookWhitelist(_notWhitelisted); + function removeFromCallhookAllowlist(address _notAllowlisted) external onlyGovernor { + require(_notAllowlisted != address(0), "INVALID_ADDRESS"); + require(callhookAllowlist[_notAllowlisted], "NOT_ALLOWLISTED"); + callhookAllowlist[_notAllowlisted] = false; + emit RemovedFromCallhookAllowlist(_notAllowlisted); } /** @@ -180,15 +188,15 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess * The ticket must be redeemed on L2 to receive tokens at the specified address. * Note that the caller must previously allow the gateway to spend the specified amount of GRT. * @dev maxGas and gasPriceBid must be set using Arbitrum's NodeInterface.estimateRetryableTicket method. - * Also note that whitelisted senders (some protocol contracts) can include additional calldata + * Also note that allowlisted senders (some protocol contracts) can include additional calldata * for a callhook to be executed on the L2 side when the tokens are received. In this case, the L2 transaction * can revert if the callhook reverts, potentially locking the tokens on the bridge if the callhook - * never succeeds. This requires extra care when adding contracts to the whitelist, but is necessary to ensure that + * never succeeds. This requires extra care when adding contracts to the allowlist, but is necessary to ensure that * the tickets can be retried in the case of a temporary failure, and to ensure the atomicity of callhooks * with token transfers. * @param _l1Token L1 Address of the GRT contract (needed for compatibility with Arbitrum Gateway Router) * @param _to Recipient address on L2 - * @param _amount Amount of tokens to tranfer + * @param _amount Amount of tokens to transfer * @param _maxGas Gas limit for L2 execution of the ticket * @param _gasPriceBid Price per gas on L2 * @param _data Encoded maxSubmissionCost and sender address along with additional calldata @@ -215,9 +223,9 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess bytes memory outboundCalldata; { bytes memory extraData; - (from, maxSubmissionCost, extraData) = parseOutboundData(_data); + (from, maxSubmissionCost, extraData) = _parseOutboundData(_data); require( - extraData.length == 0 || callhookWhitelist[msg.sender] == true, + extraData.length == 0 || callhookAllowlist[msg.sender] == true, "CALL_HOOK_DATA_NOT_ALLOWED" ); require(maxSubmissionCost != 0, "NO_SUBMISSION_COST"); @@ -255,7 +263,7 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess * and the encoded exitNum is assumed to be 0. * @param _l1Token L1 Address of the GRT contract (needed for compatibility with Arbitrum Gateway Router) * @param _from Address of the sender - * @param _to Recepient address on L1 + * @param _to Recipient address on L1 * @param _amount Amount of tokens transferred */ function finalizeInboundTransfer( @@ -277,37 +285,17 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess } /** - * @notice Decodes calldata required for migration of tokens - * @dev Data must include maxSubmissionCost, extraData can be left empty. When the router - * sends an outbound message, data also contains the from address. - * @param _data encoded callhook data - * @return Sender of the tx - * @return Base ether value required to keep retryable ticket alive - * @return Additional data sent to L2 + * @notice Calculate the L2 address of a bridged token + * @dev In our case, this would only work for GRT. + * @param _l1ERC20 address of L1 GRT contract + * @return L2 address of the bridged GRT token */ - function parseOutboundData(bytes calldata _data) - private - view - returns ( - address, - uint256, - bytes memory - ) - { - address from; - uint256 maxSubmissionCost; - bytes memory extraData; - if (msg.sender == l1Router) { - // Data encoded by the Gateway Router includes the sender address - (from, extraData) = abi.decode(_data, (address, bytes)); - } else { - from = msg.sender; - extraData = _data; + function calculateL2TokenAddress(address _l1ERC20) external view override returns (address) { + IGraphToken token = graphToken(); + if (_l1ERC20 != address(token)) { + return address(0); } - // User-encoded data contains the max retryable ticket submission cost - // and additional L2 calldata - (maxSubmissionCost, extraData) = abi.decode(extraData, (uint256, bytes)); - return (from, maxSubmissionCost, extraData); + return l2GRT; } /** @@ -318,7 +306,7 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess * @param _from Address on L1 from which we're transferring tokens * @param _to Address on L2 to which we're transferring tokens * @param _amount Amount of GRT to transfer - * @param _data Additional call data for the L2 transaction, which must be empty unless the caller is whitelisted + * @param _data Additional call data for the L2 transaction, which must be empty unless the caller is allowlisted * @return Encoded calldata (including function selector) for the L2 transaction */ function getOutboundCalldata( @@ -341,20 +329,6 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess ); } - /** - * @notice Calculate the L2 address of a bridged token - * @dev In our case, this would only work for GRT. - * @param _l1ERC20 address of L1 GRT contract - * @return L2 address of the bridged GRT token - */ - function calculateL2TokenAddress(address _l1ERC20) external view override returns (address) { - IGraphToken token = graphToken(); - if (_l1ERC20 != address(token)) { - return address(0); - } - return l2GRT; - } - /** * @dev Runs state validation before unpausing, reverts if * something is not set properly @@ -365,4 +339,38 @@ contract L1GraphTokenGateway is Initializable, GraphTokenGateway, L1ArbitrumMess require(l2Counterpart != address(0), "L2_COUNTERPART_NOT_SET"); require(escrow != address(0), "ESCROW_NOT_SET"); } + + /** + * @notice Decodes calldata required for migration of tokens + * @dev Data must include maxSubmissionCost, extraData can be left empty. When the router + * sends an outbound message, data also contains the from address. + * @param _data encoded callhook data + * @return Sender of the tx + * @return Base ether value required to keep retryable ticket alive + * @return Additional data sent to L2 + */ + function _parseOutboundData(bytes calldata _data) + private + view + returns ( + address, + uint256, + bytes memory + ) + { + address from; + uint256 maxSubmissionCost; + bytes memory extraData; + if (msg.sender == l1Router) { + // Data encoded by the Gateway Router includes the sender address + (from, extraData) = abi.decode(_data, (address, bytes)); + } else { + from = msg.sender; + extraData = _data; + } + // User-encoded data contains the max retryable ticket submission cost + // and additional L2 calldata + (maxSubmissionCost, extraData) = abi.decode(extraData, (uint256, bytes)); + return (from, maxSubmissionCost, extraData); + } } diff --git a/contracts/governance/Controller.sol b/contracts/governance/Controller.sol index 2e4418a59..bc287d2be 100644 --- a/contracts/governance/Controller.sol +++ b/contracts/governance/Controller.sol @@ -2,10 +2,10 @@ pragma solidity ^0.7.6; -import "./IController.sol"; -import "./IManaged.sol"; -import "./Governed.sol"; -import "./Pausable.sol"; +import { IController } from "./IController.sol"; +import { IManaged } from "./IManaged.sol"; +import { Governed } from "./Governed.sol"; +import { Pausable } from "./Pausable.sol"; /** * @title Graph Controller contract @@ -13,13 +13,14 @@ import "./Pausable.sol"; * https://github.com/livepeer/protocol/blob/streamflow/contracts/Controller.sol */ contract Controller is Governed, Pausable, IController { - // Track contract ids to contract proxy address - mapping(bytes32 => address) private registry; + /// @dev Track contract ids to contract proxy address + mapping(bytes32 => address) private _registry; + /// Emitted when the proxy address for a protocol contract has been set event SetContractProxy(bytes32 indexed id, address contractAddress); /** - * @dev Contract constructor. + * @notice Controller contract constructor. */ constructor() { Governed._initialize(msg.sender); @@ -58,7 +59,7 @@ contract Controller is Governed, Pausable, IController { onlyGovernor { require(_contractAddress != address(0), "Contract address must be set"); - registry[_id] = _contractAddress; + _registry[_id] = _contractAddress; emit SetContractProxy(_id, _contractAddress); } @@ -67,16 +68,17 @@ contract Controller is Governed, Pausable, IController { * @param _id Contract id (keccak256 hash of contract name) */ function unsetContractProxy(bytes32 _id) external override onlyGovernor { - registry[_id] = address(0); + _registry[_id] = address(0); emit SetContractProxy(_id, address(0)); } /** * @notice Get contract proxy address by its id * @param _id Contract id + * @return Address of the proxy contract for the provided id */ - function getContractProxy(bytes32 _id) public view override returns (address) { - return registry[_id]; + function getContractProxy(bytes32 _id) external view override returns (address) { + return _registry[_id]; } /** @@ -86,7 +88,7 @@ contract Controller is Governed, Pausable, IController { */ function updateController(bytes32 _id, address _controller) external override onlyGovernor { require(_controller != address(0), "Controller must be set"); - return IManaged(registry[_id]).setController(_controller); + return IManaged(_registry[_id]).setController(_controller); } // -- Pausing -- @@ -94,17 +96,19 @@ contract Controller is Governed, Pausable, IController { /** * @notice Change the partial paused state of the contract * Partial pause is intended as a partial pause of the protocol + * @param _toPause True if the contracts should be (partially) paused, false otherwise */ - function setPartialPaused(bool _partialPaused) external override onlyGovernorOrGuardian { - _setPartialPaused(_partialPaused); + function setPartialPaused(bool _toPause) external override onlyGovernorOrGuardian { + _setPartialPaused(_toPause); } /** * @notice Change the paused state of the contract * Full pause most of protocol functions + * @param _toPause True if the contracts should be paused, false otherwise */ - function setPaused(bool _paused) external override onlyGovernorOrGuardian { - _setPaused(_paused); + function setPaused(bool _toPause) external override onlyGovernorOrGuardian { + _setPaused(_toPause); } /** @@ -118,6 +122,7 @@ contract Controller is Governed, Pausable, IController { /** * @notice Getter to access paused + * @return True if the contracts are paused, false otherwise */ function paused() external view override returns (bool) { return _paused; @@ -125,6 +130,7 @@ contract Controller is Governed, Pausable, IController { /** * @notice Getter to access partial pause status + * @return True if the contracts are partially paused, false otherwise */ function partialPaused() external view override returns (bool) { return _partialPaused; diff --git a/contracts/governance/Governed.sol b/contracts/governance/Governed.sol index 697f2fa12..f692b2d19 100644 --- a/contracts/governance/Governed.sol +++ b/contracts/governance/Governed.sol @@ -6,15 +6,19 @@ pragma solidity ^0.7.6; * @title Graph Governance contract * @dev All contracts that will be owned by a Governor entity should extend this contract. */ -contract Governed { +abstract contract Governed { // -- State -- + /// Address of the governor address public governor; + /// Address of the new governor that is pending acceptance address public pendingGovernor; // -- Events -- + /// Emitted when a new owner/governor has been set, but is pending acceptance event NewPendingOwnership(address indexed from, address indexed to); + /// Emitted when a new owner/governor has accepted their role event NewOwnership(address indexed from, address indexed to); /** @@ -26,14 +30,15 @@ contract Governed { } /** - * @dev Initialize the governor to the contract caller. + * @dev Initialize the governor for this contract + * @param _initGovernor Address of the governor */ function _initialize(address _initGovernor) internal { governor = _initGovernor; } /** - * @dev Admin function to begin change of governor. The `_newGovernor` must call + * @notice Admin function to begin change of governor. The `_newGovernor` must call * `acceptOwnership` to finalize the transfer. * @param _newGovernor Address of new `governor` */ @@ -47,7 +52,7 @@ contract Governed { } /** - * @dev Admin function for pending governor to accept role and update governor. + * @notice Admin function for pending governor to accept role and update governor. * This function must called by the pending governor. */ function acceptOwnership() external { diff --git a/contracts/governance/Managed.sol b/contracts/governance/Managed.sol index 562028c15..6b8fe624e 100644 --- a/contracts/governance/Managed.sol +++ b/contracts/governance/Managed.sol @@ -2,14 +2,16 @@ pragma solidity ^0.7.6; -import "./IController.sol"; +import { IController } from "./IController.sol"; -import "../curation/ICuration.sol"; -import "../epochs/IEpochManager.sol"; -import "../rewards/IRewardsManager.sol"; -import "../staking/IStaking.sol"; -import "../token/IGraphToken.sol"; -import "../arbitrum/ITokenGateway.sol"; +import { ICuration } from "../curation/ICuration.sol"; +import { IEpochManager } from "../epochs/IEpochManager.sol"; +import { IRewardsManager } from "../rewards/IRewardsManager.sol"; +import { IStaking } from "../staking/IStaking.sol"; +import { IGraphToken } from "../token/IGraphToken.sol"; +import { ITokenGateway } from "../arbitrum/ITokenGateway.sol"; + +import { IManaged } from "./IManaged.sol"; /** * @title Graph Managed contract @@ -20,12 +22,14 @@ import "../arbitrum/ITokenGateway.sol"; * Inspired by Livepeer: * https://github.com/livepeer/protocol/blob/streamflow/contracts/Controller.sol */ -contract Managed { +abstract contract Managed is IManaged { // -- State -- - // Controller that contract is registered with + /// Controller that contract is registered with IController public controller; - mapping(bytes32 => address) private addressCache; + /// @dev Cache for the addresses of the contracts retrieved from the controller + mapping(bytes32 => address) private _addressCache; + /// @dev Gap for future storage variables uint256[10] private __gap; // Immutables @@ -38,50 +42,72 @@ contract Managed { // -- Events -- + /// Emitted when a contract parameter has been updated event ParameterUpdated(string param); + /// Emitted when the controller address has been set event SetController(address controller); - /** - * @dev Emitted when contract with `nameHash` is synced to `contractAddress`. - */ + /// Emitted when contract with `nameHash` is synced to `contractAddress`. event ContractSynced(bytes32 indexed nameHash, address contractAddress); // -- Modifiers -- + /** + * @dev Revert if the controller is paused or partially paused + */ function _notPartialPaused() internal view { require(!controller.paused(), "Paused"); require(!controller.partialPaused(), "Partial-paused"); } + /** + * @dev Revert if the controller is paused + */ function _notPaused() internal view virtual { require(!controller.paused(), "Paused"); } + /** + * @dev Revert if the caller is not the governor + */ function _onlyGovernor() internal view { require(msg.sender == controller.getGovernor(), "Only Controller governor"); } + /** + * @dev Revert if the caller is not the Controller + */ function _onlyController() internal view { require(msg.sender == address(controller), "Caller must be Controller"); } + /** + * @dev Revert if the controller is paused or partially paused + */ modifier notPartialPaused() { _notPartialPaused(); _; } + /** + * @dev Revert if the controller is paused + */ modifier notPaused() { _notPaused(); _; } - // Check if sender is controller. + /** + * @dev Revert if the caller is not the Controller + */ modifier onlyController() { _onlyController(); _; } - // Check if sender is the governor. + /** + * @dev Revert if the caller is not the governor + */ modifier onlyGovernor() { _onlyGovernor(); _; @@ -90,7 +116,8 @@ contract Managed { // -- Functions -- /** - * @dev Initialize the controller. + * @dev Initialize a Managed contract + * @param _controller Address for the Controller that manages this contract */ function _initialize(address _controller) internal { _setController(_controller); @@ -100,7 +127,7 @@ contract Managed { * @notice Set Controller. Only callable by current controller. * @param _controller Controller contract address */ - function setController(address _controller) external onlyController { + function setController(address _controller) external override onlyController { _setController(_controller); } @@ -115,7 +142,7 @@ contract Managed { } /** - * @dev Return Curation interface. + * @dev Return Curation interface * @return Curation contract registered with Controller */ function curation() internal view returns (ICuration) { @@ -123,7 +150,7 @@ contract Managed { } /** - * @dev Return EpochManager interface. + * @dev Return EpochManager interface * @return Epoch manager contract registered with Controller */ function epochManager() internal view returns (IEpochManager) { @@ -131,7 +158,7 @@ contract Managed { } /** - * @dev Return RewardsManager interface. + * @dev Return RewardsManager interface * @return Rewards manager contract registered with Controller */ function rewardsManager() internal view returns (IRewardsManager) { @@ -139,7 +166,7 @@ contract Managed { } /** - * @dev Return Staking interface. + * @dev Return Staking interface * @return Staking contract registered with Controller */ function staking() internal view returns (IStaking) { @@ -147,7 +174,7 @@ contract Managed { } /** - * @dev Return GraphToken interface. + * @dev Return GraphToken interface * @return Graph token contract registered with Controller */ function graphToken() internal view returns (IGraphToken) { @@ -155,7 +182,7 @@ contract Managed { } /** - * @dev Return GraphTokenGateway (L1 or L2) interface. + * @dev Return GraphTokenGateway (L1 or L2) interface * @return Graph token gateway contract registered with Controller */ function graphTokenGateway() internal view returns (ITokenGateway) { @@ -163,11 +190,12 @@ contract Managed { } /** - * @dev Resolve a contract address from the cache or the Controller if not found. + * @dev Resolve a contract address from the cache or the Controller if not found + * @param _nameHash keccak256 hash of the contract name * @return Address of the contract */ function _resolveContract(bytes32 _nameHash) internal view returns (address) { - address contractAddress = addressCache[_nameHash]; + address contractAddress = _addressCache[_nameHash]; if (contractAddress == address(0)) { contractAddress = controller.getContractProxy(_nameHash); } @@ -181,15 +209,15 @@ contract Managed { function _syncContract(string memory _name) internal { bytes32 nameHash = keccak256(abi.encodePacked(_name)); address contractAddress = controller.getContractProxy(nameHash); - if (addressCache[nameHash] != contractAddress) { - addressCache[nameHash] = contractAddress; + if (_addressCache[nameHash] != contractAddress) { + _addressCache[nameHash] = contractAddress; emit ContractSynced(nameHash, contractAddress); } } /** - * @dev Sync protocol contract addresses from the Controller registry. - * This function will cache all the contracts using the latest addresses + * @notice Sync protocol contract addresses from the Controller registry + * @dev This function will cache all the contracts using the latest addresses * Anyone can call the function whenever a Proxy contract change in the * controller to ensure the protocol is using the latest version */ diff --git a/contracts/governance/Pausable.sol b/contracts/governance/Pausable.sol index b1ecbdf30..552b0aa15 100644 --- a/contracts/governance/Pausable.sol +++ b/contracts/governance/Pausable.sol @@ -2,26 +2,36 @@ pragma solidity ^0.7.6; -contract Pausable { - // Partial paused paused exit and enter functions for GRT, but not internal - // functions, such as allocating +abstract contract Pausable { + /** + * @dev "Partial paused" pauses exit and enter functions for GRT, but not internal + * functions, such as allocating + */ bool internal _partialPaused; - // Paused will pause all major protocol functions + /** + * @dev Paused will pause all major protocol functions + */ bool internal _paused; - // Time last paused for both pauses + /// Timestamp for the last time the partial pause was set uint256 public lastPausePartialTime; + /// Timestamp for the last time the full pause was set uint256 public lastPauseTime; - // Pause guardian is a separate entity from the governor that can pause + /// Pause guardian is a separate entity from the governor that can + /// pause and unpause the protocol, fully or partially address public pauseGuardian; + /// Emitted when the partial pause state changed event PartialPauseChanged(bool isPaused); + /// Emitted when the full pause state changed event PauseChanged(bool isPaused); + /// Emitted when the pause guardian is changed event NewPauseGuardian(address indexed oldPauseGuardian, address indexed pauseGuardian); /** - * @notice Change the partial paused state of the contract + * @dev Change the partial paused state of the contract + * @param _toPause New value for the partial pause state (true means the contracts will be partially paused) */ function _setPartialPaused(bool _toPause) internal { if (_toPause == _partialPaused) { @@ -35,7 +45,8 @@ contract Pausable { } /** - * @notice Change the paused state of the contract + * @dev Change the paused state of the contract + * @param _toPause New value for the pause state (true means the contracts will be paused) */ function _setPaused(bool _toPause) internal { if (_toPause == _paused) { @@ -49,7 +60,7 @@ contract Pausable { } /** - * @notice Change the Pause Guardian + * @dev Change the Pause Guardian * @param newPauseGuardian The address of the new Pause Guardian */ function _setPauseGuardian(address newPauseGuardian) internal { diff --git a/contracts/l2/gateway/L2GraphTokenGateway.sol b/contracts/l2/gateway/L2GraphTokenGateway.sol index 371db711f..ca9c62bca 100644 --- a/contracts/l2/gateway/L2GraphTokenGateway.sol +++ b/contracts/l2/gateway/L2GraphTokenGateway.sol @@ -3,14 +3,16 @@ pragma solidity ^0.7.6; pragma abicoder v2; -import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; -import "@openzeppelin/contracts/math/SafeMath.sol"; +import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; +import { SafeMathUpgradeable } from "@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol"; -import "../../arbitrum/L2ArbitrumMessenger.sol"; -import "../../arbitrum/AddressAliasHelper.sol"; -import "../../gateway/GraphTokenGateway.sol"; -import "../../gateway/ICallhookReceiver.sol"; -import "../token/L2GraphToken.sol"; +import { L2ArbitrumMessenger } from "../../arbitrum/L2ArbitrumMessenger.sol"; +import { AddressAliasHelper } from "../../arbitrum/AddressAliasHelper.sol"; +import { ITokenGateway } from "../../arbitrum/ITokenGateway.sol"; +import { Managed } from "../../governance/Managed.sol"; +import { GraphTokenGateway } from "../../gateway/GraphTokenGateway.sol"; +import { ICallhookReceiver } from "../../gateway/ICallhookReceiver.sol"; +import { L2GraphToken } from "../token/L2GraphToken.sol"; /** * @title L2 Graph Token Gateway Contract @@ -21,22 +23,22 @@ import "../token/L2GraphToken.sol"; * and https://github.com/livepeer/arbitrum-lpt-bridge) */ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, ReentrancyGuardUpgradeable { - using SafeMath for uint256; + using SafeMathUpgradeable for uint256; - // Address of the Graph Token contract on L1 + /// Address of the Graph Token contract on L1 address public l1GRT; - // Address of the L1GraphTokenGateway that is the counterpart of this gateway on L1 + /// Address of the L1GraphTokenGateway that is the counterpart of this gateway on L1 address public l1Counterpart; - // Address of the Arbitrum Gateway Router on L2 + /// Address of the Arbitrum Gateway Router on L2 address public l2Router; - // Calldata included in an outbound transfer, stored as a structure for convenience and stack depth + /// @dev Calldata included in an outbound transfer, stored as a structure for convenience and stack depth struct OutboundCalldata { address from; bytes extraData; } - // Emitted when an incoming transfer is finalized, i.e. tokens were deposited from L1 to L2 + /// Emitted when an incoming transfer is finalized, i.e. tokens were deposited from L1 to L2 event DepositFinalized( address indexed l1Token, address indexed from, @@ -44,7 +46,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran uint256 amount ); - // Emitted when an outbound transfer is initiated, i.e. tokens are being withdrawn from L2 back to L1 + /// Emitted when an outbound transfer is initiated, i.e. tokens are being withdrawn from L2 back to L1 event WithdrawalInitiated( address l1Token, address indexed from, @@ -54,11 +56,11 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran uint256 amount ); - // Emitted when the Arbitrum Gateway Router address on L2 has been updated + /// Emitted when the Arbitrum Gateway Router address on L2 has been updated event L2RouterSet(address l2Router); - // Emitted when the L1 Graph Token address has been updated + /// Emitted when the L1 Graph Token address has been updated event L1TokenAddressSet(address l1GRT); - // Emitted when the address of the counterpart gateway on L1 has been updated + /// Emitted when the address of the counterpart gateway on L1 has been updated event L1CounterpartAddressSet(address l1Counterpart); /** @@ -74,8 +76,8 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran } /** - * @dev Initialize this contract. - * The contract will be paused. + * @notice Initialize the L2GraphTokenGateway contract. + * @dev The contract will be paused. * Note some parameters have to be set separately as they are generally * not expected to be available at initialization time: * - l2Router using setL2Router @@ -91,7 +93,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran } /** - * @dev Sets the address of the Arbitrum Gateway Router on L2 + * @notice Sets the address of the Arbitrum Gateway Router on L2 * @param _l2Router Address of the L2 Router (provided by Arbitrum) */ function setL2Router(address _l2Router) external onlyGovernor { @@ -101,7 +103,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran } /** - * @dev Sets the address of the Graph Token on L1 + * @notice Sets the address of the Graph Token on L1 * @param _l1GRT L1 address of the Graph Token contract */ function setL1TokenAddress(address _l1GRT) external onlyGovernor { @@ -111,7 +113,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran } /** - * @dev Sets the address of the counterpart gateway on L1 + * @notice Sets the address of the counterpart gateway on L1 * @param _l1Counterpart Address of the L1GraphTokenGateway on L1 */ function setL1CounterpartAddress(address _l1Counterpart) external onlyGovernor { @@ -120,6 +122,65 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran emit L1CounterpartAddressSet(_l1Counterpart); } + /** + * @notice Burns L2 tokens and initiates a transfer to L1. + * The tokens will be received on L1 only after the wait period (7 days) is over, + * and will require an Outbox.executeTransaction to finalize. + * @dev no additional callhook data is allowed + * @param _l1Token L1 Address of GRT (needed for compatibility with Arbitrum Gateway Router) + * @param _to Recipient address on L1 + * @param _amount Amount of tokens to burn + * @param _data Contains sender and additional data to send to L1 + * @return ID of the withdraw tx + */ + function outboundTransfer( + address _l1Token, + address _to, + uint256 _amount, + bytes calldata _data + ) external returns (bytes memory) { + return outboundTransfer(_l1Token, _to, _amount, 0, 0, _data); + } + + /** + * @notice Receives token amount from L1 and mints the equivalent tokens to the receiving address + * @dev Only accepts transactions from the L1 GRT Gateway. + * The function is payable for ITokenGateway compatibility, but msg.value must be zero. + * Note that allowlisted senders (some protocol contracts) can include additional calldata + * for a callhook to be executed on the L2 side when the tokens are received. In this case, the L2 transaction + * can revert if the callhook reverts, potentially locking the tokens on the bridge if the callhook + * never succeeds. This requires extra care when adding contracts to the allowlist, but is necessary to ensure that + * the tickets can be retried in the case of a temporary failure, and to ensure the atomicity of callhooks + * with token transfers. + * @param _l1Token L1 Address of GRT + * @param _from Address of the sender on L1 + * @param _to Recipient address on L2 + * @param _amount Amount of tokens transferred + * @param _data Extra callhook data, only used when the sender is allowlisted + */ + function finalizeInboundTransfer( + address _l1Token, + address _from, + address _to, + uint256 _amount, + bytes calldata _data + ) external payable override nonReentrant notPaused onlyL1Counterpart { + require(_l1Token == l1GRT, "TOKEN_NOT_GRT"); + require(msg.value == 0, "INVALID_NONZERO_VALUE"); + + L2GraphToken(calculateL2TokenAddress(l1GRT)).bridgeMint(_to, _amount); + + if (_data.length > 0) { + bytes memory callhookData; + { + (, callhookData) = abi.decode(_data, (bytes, bytes)); + } + ICallhookReceiver(_to).onTokenTransfer(_from, _amount, callhookData); + } + + emit DepositFinalized(_l1Token, _from, _to, _amount); + } + /** * @notice Burns L2 tokens and initiates a transfer to L1. * The tokens will be available on L1 only after the wait period (7 days) is over, @@ -141,7 +202,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran uint256, // unused on L2 uint256, // unused on L2 bytes calldata _data - ) public payable override notPaused nonReentrant returns (bytes memory) { + ) public payable override nonReentrant notPaused returns (bytes memory) { require(_l1Token == l1GRT, "TOKEN_NOT_GRT"); require(_amount != 0, "INVALID_ZERO_AMOUNT"); require(msg.value == 0, "INVALID_NONZERO_VALUE"); @@ -149,7 +210,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran OutboundCalldata memory outboundCalldata; - (outboundCalldata.from, outboundCalldata.extraData) = parseOutboundData(_data); + (outboundCalldata.from, outboundCalldata.extraData) = _parseOutboundData(_data); require(outboundCalldata.extraData.length == 0, "CALL_HOOK_DATA_NOT_ALLOWED"); // from needs to approve this contract to burn the amount first @@ -174,26 +235,6 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran return abi.encode(id); } - /** - * @notice Burns L2 tokens and initiates a transfer to L1. - * The tokens will be received on L1 only after the wait period (7 days) is over, - * and will require an Outbox.executeTransaction to finalize. - * @dev no additional callhook data is allowed - * @param _l1Token L1 Address of GRT (needed for compatibility with Arbitrum Gateway Router) - * @param _to Recipient address on L1 - * @param _amount Amount of tokens to burn - * @param _data Contains sender and additional data to send to L1 - * @return ID of the withdraw tx - */ - function outboundTransfer( - address _l1Token, - address _to, - uint256 _amount, - bytes calldata _data - ) external returns (bytes memory) { - return outboundTransfer(_l1Token, _to, _amount, 0, 0, _data); - } - /** * @notice Calculate the L2 address of a bridged token * @dev In our case, this would only work for GRT. @@ -207,45 +248,6 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran return address(graphToken()); } - /** - * @notice Receives token amount from L1 and mints the equivalent tokens to the receiving address - * @dev Only accepts transactions from the L1 GRT Gateway. - * The function is payable for ITokenGateway compatibility, but msg.value must be zero. - * Note that whitelisted senders (some protocol contracts) can include additional calldata - * for a callhook to be executed on the L2 side when the tokens are received. In this case, the L2 transaction - * can revert if the callhook reverts, potentially locking the tokens on the bridge if the callhook - * never succeeds. This requires extra care when adding contracts to the whitelist, but is necessary to ensure that - * the tickets can be retried in the case of a temporary failure, and to ensure the atomicity of callhooks - * with token transfers. - * @param _l1Token L1 Address of GRT - * @param _from Address of the sender on L1 - * @param _to Recipient address on L2 - * @param _amount Amount of tokens transferred - * @param _data Extra callhook data, only used when the sender is whitelisted - */ - function finalizeInboundTransfer( - address _l1Token, - address _from, - address _to, - uint256 _amount, - bytes calldata _data - ) external payable override notPaused onlyL1Counterpart nonReentrant { - require(_l1Token == l1GRT, "TOKEN_NOT_GRT"); - require(msg.value == 0, "INVALID_NONZERO_VALUE"); - - L2GraphToken(calculateL2TokenAddress(l1GRT)).bridgeMint(_to, _amount); - - if (_data.length > 0) { - bytes memory callhookData; - { - (, callhookData) = abi.decode(_data, (bytes, bytes)); - } - ICallhookReceiver(_to).onTokenTransfer(_from, _amount, callhookData); - } - - emit DepositFinalized(_l1Token, _from, _to, _amount); - } - /** * @notice Creates calldata required to send tx to L1 * @dev encodes the target function with its params which @@ -275,6 +277,16 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran ); } + /** + * @dev Runs state validation before unpausing, reverts if + * something is not set properly + */ + function _checksBeforeUnpause() internal view override { + require(l2Router != address(0), "L2_ROUTER_NOT_SET"); + require(l1Counterpart != address(0), "L1_COUNTERPART_NOT_SET"); + require(l1GRT != address(0), "L1_GRT_NOT_SET"); + } + /** * @notice Decodes calldata required for migration of tokens * @dev extraData can be left empty @@ -282,7 +294,7 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran * @return Sender of the tx * @return Any other data sent to L1 */ - function parseOutboundData(bytes calldata _data) private view returns (address, bytes memory) { + function _parseOutboundData(bytes calldata _data) private view returns (address, bytes memory) { address from; bytes memory extraData; if (msg.sender == l2Router) { @@ -293,14 +305,4 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran } return (from, extraData); } - - /** - * @dev Runs state validation before unpausing, reverts if - * something is not set properly - */ - function _checksBeforeUnpause() internal view override { - require(l2Router != address(0), "ROUTER_NOT_SET"); - require(l1Counterpart != address(0), "L1_COUNTERPART_NOT_SET"); - require(l1GRT != address(0), "L1GRT_NOT_SET"); - } } diff --git a/contracts/l2/token/GraphTokenUpgradeable.sol b/contracts/l2/token/GraphTokenUpgradeable.sol index ac6ce63f0..3f7882b5c 100644 --- a/contracts/l2/token/GraphTokenUpgradeable.sol +++ b/contracts/l2/token/GraphTokenUpgradeable.sol @@ -2,11 +2,11 @@ pragma solidity ^0.7.6; -import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20BurnableUpgradeable.sol"; -import "@openzeppelin/contracts/cryptography/ECDSA.sol"; +import { ERC20BurnableUpgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20BurnableUpgradeable.sol"; +import { ECDSAUpgradeable } from "@openzeppelin/contracts-upgradeable/cryptography/ECDSAUpgradeable.sol"; -import "../../upgrades/GraphUpgradeable.sol"; -import "../../governance/Governed.sol"; +import { GraphUpgradeable } from "../../upgrades/GraphUpgradeable.sol"; +import { Governed } from "../../governance/Governed.sol"; /** * @title GraphTokenUpgradeable contract @@ -24,18 +24,23 @@ import "../../governance/Governed.sol"; * initializer functions and upgradeable OpenZeppelin contracts instead of * the original's constructor + non-upgradeable approach. */ -contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgradeable { +abstract contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgradeable { // -- EIP712 -- // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-712.md#definition-of-domainseparator + /// @dev Hash of the EIP-712 Domain type bytes32 private immutable DOMAIN_TYPE_HASH = keccak256( "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract,bytes32 salt)" ); + /// @dev Hash of the EIP-712 Domain name bytes32 private immutable DOMAIN_NAME_HASH = keccak256("Graph Token"); + /// @dev Hash of the EIP-712 Domain version bytes32 private immutable DOMAIN_VERSION_HASH = keccak256("0"); + /// @dev EIP-712 Domain salt bytes32 private immutable DOMAIN_SALT = 0xe33842a7acd1d5a1d28f25a931703e5605152dc48d64dc4716efdae1f5659591; // Randomly generated salt + /// @dev Hash of the EIP-712 permit type bytes32 private immutable PERMIT_TYPEHASH = keccak256( "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)" @@ -43,25 +48,30 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra // -- State -- - // solhint-disable-next-line var-name-mixedcase - bytes32 private DOMAIN_SEPARATOR; + /// @dev EIP-712 Domain separator + bytes32 private DOMAIN_SEPARATOR; // solhint-disable-line var-name-mixedcase + /// @dev Addresses for which this mapping is true are allowed to mint tokens mapping(address => bool) private _minters; + /// Nonces for permit signatures for each token holder mapping(address => uint256) public nonces; - // Storage gap added in case we need to add state variables to this contract + /// @dev Storage gap added in case we need to add state variables to this contract uint256[47] private __gap; // -- Events -- + /// Emitted when a new minter is added event MinterAdded(address indexed account); + /// Emitted when a minter is removed event MinterRemoved(address indexed account); + /// @dev Reverts if the caller is not an authorized minter modifier onlyMinter() { require(isMinter(msg.sender), "Only minter can call"); _; } /** - * @dev Approve token allowance by validating a message signed by the holder. + * @notice Approve token allowance by validating a message signed by the holder. * @param _owner Address of the token holder * @param _spender Address of the approved spender * @param _value Amount of tokens to approve the spender @@ -79,6 +89,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra bytes32 _r, bytes32 _s ) external { + require(_deadline == 0 || block.timestamp <= _deadline, "GRT: expired permit"); bytes32 digest = keccak256( abi.encodePacked( "\x19\x01", @@ -89,16 +100,15 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra ) ); - address recoveredAddress = ECDSA.recover(digest, _v, _r, _s); + address recoveredAddress = ECDSAUpgradeable.recover(digest, _v, _r, _s); require(_owner == recoveredAddress, "GRT: invalid permit"); - require(_deadline == 0 || block.timestamp <= _deadline, "GRT: expired permit"); nonces[_owner] = nonces[_owner] + 1; _approve(_owner, _spender, _value); } /** - * @dev Add a new minter. + * @notice Add a new minter. * @param _account Address of the minter */ function addMinter(address _account) external onlyGovernor { @@ -107,7 +117,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra } /** - * @dev Remove a minter. + * @notice Remove a minter. * @param _account Address of the minter */ function removeMinter(address _account) external onlyGovernor { @@ -116,7 +126,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra } /** - * @dev Renounce to be a minter. + * @notice Renounce being a minter. */ function renounceMinter() external { require(isMinter(msg.sender), "NOT_A_MINTER"); @@ -124,7 +134,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra } /** - * @dev Mint new tokens. + * @notice Mint new tokens. * @param _to Address to send the newly minted tokens * @param _amount Amount of tokens to mint */ @@ -133,7 +143,7 @@ contract GraphTokenUpgradeable is GraphUpgradeable, Governed, ERC20BurnableUpgra } /** - * @dev Return if the `_account` is a minter or not. + * @notice Return if the `_account` is a minter or not. * @param _account Address to check * @return True if the `_account` is minter */ diff --git a/contracts/l2/token/L2GraphToken.sol b/contracts/l2/token/L2GraphToken.sol index 7a5abf78c..639444870 100644 --- a/contracts/l2/token/L2GraphToken.sol +++ b/contracts/l2/token/L2GraphToken.sol @@ -2,10 +2,8 @@ pragma solidity ^0.7.6; -import "@openzeppelin/contracts/math/SafeMath.sol"; - -import "./GraphTokenUpgradeable.sol"; -import "../../arbitrum/IArbToken.sol"; +import { GraphTokenUpgradeable } from "./GraphTokenUpgradeable.sol"; +import { IArbToken } from "../../arbitrum/IArbToken.sol"; /** * @title L2 Graph Token Contract @@ -13,20 +11,18 @@ import "../../arbitrum/IArbToken.sol"; * through the L2GraphTokenGateway. */ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { - using SafeMath for uint256; - - // Address of the gateway (on L2) that is allowed to mint tokens + /// Address of the gateway (on L2) that is allowed to mint tokens address public gateway; - // Address of the corresponding Graph Token contract on L1 + /// Address of the corresponding Graph Token contract on L1 address public override l1Address; - // Emitted when the bridge / gateway has minted new tokens, i.e. tokens were transferred to L2 + /// Emitted when the bridge / gateway has minted new tokens, i.e. tokens were transferred to L2 event BridgeMinted(address indexed account, uint256 amount); - // Emitted when the bridge / gateway has burned tokens, i.e. tokens were transferred back to L1 + /// Emitted when the bridge / gateway has burned tokens, i.e. tokens were transferred back to L1 event BridgeBurned(address indexed account, uint256 amount); - // Emitted when the address of the gateway has been updated + /// Emitted when the address of the gateway has been updated event GatewaySet(address gateway); - // Emitted when the address of the Graph Token contract on L1 has been updated + /// Emitted when the address of the Graph Token contract on L1 has been updated event L1AddressSet(address l1Address); /** @@ -38,8 +34,8 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { } /** - * @dev L2 Graph Token Contract initializer. - * Note some parameters have to be set separately as they are generally + * @notice L2 Graph Token Contract initializer. + * @dev Note some parameters have to be set separately as they are generally * not expected to be available at initialization time: * - gateway using setGateway * - l1Address using setL1Address @@ -53,7 +49,7 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { } /** - * @dev Sets the address of the L2 gateway allowed to mint tokens + * @notice Sets the address of the L2 gateway allowed to mint tokens * @param _gw Address for the L2GraphTokenGateway that will be allowed to mint tokens */ function setGateway(address _gw) external onlyGovernor { @@ -63,7 +59,7 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { } /** - * @dev Sets the address of the counterpart token on L1 + * @notice Sets the address of the counterpart token on L1 * @param _addr Address for the GraphToken contract on L1 */ function setL1Address(address _addr) external onlyGovernor { @@ -73,7 +69,7 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { } /** - * @dev Increases token supply, only callable by the L1/L2 bridge (when tokens are transferred to L2) + * @notice Increases token supply, only callable by the L1/L2 bridge (when tokens are transferred to L2) * @param _account Address to credit with the new tokens * @param _amount Number of tokens to mint */ @@ -83,7 +79,7 @@ contract L2GraphToken is GraphTokenUpgradeable, IArbToken { } /** - * @dev Decreases token supply, only callable by the L1/L2 bridge (when tokens are transferred to L1). + * @notice Decreases token supply, only callable by the L1/L2 bridge (when tokens are transferred to L1). * @param _account Address from which to extract the tokens * @param _amount Number of tokens to burn */ diff --git a/contracts/upgrades/GraphProxy.sol b/contracts/upgrades/GraphProxy.sol index cf7ad7812..d3f6eacec 100644 --- a/contracts/upgrades/GraphProxy.sol +++ b/contracts/upgrades/GraphProxy.sol @@ -2,9 +2,11 @@ pragma solidity ^0.7.6; -import "@openzeppelin/contracts/utils/Address.sol"; +import { Address } from "@openzeppelin/contracts/utils/Address.sol"; -import "./GraphProxyStorage.sol"; +import { GraphProxyStorage } from "./GraphProxyStorage.sol"; + +import { IGraphProxy } from "./IGraphProxy.sol"; /** * @title Graph Proxy @@ -13,13 +15,13 @@ import "./GraphProxyStorage.sol"; * This contract implements a proxy that is upgradeable by an admin. * https://docs.openzeppelin.com/upgrades-plugins/1.x/proxies#transparent-proxies-and-function-clashes */ -contract GraphProxy is GraphProxyStorage { +contract GraphProxy is GraphProxyStorage, IGraphProxy { /** * @dev Modifier used internally that will delegate the call to the implementation unless * the sender is the admin. */ modifier ifAdmin() { - if (msg.sender == _admin()) { + if (msg.sender == _getAdmin()) { _; } else { _fallback(); @@ -31,7 +33,7 @@ contract GraphProxy is GraphProxyStorage { * the sender is the admin or pending implementation. */ modifier ifAdminOrPendingImpl() { - if (msg.sender == _admin() || msg.sender == _pendingImplementation()) { + if (msg.sender == _getAdmin() || msg.sender == _getPendingImplementation()) { _; } else { _fallback(); @@ -39,7 +41,7 @@ contract GraphProxy is GraphProxyStorage { } /** - * @dev Contract constructor. + * @notice GraphProxy contract constructor. * @param _impl Address of the initial implementation * @param _admin Address of the proxy admin */ @@ -58,86 +60,111 @@ contract GraphProxy is GraphProxyStorage { } /** - * @dev Returns the current admin. + * @notice Fallback function that delegates calls to implementation. Will run if call data + * is empty. + */ + receive() external payable { + _fallback(); + } + + /** + * @notice Fallback function that delegates calls to implementation. Will run if no other + * function in the contract matches the call data. + */ + fallback() external payable { + _fallback(); + } + + /** + * @notice Get the current admin * - * NOTE: Only the admin and implementation can call this function. + * @dev NOTE: Only the admin and implementation can call this function. * * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using the * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call. * `0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103` + * + * @return The address of the current admin */ - function admin() external ifAdminOrPendingImpl returns (address) { - return _admin(); + function admin() external override ifAdminOrPendingImpl returns (address) { + return _getAdmin(); } /** - * @dev Returns the current implementation. + * @notice Get the current implementation. * - * NOTE: Only the admin can call this function. + * @dev NOTE: Only the admin can call this function. * * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using the * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call. * `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` + * + * @return The address of the current implementation for this proxy */ - function implementation() external ifAdminOrPendingImpl returns (address) { - return _implementation(); + function implementation() external override ifAdminOrPendingImpl returns (address) { + return _getImplementation(); } /** - * @dev Returns the current pending implementation. + * @notice Get the current pending implementation. * - * NOTE: Only the admin can call this function. + * @dev NOTE: Only the admin can call this function. * * TIP: To get this value clients can read directly from the storage slot shown below (specified by EIP1967) using the * https://eth.wiki/json-rpc/API#eth_getstorageat[`eth_getStorageAt`] RPC call. * `0x9e5eddc59e0b171f57125ab86bee043d9128098c3a6b9adb4f2e86333c2f6f8c` + * + * @return The address of the current pending implementation for this proxy */ - function pendingImplementation() external ifAdminOrPendingImpl returns (address) { - return _pendingImplementation(); + function pendingImplementation() external override ifAdminOrPendingImpl returns (address) { + return _getPendingImplementation(); } /** - * @dev Changes the admin of the proxy. + * @notice Changes the admin of the proxy. * - * NOTE: Only the admin can call this function. + * @dev NOTE: Only the admin can call this function. + * + * @param _newAdmin Address of the new admin */ - function setAdmin(address _newAdmin) external ifAdmin { + function setAdmin(address _newAdmin) external override ifAdmin { require(_newAdmin != address(0), "Admin cant be the zero address"); _setAdmin(_newAdmin); } /** - * @dev Upgrades to a new implementation contract. + * @notice Upgrades to a new implementation contract. + * @dev NOTE: Only the admin can call this function. * @param _newImplementation Address of implementation contract - * - * NOTE: Only the admin can call this function. */ - function upgradeTo(address _newImplementation) external ifAdmin { + function upgradeTo(address _newImplementation) external override ifAdmin { _setPendingImplementation(_newImplementation); } /** - * @dev Admin function for new implementation to accept its role as implementation. + * @notice Admin function for new implementation to accept its role as implementation. */ - function acceptUpgrade() external ifAdminOrPendingImpl { + function acceptUpgrade() external override ifAdminOrPendingImpl { _acceptUpgrade(); } /** - * @dev Admin function for new implementation to accept its role as implementation. + * @notice Admin function for new implementation to accept its role as implementation, + * calling a function on the new implementation. + * @param data Calldata (including selector) for the function to delegatecall into the implementation */ - function acceptUpgradeAndCall(bytes calldata data) external ifAdminOrPendingImpl { + function acceptUpgradeAndCall(bytes calldata data) external override ifAdminOrPendingImpl { _acceptUpgrade(); // solhint-disable-next-line avoid-low-level-calls - (bool success, ) = _implementation().delegatecall(data); - require(success); + (bool success, ) = _getImplementation().delegatecall(data); + require(success, "Impl call failed"); } /** * @dev Admin function for new implementation to accept its role as implementation. */ function _acceptUpgrade() internal { - address _pendingImplementation = _pendingImplementation(); + address _pendingImplementation = _getPendingImplementation(); require(Address.isContract(_pendingImplementation), "Impl must be a contract"); require(_pendingImplementation != address(0), "Impl cannot be zero address"); require(msg.sender == _pendingImplementation, "Only pending implementation"); @@ -152,8 +179,9 @@ contract GraphProxy is GraphProxyStorage { * external caller. */ function _fallback() internal { - require(msg.sender != _admin(), "Cannot fallback to proxy target"); + require(msg.sender != _getAdmin(), "Cannot fallback to proxy target"); + // solhint-disable-next-line no-inline-assembly assembly { // (a) get free memory pointer let ptr := mload(0x40) @@ -181,20 +209,4 @@ contract GraphProxy is GraphProxyStorage { } } } - - /** - * @dev Fallback function that delegates calls to implementation. Will run if no other - * function in the contract matches the call data. - */ - fallback() external payable { - _fallback(); - } - - /** - * @dev Fallback function that delegates calls to implementation. Will run if call data - * is empty. - */ - receive() external payable { - _fallback(); - } } diff --git a/contracts/upgrades/GraphProxyAdmin.sol b/contracts/upgrades/GraphProxyAdmin.sol index 3775b9df1..d96dbd449 100644 --- a/contracts/upgrades/GraphProxyAdmin.sol +++ b/contracts/upgrades/GraphProxyAdmin.sol @@ -2,10 +2,10 @@ pragma solidity ^0.7.6; -import "../governance/Governed.sol"; +import { Governed } from "../governance/Governed.sol"; -import "./IGraphProxy.sol"; -import "./GraphUpgradeable.sol"; +import { IGraphProxy } from "./IGraphProxy.sol"; +import { GraphUpgradeable } from "./GraphUpgradeable.sol"; /** * @title GraphProxyAdmin @@ -16,79 +16,85 @@ import "./GraphUpgradeable.sol"; */ contract GraphProxyAdmin is Governed { /** - * @dev Contract constructor. + * @notice Contract constructor. */ constructor() { Governed._initialize(msg.sender); } /** - * @dev Returns the current implementation of a proxy. - * This is needed because only the proxy admin can query it. + * @notice Returns the current implementation of a proxy. + * @dev This is needed because only the proxy admin can query it. + * @param _proxy Address of the proxy for which to get the implementation. * @return The address of the current implementation of the proxy. */ - function getProxyImplementation(IGraphProxy _proxy) public view returns (address) { + function getProxyImplementation(IGraphProxy _proxy) external view returns (address) { // We need to manually run the static call since the getter cannot be flagged as view // bytes4(keccak256("implementation()")) == 0x5c60da1b (bool success, bytes memory returndata) = address(_proxy).staticcall(hex"5c60da1b"); - require(success); + require(success, "Proxy impl call failed"); return abi.decode(returndata, (address)); } /** - * @dev Returns the pending implementation of a proxy. - * This is needed because only the proxy admin can query it. + * @notice Returns the pending implementation of a proxy. + * @dev This is needed because only the proxy admin can query it. + * @param _proxy Address of the proxy for which to get the pending implementation. * @return The address of the pending implementation of the proxy. */ - function getProxyPendingImplementation(IGraphProxy _proxy) public view returns (address) { + function getProxyPendingImplementation(IGraphProxy _proxy) external view returns (address) { // We need to manually run the static call since the getter cannot be flagged as view // bytes4(keccak256("pendingImplementation()")) == 0x396f7b23 (bool success, bytes memory returndata) = address(_proxy).staticcall(hex"396f7b23"); - require(success); + require(success, "Proxy pendingImpl call failed"); return abi.decode(returndata, (address)); } /** - * @dev Returns the admin of a proxy. Only the admin can query it. + * @notice Returns the admin of a proxy. Only the admin can query it. + * @param _proxy Address of the proxy for which to get the admin. * @return The address of the current admin of the proxy. */ - function getProxyAdmin(IGraphProxy _proxy) public view returns (address) { + function getProxyAdmin(IGraphProxy _proxy) external view returns (address) { // We need to manually run the static call since the getter cannot be flagged as view // bytes4(keccak256("admin()")) == 0xf851a440 (bool success, bytes memory returndata) = address(_proxy).staticcall(hex"f851a440"); - require(success); + require(success, "Proxy admin call failed"); return abi.decode(returndata, (address)); } /** - * @dev Changes the admin of a proxy. + * @notice Changes the admin of a proxy. * @param _proxy Proxy to change admin. * @param _newAdmin Address to transfer proxy administration to. */ - function changeProxyAdmin(IGraphProxy _proxy, address _newAdmin) public onlyGovernor { + function changeProxyAdmin(IGraphProxy _proxy, address _newAdmin) external onlyGovernor { _proxy.setAdmin(_newAdmin); } /** - * @dev Upgrades a proxy to the newest implementation of a contract. + * @notice Upgrades a proxy to the newest implementation of a contract. * @param _proxy Proxy to be upgraded. * @param _implementation the address of the Implementation. */ - function upgrade(IGraphProxy _proxy, address _implementation) public onlyGovernor { + function upgrade(IGraphProxy _proxy, address _implementation) external onlyGovernor { _proxy.upgradeTo(_implementation); } /** - * @dev Accepts a proxy. + * @notice Accepts a proxy. * @param _implementation Address of the implementation accepting the proxy. * @param _proxy Address of the proxy being accepted. */ - function acceptProxy(GraphUpgradeable _implementation, IGraphProxy _proxy) public onlyGovernor { + function acceptProxy(GraphUpgradeable _implementation, IGraphProxy _proxy) + external + onlyGovernor + { _implementation.acceptProxy(_proxy); } /** - * @dev Accepts a proxy and call a function on the implementation. + * @notice Accepts a proxy and call a function on the implementation. * @param _implementation Address of the implementation accepting the proxy. * @param _proxy Address of the proxy being accepted. * @param _data Encoded function to call on the implementation after accepting the proxy. diff --git a/contracts/upgrades/GraphProxyStorage.sol b/contracts/upgrades/GraphProxyStorage.sol index 950f9776b..b308c0a0c 100644 --- a/contracts/upgrades/GraphProxyStorage.sol +++ b/contracts/upgrades/GraphProxyStorage.sol @@ -8,7 +8,7 @@ pragma solidity ^0.7.6; * This contract does not actually define state variables managed by the compiler * but uses fixed slot locations. */ -contract GraphProxyStorage { +abstract contract GraphProxyStorage { /** * @dev Storage slot with the address of the current implementation. * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is @@ -59,15 +59,16 @@ contract GraphProxyStorage { * @dev Modifier to check whether the `msg.sender` is the admin. */ modifier onlyAdmin() { - require(msg.sender == _admin(), "Caller must be admin"); + require(msg.sender == _getAdmin(), "Caller must be admin"); _; } /** * @return adm The admin slot. */ - function _admin() internal view returns (address adm) { + function _getAdmin() internal view returns (address adm) { bytes32 slot = ADMIN_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { adm := sload(slot) } @@ -78,20 +79,23 @@ contract GraphProxyStorage { * @param _newAdmin Address of the new proxy admin */ function _setAdmin(address _newAdmin) internal { + address oldAdmin = _getAdmin(); bytes32 slot = ADMIN_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { sstore(slot, _newAdmin) } - emit AdminUpdated(_admin(), _newAdmin); + emit AdminUpdated(oldAdmin, _newAdmin); } /** * @dev Returns the current implementation. * @return impl Address of the current implementation */ - function _implementation() internal view returns (address impl) { + function _getImplementation() internal view returns (address impl) { bytes32 slot = IMPLEMENTATION_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { impl := sload(slot) } @@ -101,8 +105,9 @@ contract GraphProxyStorage { * @dev Returns the current pending implementation. * @return impl Address of the current pending implementation */ - function _pendingImplementation() internal view returns (address impl) { + function _getPendingImplementation() internal view returns (address impl) { bytes32 slot = PENDING_IMPLEMENTATION_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { impl := sload(slot) } @@ -113,9 +118,10 @@ contract GraphProxyStorage { * @param _newImplementation Address of the new implementation */ function _setImplementation(address _newImplementation) internal { - address oldImplementation = _implementation(); + address oldImplementation = _getImplementation(); bytes32 slot = IMPLEMENTATION_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { sstore(slot, _newImplementation) } @@ -128,9 +134,10 @@ contract GraphProxyStorage { * @param _newImplementation Address of the new pending implementation */ function _setPendingImplementation(address _newImplementation) internal { - address oldPendingImplementation = _pendingImplementation(); + address oldPendingImplementation = _getPendingImplementation(); bytes32 slot = PENDING_IMPLEMENTATION_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { sstore(slot, _newImplementation) } diff --git a/contracts/upgrades/GraphUpgradeable.sol b/contracts/upgrades/GraphUpgradeable.sol index 3f3b505ba..92ce80ad7 100644 --- a/contracts/upgrades/GraphUpgradeable.sol +++ b/contracts/upgrades/GraphUpgradeable.sol @@ -2,13 +2,13 @@ pragma solidity ^0.7.6; -import "./IGraphProxy.sol"; +import { IGraphProxy } from "./IGraphProxy.sol"; /** * @title Graph Upgradeable * @dev This contract is intended to be inherited from upgradeable contracts. */ -contract GraphUpgradeable { +abstract contract GraphUpgradeable { /** * @dev Storage slot with the address of the current implementation. * This is the keccak-256 hash of "eip1967.proxy.implementation" subtracted by 1, and is @@ -39,22 +39,26 @@ contract GraphUpgradeable { */ function _implementation() internal view returns (address impl) { bytes32 slot = IMPLEMENTATION_SLOT; + // solhint-disable-next-line no-inline-assembly assembly { impl := sload(slot) } } /** - * @dev Accept to be an implementation of proxy. + * @notice Accept to be an implementation of proxy. + * @param _proxy Proxy to accept */ function acceptProxy(IGraphProxy _proxy) external onlyProxyAdmin(_proxy) { _proxy.acceptUpgrade(); } /** - * @dev Accept to be an implementation of proxy and then call a function from the new + * @notice Accept to be an implementation of proxy and then call a function from the new * implementation as specified by `_data`, which should be an encoded function call. This is * useful to initialize new storage variables in the proxied contract. + * @param _proxy Proxy to accept + * @param _data Calldata for the initialization function call (including selector) */ function acceptProxyAndCall(IGraphProxy _proxy, bytes calldata _data) external diff --git a/e2e/deployment/config/l1/l1GraphTokenGateway.test.ts b/e2e/deployment/config/l1/l1GraphTokenGateway.test.ts index cce20c735..88abc6c1c 100644 --- a/e2e/deployment/config/l1/l1GraphTokenGateway.test.ts +++ b/e2e/deployment/config/l1/l1GraphTokenGateway.test.ts @@ -54,15 +54,15 @@ describe('[L1] L1GraphTokenGateway configuration', function () { await expect(tx).revertedWith('Only Controller governor') }) - it('addToCallhookWhitelist should revert', async function () { - const tx = L1GraphTokenGateway.connect(unauthorized).addToCallhookWhitelist( + it('addToCallhookAllowlist should revert', async function () { + const tx = L1GraphTokenGateway.connect(unauthorized).addToCallhookAllowlist( unauthorized.address, ) await expect(tx).revertedWith('Only Controller governor') }) - it('removeFromCallhookWhitelist should revert', async function () { - const tx = L1GraphTokenGateway.connect(unauthorized).removeFromCallhookWhitelist( + it('removeFromCallhookAllowlist should revert', async function () { + const tx = L1GraphTokenGateway.connect(unauthorized).removeFromCallhookAllowlist( unauthorized.address, ) await expect(tx).revertedWith('Only Controller governor') diff --git a/test/gateway/l1GraphTokenGateway.test.ts b/test/gateway/l1GraphTokenGateway.test.ts index 7c2d3db5e..2542832ab 100644 --- a/test/gateway/l1GraphTokenGateway.test.ts +++ b/test/gateway/l1GraphTokenGateway.test.ts @@ -205,59 +205,59 @@ describe('L1GraphTokenGateway', () => { expect(await l1GraphTokenGateway.escrow()).eq(bridgeEscrow.address) }) }) - describe('addToCallhookWhitelist', function () { + describe('addToCallhookAllowlist', function () { it('is not callable by addreses that are not the governor', async function () { const tx = l1GraphTokenGateway .connect(tokenSender.signer) - .addToCallhookWhitelist(fixtureContracts.rewardsManager.address) + .addToCallhookAllowlist(fixtureContracts.rewardsManager.address) await expect(tx).revertedWith('Only Controller governor') expect( - await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address), + await l1GraphTokenGateway.callhookAllowlist(fixtureContracts.rewardsManager.address), ).eq(false) }) - it('rejects adding an EOA to the callhook whitelist', async function () { + it('rejects adding an EOA to the callhook allowlist', async function () { const tx = l1GraphTokenGateway .connect(governor.signer) - .addToCallhookWhitelist(tokenSender.address) + .addToCallhookAllowlist(tokenSender.address) await expect(tx).revertedWith('MUST_BE_CONTRACT') }) - it('adds an address to the callhook whitelist', async function () { + it('adds an address to the callhook allowlist', async function () { const tx = l1GraphTokenGateway .connect(governor.signer) - .addToCallhookWhitelist(fixtureContracts.rewardsManager.address) + .addToCallhookAllowlist(fixtureContracts.rewardsManager.address) await expect(tx) - .emit(l1GraphTokenGateway, 'AddedToCallhookWhitelist') + .emit(l1GraphTokenGateway, 'AddedToCallhookAllowlist') .withArgs(fixtureContracts.rewardsManager.address) expect( - await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address), + await l1GraphTokenGateway.callhookAllowlist(fixtureContracts.rewardsManager.address), ).eq(true) }) }) - describe('removeFromCallhookWhitelist', function () { + describe('removeFromCallhookAllowlist', function () { it('is not callable by addreses that are not the governor', async function () { await l1GraphTokenGateway .connect(governor.signer) - .addToCallhookWhitelist(fixtureContracts.rewardsManager.address) + .addToCallhookAllowlist(fixtureContracts.rewardsManager.address) const tx = l1GraphTokenGateway .connect(tokenSender.signer) - .removeFromCallhookWhitelist(fixtureContracts.rewardsManager.address) + .removeFromCallhookAllowlist(fixtureContracts.rewardsManager.address) await expect(tx).revertedWith('Only Controller governor') expect( - await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address), + await l1GraphTokenGateway.callhookAllowlist(fixtureContracts.rewardsManager.address), ).eq(true) }) - it('removes an address from the callhook whitelist', async function () { + it('removes an address from the callhook allowlist', async function () { await l1GraphTokenGateway .connect(governor.signer) - .addToCallhookWhitelist(fixtureContracts.rewardsManager.address) + .addToCallhookAllowlist(fixtureContracts.rewardsManager.address) const tx = l1GraphTokenGateway .connect(governor.signer) - .removeFromCallhookWhitelist(fixtureContracts.rewardsManager.address) + .removeFromCallhookAllowlist(fixtureContracts.rewardsManager.address) await expect(tx) - .emit(l1GraphTokenGateway, 'RemovedFromCallhookWhitelist') + .emit(l1GraphTokenGateway, 'RemovedFromCallhookAllowlist') .withArgs(fixtureContracts.rewardsManager.address) expect( - await l1GraphTokenGateway.callhookWhitelist(fixtureContracts.rewardsManager.address), + await l1GraphTokenGateway.callhookAllowlist(fixtureContracts.rewardsManager.address), ).eq(false) }) }) @@ -492,7 +492,7 @@ describe('L1GraphTokenGateway', () => { ) await expect(tx).revertedWith('NO_SUBMISSION_COST') }) - it('reverts when called with nonempty calldata, if the sender is not whitelisted', async function () { + it('reverts when called with nonempty calldata, if the sender is not allowlisted', async function () { await grt.connect(tokenSender.signer).approve(l1GraphTokenGateway.address, toGRT('10')) const tx = l1GraphTokenGateway .connect(tokenSender.signer) @@ -509,12 +509,12 @@ describe('L1GraphTokenGateway', () => { ) await expect(tx).revertedWith('CALL_HOOK_DATA_NOT_ALLOWED') }) - it('allows sending nonempty calldata, if the sender is whitelisted', async function () { + it('allows sending nonempty calldata, if the sender is allowlisted', async function () { // Make the sender a contract so that it can be allowed to send callhooks await provider().send('hardhat_setCode', [tokenSender.address, '0x1234']) await l1GraphTokenGateway .connect(governor.signer) - .addToCallhookWhitelist(tokenSender.address) + .addToCallhookAllowlist(tokenSender.address) await grt.connect(tokenSender.signer).approve(l1GraphTokenGateway.address, toGRT('10')) await testValidOutboundTransfer( tokenSender.signer, diff --git a/test/l2/l2GraphTokenGateway.test.ts b/test/l2/l2GraphTokenGateway.test.ts index 7c222b73d..f85ea7dfb 100644 --- a/test/l2/l2GraphTokenGateway.test.ts +++ b/test/l2/l2GraphTokenGateway.test.ts @@ -175,7 +175,7 @@ describe('L2GraphTokenGateway', () => { }) it('cannot be paused if some state variables are not set', async function () { let tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false) - await expect(tx).revertedWith('ROUTER_NOT_SET') + await expect(tx).revertedWith('L2_ROUTER_NOT_SET') await l2GraphTokenGateway.connect(governor.signer).setL2Router(mockRouter.address) tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false) await expect(tx).revertedWith('L1_COUNTERPART_NOT_SET') @@ -183,7 +183,7 @@ describe('L2GraphTokenGateway', () => { .connect(governor.signer) .setL1CounterpartAddress(mockL1Gateway.address) tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false) - await expect(tx).revertedWith('L1GRT_NOT_SET') + await expect(tx).revertedWith('L1_GRT_NOT_SET') }) it('can be paused and unpaused by the governor', async function () { await fixture.configureL2Bridge(