feature: make main contract pausable (#110)

* feat: add FleekPausable

* feat: add FleekPausable functions in FleekERC721

* fix: requirePaused logic

* feat: add pause modifiers in FleekERC721

* refactor: move functions to the main contract to add pause modifier

* test: add unpause to test setups

* fix: revokeTokenRole modifier

* test: add initial tests and setup for pausable

* test: all test for pause and pausable states

* test: add test for functions when contract is paused

* test: add pausable hardhat tests

* test: foundry access control test for pausable

* refactor: function names

* fix: remove virtual keywords for functions that must not be overriden

* refactor: set inital state for unpaused
This commit is contained in:
Felipe Mendes 2023-02-21 10:56:26 -03:00 committed by GitHub
parent 197a7a28c5
commit 0f05a912a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 599 additions and 61 deletions

View File

@ -60,62 +60,6 @@ contract FleekAccessControl is Initializable {
_;
}
/**
* @dev Grants the collection role to an address.
*
* Requirements:
*
* - the caller should have the collection role.
*
*/
function grantCollectionRole(Roles role, address account) public requireCollectionRole(Roles.Owner) {
_grantCollectionRole(role, account);
}
/**
* @dev Grants the token role to an address.
*
* Requirements:
*
* - the caller should have the token role.
*
*/
function grantTokenRole(
uint256 tokenId,
Roles role,
address account
) public requireTokenRole(tokenId, Roles.Owner) {
_grantTokenRole(tokenId, role, account);
}
/**
* @dev Revokes the collection role of an address.
*
* Requirements:
*
* - the caller should have the collection role.
*
*/
function revokeCollectionRole(Roles role, address account) public requireCollectionRole(Roles.Owner) {
_revokeCollectionRole(role, account);
}
/**
* @dev Revokes the token role of an address.
*
* Requirements:
*
* - the caller should have the token role.
*
*/
function revokeTokenRole(
uint256 tokenId,
Roles role,
address account
) public requireTokenRole(tokenId, Roles.Owner) {
_revokeTokenRole(tokenId, role, account);
}
/**
* @dev Returns `True` if a certain address has the collection role.
*/
@ -228,4 +172,11 @@ contract FleekAccessControl is Initializable {
_clearAllTokenRoles(tokenId);
_grantTokenRole(tokenId, Roles.Owner, newOwner);
}
/**
* @dev This empty reserved space is put in place to allow future versions to add new
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[49] private __gap;
}

View File

@ -8,10 +8,11 @@ import "@openzeppelin/contracts/utils/Base64.sol";
import "@openzeppelin/contracts/utils/Strings.sol";
import "./FleekAccessControl.sol";
import "./util/FleekStrings.sol";
import "./FleekPausable.sol";
error ThereIsNoTokenMinted();
contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl, FleekPausable {
using Strings for uint256;
using Counters for Counters.Counter;
using FleekStrings for FleekERC721.App;
@ -84,6 +85,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
function initialize(string memory _name, string memory _symbol) public initializer {
__ERC721_init(_name, _symbol);
__FleekAccessControl_init();
__FleekPausable_init();
}
/**
@ -102,6 +104,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
* Requirements:
*
* - the caller must have ``collectionOwner``'s admin role.
* - the contract must be not paused.
*
*/
function mint(
@ -200,7 +203,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
address to,
uint256 tokenId,
uint256 batchSize
) internal virtual override {
) internal virtual override whenNotPaused {
if (from != address(0) && to != address(0)) {
// Transfer
_clearAllTokenRoles(tokenId, to);
@ -367,10 +370,11 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
* Requirements:
*
* - the tokenId must be minted and valid.
* - the contract must be not paused.
*
* IMPORTANT: The payment is not set yet
*/
function addAccessPoint(uint256 tokenId, string memory apName) public payable {
function addAccessPoint(uint256 tokenId, string memory apName) public payable whenNotPaused {
// require(msg.value == 0.1 ether, "You need to pay at least 0.1 ETH"); // TODO: define a minimum price
_requireMinted(tokenId);
require(_accessPoints[apName].owner == address(0), "FleekERC721: AP already exists");
@ -390,8 +394,10 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
*
* - the AP must exist.
* - must be called by the AP owner.
* - the contract must be not paused.
*
*/
function removeAccessPoint(string memory apName) public requireAP(apName) {
function removeAccessPoint(string memory apName) public whenNotPaused requireAP(apName) {
require(msg.sender == _accessPoints[apName].owner, "FleekERC721: must be AP owner");
uint256 tokenId = _accessPoints[apName].tokenId;
@ -524,6 +530,7 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
*
* - the tokenId must be minted and valid.
* - the sender must have the `tokenOwner` role.
* - the contract must be not paused.
*
*/
function burn(uint256 tokenId) public virtual requireTokenRole(tokenId, Roles.Owner) {
@ -533,4 +540,108 @@ contract FleekERC721 is Initializable, ERC721Upgradeable, FleekAccessControl {
delete _apps[tokenId];
}
}
/*//////////////////////////////////////////////////////////////
ACCESS CONTROL
//////////////////////////////////////////////////////////////*/
/**
* @dev Grants the collection role to an address.
*
* Requirements:
*
* - the caller should have the collection role.
*
*/
function grantCollectionRole(Roles role, address account) public whenNotPaused requireCollectionRole(Roles.Owner) {
_grantCollectionRole(role, account);
}
/**
* @dev Grants the token role to an address.
*
* Requirements:
*
* - the caller should have the token role.
*
*/
function grantTokenRole(
uint256 tokenId,
Roles role,
address account
) public whenNotPaused requireTokenRole(tokenId, Roles.Owner) {
_grantTokenRole(tokenId, role, account);
}
/**
* @dev Revokes the collection role of an address.
*
* Requirements:
*
* - the caller should have the collection role.
*
*/
function revokeCollectionRole(Roles role, address account) public whenNotPaused requireCollectionRole(Roles.Owner) {
_revokeCollectionRole(role, account);
}
/**
* @dev Revokes the token role of an address.
*
* Requirements:
*
* - the caller should have the token role.
*
*/
function revokeTokenRole(
uint256 tokenId,
Roles role,
address account
) public whenNotPaused requireTokenRole(tokenId, Roles.Owner) {
_revokeTokenRole(tokenId, role, account);
}
/*//////////////////////////////////////////////////////////////
PAUSABLE
//////////////////////////////////////////////////////////////*/
/**
* @dev Sets the contract to paused state.
*
* Requirements:
*
* - the sender must have the `controller` role.
* - the contract must be pausable.
* - the contract must be not paused.
*
*/
function pause() public requireCollectionRole(Roles.Controller) {
_pause();
}
/**
* @dev Sets the contract to unpaused state.
*
* Requirements:
*
* - the sender must have the `controller` role.
* - the contract must be paused.
*
*/
function unpause() public requireCollectionRole(Roles.Controller) {
_unpause();
}
/**
* @dev Sets the contract to pausable state.
*
* Requirements:
*
* - the sender must have the `owner` role.
* - the contract must be in the oposite pausable state.
*
*/
function setPausable(bool pausable) public requireCollectionRole(Roles.Owner) {
_setPausable(pausable);
}
}

View File

@ -0,0 +1,134 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.7;
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
error ContractIsPaused();
error ContractIsNotPaused();
error ContractIsNotPausable();
error PausableIsSetTo(bool state);
abstract contract FleekPausable is Initializable {
/**
* @dev Emitted when the pause is triggered by `account` and set to `isPaused`.
*/
event PauseStatusChange(bool indexed isPaused, address account);
/**
* @dev Emitted when the pausable is triggered by `account` and set to `isPausable`.
*/
event PausableStatusChange(bool indexed isPausable, address account);
bool private _paused;
bool private _canPause; // TODO: how should we verify if the contract is pausable or not?
/**
* @dev Initializes the contract in unpaused state.
*/
function __FleekPausable_init() internal onlyInitializing {
_paused = false;
_canPause = true;
}
/**
* @dev Modifier to make a function callable only when the contract is not paused.
*
* Requirements:
*
* - The contract must not be paused.
*/
modifier whenNotPaused() {
_requireNotPaused();
_;
}
/**
* @dev Modifier to make a function callable only when the contract is paused.
*
* Requirements:
*
* - The contract must be paused.
*/
modifier whenPaused() {
_requirePaused();
_;
}
/**
* @dev Returns true if the contract is paused, and false otherwise.
*/
function isPaused() public view returns (bool) {
return _paused;
}
/**
* @dev Returns true if the contract is pausable, and false otherwise.
*/
function isPausable() public view returns (bool) {
return _canPause;
}
/**
* @dev Throws if the contract is paused.
*/
function _requireNotPaused() internal view {
if (isPaused()) revert ContractIsPaused();
}
/**
* @dev Throws if the contract is not paused.
*/
function _requirePaused() internal view {
if (!isPaused()) revert ContractIsNotPaused();
}
/**
* @dev Throws if the contract is not pausable.
*/
function _requirePausable() internal view {
if (!isPausable()) revert ContractIsNotPausable();
}
/**
* @dev Sets the contract to be pausable or not.
* @param canPause true if the contract is pausable, and false otherwise.
*/
function _setPausable(bool canPause) internal {
if (canPause == _canPause) revert PausableIsSetTo(canPause);
_canPause = canPause;
emit PausableStatusChange(canPause, msg.sender);
}
/**
* @dev Triggers stopped state.
*
* Requirements:
*
* - The contract must not be paused.
*/
function _pause() internal whenNotPaused {
_requirePausable();
_paused = true;
emit PauseStatusChange(false, msg.sender);
}
/**
* @dev Returns to normal state.
*
* Requirements:
*
* - The contract must be paused.
*/
function _unpause() internal whenPaused {
_paused = false;
emit PauseStatusChange(false, msg.sender);
}
/**
* @dev This empty reserved space is put in place to allow future versions to add new
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[49] private __gap;
}

View File

@ -400,7 +400,7 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base {
CuT.setTokenBuild(tokenId, commitHash, gitRepository);
}
function test_testBurn() public {
function test_burn() public {
// ColletionOwner
vm.prank(collectionOwner);
expectRevertWithTokenRole();
@ -425,4 +425,68 @@ contract Test_FleekERC721_AccessControl is Test_FleekERC721_Base {
vm.prank(tokenOwner);
CuT.burn(tokenId);
}
function test_pauseAndUnpause() public {
// ColletionOwner
vm.startPrank(collectionOwner);
CuT.pause();
CuT.unpause();
vm.stopPrank();
// CollectionController
vm.startPrank(collectionController);
CuT.pause();
CuT.unpause();
vm.stopPrank();
// TokenOwner
vm.startPrank(tokenOwner);
expectRevertWithCollectionRole();
CuT.pause();
expectRevertWithCollectionRole();
CuT.unpause();
vm.stopPrank();
// TokenController
vm.startPrank(tokenController);
expectRevertWithCollectionRole();
CuT.pause();
expectRevertWithCollectionRole();
CuT.unpause();
vm.stopPrank();
// AnyAddress
vm.startPrank(anyAddress);
expectRevertWithCollectionRole();
CuT.pause();
expectRevertWithCollectionRole();
CuT.unpause();
vm.stopPrank();
}
function test_setPausable() public {
// ColletionOwner
vm.prank(collectionOwner);
CuT.setPausable(false);
// CollectionController
vm.prank(collectionController);
expectRevertWithCollectionRole();
CuT.setPausable(true);
// TokenOwner
vm.prank(tokenOwner);
expectRevertWithCollectionRole();
CuT.setPausable(true);
// TokenController
vm.prank(tokenController);
expectRevertWithCollectionRole();
CuT.setPausable(true);
// AnyAddress
vm.prank(anyAddress);
expectRevertWithCollectionRole();
CuT.setPausable(true);
}
}

View File

@ -0,0 +1,120 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;
import "./TestBase.sol";
import "contracts/FleekPausable.sol";
import "contracts/FleekAccessControl.sol";
contract Test_FleekERC721_PausableAssertions is Test {
function expectRevertWithContractIsPaused() public {
vm.expectRevert(ContractIsPaused.selector);
}
function expectRevertWithContractIsNotPaused() public {
vm.expectRevert(ContractIsNotPaused.selector);
}
function expectRevertWithContractIsNotPausable() public {
vm.expectRevert(ContractIsNotPausable.selector);
}
function expectRevertWithPausableIsSetTo(bool value) public {
vm.expectRevert(abi.encodeWithSelector(PausableIsSetTo.selector, value));
}
}
contract Test_FleekERC721_Pausable is Test_FleekERC721_Base, Test_FleekERC721_PausableAssertions {
function setUp() public {
baseSetUp();
}
function test_shouldBeInitializedPausedAndPausable() public {
assertFalse(CuT.isPaused());
assertTrue(CuT.isPausable());
}
function test_shouldUnpause() public {
CuT.pause();
assertTrue(CuT.isPaused());
CuT.unpause();
assertFalse(CuT.isPaused());
}
function test_shouldPause() public {
CuT.pause();
assertTrue(CuT.isPaused());
}
function test_cannotPauseWhenAlreadyPaused() public {
CuT.pause();
expectRevertWithContractIsPaused();
CuT.pause();
}
function test_cannotUnpauseWhenAlreadyUnpaused() public {
expectRevertWithContractIsNotPaused();
CuT.unpause();
}
function test_shouldUnpauseWhenPausableIsFalse() public {
CuT.pause();
CuT.setPausable(false);
CuT.unpause();
assertFalse(CuT.isPaused());
}
function test_cannotPauseWhenPausableIsFalse() public {
CuT.setPausable(false);
expectRevertWithContractIsNotPausable();
CuT.pause();
}
function test_cannotSetPausableWhenIsAlreadyTrue() public {
expectRevertWithPausableIsSetTo(true);
CuT.setPausable(true);
}
function test_cannotSetPausableWhenIsAlreadyFalse() public {
CuT.setPausable(false);
expectRevertWithPausableIsSetTo(false);
CuT.setPausable(false);
}
function test_shouldRevertForFunctionsWhenContractIsPaused() public {
address randomAddress = address(1);
uint256 tokenId = mintDefault(deployer);
CuT.pause();
expectRevertWithContractIsPaused();
mintDefault(deployer);
expectRevertWithContractIsPaused();
CuT.burn(tokenId);
expectRevertWithContractIsPaused();
CuT.transferFrom(deployer, randomAddress, tokenId);
expectRevertWithContractIsPaused();
CuT.addAccessPoint(tokenId, "accesspoint.com");
expectRevertWithContractIsPaused();
CuT.removeAccessPoint("accesspoint.com");
expectRevertWithContractIsPaused();
CuT.grantCollectionRole(FleekAccessControl.Roles.Controller, randomAddress);
expectRevertWithContractIsPaused();
CuT.revokeCollectionRole(FleekAccessControl.Roles.Controller, randomAddress);
expectRevertWithContractIsPaused();
CuT.grantTokenRole(tokenId, FleekAccessControl.Roles.Controller, randomAddress);
expectRevertWithContractIsPaused();
CuT.revokeTokenRole(tokenId, FleekAccessControl.Roles.Controller, randomAddress);
}
}

View File

@ -1,3 +1,7 @@
export const Errors = Object.freeze({
ContractIsPaused: 'ContractIsPaused',
ContractIsNotPaused: 'ContractIsNotPaused',
ContractIsNotPausable: 'ContractIsNotPausable',
PausableIsSetTo: 'PausableIsSetTo',
ThereIsNoTokenMinted: 'ThereIsNoTokenMinted',
});

View File

@ -2,6 +2,8 @@ import { ethers, upgrades } from 'hardhat';
import { TestConstants } from './constants';
export abstract class Fixtures {
static async paused() {}
static async default() {
// Contracts are deployed using the first signer/account by default
const [owner, otherAccount] = await ethers.getSigners();

View File

@ -0,0 +1,152 @@
import { loadFixture } from '@nomicfoundation/hardhat-network-helpers';
import { expect } from 'chai';
import { TestConstants, Fixtures, Errors } from './helpers';
const { MintParams, Roles } = TestConstants;
describe('FleekERC721.Pausable', () => {
let fixture: Awaited<ReturnType<typeof Fixtures.default>>;
const mint = () => {
const { owner, contract } = fixture;
return contract.mint(
owner.address,
MintParams.name,
MintParams.description,
MintParams.externalUrl,
MintParams.ens,
MintParams.commitHash,
MintParams.gitRepository,
MintParams.logo,
MintParams.color
);
};
beforeEach(async () => {
fixture = await loadFixture(Fixtures.default);
});
it('should be initalized as paused and pausable', async () => {
const { contract } = fixture;
expect(await contract.isPaused()).to.be.false;
expect(await contract.isPausable()).to.be.true;
});
it('should unpause', async () => {
const { contract } = fixture;
await contract.pause();
await contract.unpause();
expect(await contract.isPaused()).to.be.false;
});
it('should pause', async () => {
const { contract } = fixture;
await contract.pause();
expect(await contract.isPaused()).to.be.true;
});
it('should not allow pause if is paused', async () => {
const { contract } = fixture;
await contract.pause();
await expect(contract.pause()).to.be.revertedWithCustomError(
contract,
Errors.ContractIsPaused
);
});
it('should not allow unpause if is not paused', async () => {
const { contract } = fixture;
await expect(contract.unpause()).to.be.revertedWithCustomError(
contract,
Errors.ContractIsNotPaused
);
});
it('should unpause when contract is not pausable', async () => {
const { contract } = fixture;
await contract.pause();
await contract.setPausable(false);
await contract.unpause();
expect(await contract.isPaused()).to.be.false;
});
it('should not allow pause when contract is not pausable', async () => {
const { contract } = fixture;
await contract.setPausable(false);
await expect(contract.pause()).to.be.revertedWithCustomError(
contract,
Errors.ContractIsNotPausable
);
});
it('should not allow set pausable if is already set', async () => {
const { contract } = fixture;
await expect(contract.setPausable(true))
.to.be.revertedWithCustomError(contract, Errors.PausableIsSetTo)
.withArgs(true);
await contract.setPausable(false);
await expect(contract.setPausable(false))
.to.be.revertedWithCustomError(contract, Errors.PausableIsSetTo)
.withArgs(false);
});
it('should not allow call functions when paused', async () => {
const { contract, owner, otherAccount } = fixture;
const tokenId = (await mint()).value.toNumber();
await contract.pause();
await expect(mint()).to.be.revertedWithCustomError(
contract,
Errors.ContractIsPaused
);
await expect(contract.burn(tokenId)).to.be.revertedWithCustomError(
contract,
Errors.ContractIsPaused
);
await expect(
contract.transferFrom(owner.address, otherAccount.address, tokenId)
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.addAccessPoint(tokenId, 'accesspoint.com')
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.removeAccessPoint('accesspoint.com')
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.grantCollectionRole(Roles.Controller, otherAccount.address)
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.revokeCollectionRole(Roles.Controller, otherAccount.address)
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.grantTokenRole(Roles.Controller, tokenId, otherAccount.address)
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
await expect(
contract.revokeTokenRole(Roles.Controller, tokenId, otherAccount.address)
).to.be.revertedWithCustomError(contract, Errors.ContractIsPaused);
});
});