Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C4 QA fixes for #699 #743

Merged
merged 19 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
23f9dad
fix: avoid variable shadowing (C4 QA)
pcarranzav Oct 24, 2022
d90a2f3
fix: improve NatSpec and a few other details (C4 QA)
pcarranzav Oct 24, 2022
289f328
fix: typos in L1GraphTokenGateway (C4 QA)
pcarranzav Oct 24, 2022
d89c7a7
fix: make public functions external in GraphProxyAdmin (C4 QA)
pcarranzav Oct 24, 2022
a42bcbc
fix: make nonReentrant the first modifier (C4 QA)
pcarranzav Oct 24, 2022
ba1a870
fix: use underscore for private variables (C4 QA)
pcarranzav Oct 24, 2022
751240d
fix: emit the correct old admin in _setAdmin (C4 QA)
pcarranzav Oct 24, 2022
e789403
fix: prefer the term 'allowlist' for callhook senders
pcarranzav Oct 24, 2022
8d73fc3
fix: check deadline before recovering signature (C4 QA)
pcarranzav Oct 24, 2022
c74885f
fix: add some missing revert strings (C4 QA)
pcarranzav Oct 24, 2022
cc78a5c
fix: dont allow the router to be allowlisted (C4 QA)
pcarranzav Oct 24, 2022
38fa468
fix: mark some non-deployable contracts as abstract (C4 QA)
pcarranzav Oct 24, 2022
db2a17c
fix(Controller): mark a function that was public as external
pcarranzav Oct 24, 2022
dfd0a24
fix: use upgradeable OZ contracts (C4 QA)
pcarranzav Oct 24, 2022
eeb4bd0
fix: use explicit imports (C4 QA)
pcarranzav Oct 24, 2022
ae03579
fix: standardize function order in some contracts (C4 QA)
pcarranzav Oct 24, 2022
19928cc
fix: whitelist to allowlist
tmigone Oct 25, 2022
4bea094
fix: adjust some revert strings
pcarranzav Oct 25, 2022
ce0d0a3
test: fix revert strings
pcarranzav Oct 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/commands/bridge/to-l2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 14 additions & 11 deletions contracts/curation/Curation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
tmigone marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
4 changes: 3 additions & 1 deletion contracts/curation/CurationStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 --
Expand Down
12 changes: 6 additions & 6 deletions contracts/gateway/BridgeEscrow.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -16,23 +16,23 @@ 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 {
Managed._initialize(_controller);
}

/**
* @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 {
graphToken().approve(_spender, type(uint256).max);
}

/**
* @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 {
Expand Down
27 changes: 14 additions & 13 deletions contracts/gateway/GraphTokenGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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)
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions contracts/gateway/ICallhookReceiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading