From b5e785bdcbd41315c644982eb4d1c5cdb81d243e Mon Sep 17 00:00:00 2001 From: Nicola Miotto Date: Fri, 20 Oct 2023 11:12:16 +0200 Subject: [PATCH] La audit fixes (#116) - [x] close #90 - [x] close #91 - [x] close #92 - [x] close #93 - [x] close #95 - [x] close #96 - [x] close #97 - [x] close #98 - [x] close #119 --------- Co-authored-by: Alberto Granzotto --- contracts/GovernanceToken/GovernanceToken.sol | 32 +++++--- .../GovernanceToken/GovernanceTokenBase.sol | 8 +- .../GovernanceTokenSnapshot.sol | 3 +- .../GovernanceToken/IGovernanceToken.sol | 3 +- contracts/InternalMarket/InternalMarket.sol | 14 ++-- .../InternalMarket/InternalMarketBase.sol | 3 +- .../NeokingdomToken/INeokingdomToken.sol | 3 +- contracts/NeokingdomToken/NeokingdomToken.sol | 2 +- .../IRedemptionController.sol | 3 +- .../RedemptionController.sol | 7 +- .../RedemptionControllerBase.sol | 2 +- .../ResolutionManager/ResolutionManager.sol | 30 ++------ .../ResolutionManagerBase.sol | 5 +- .../IShareholderRegistry.sol | 3 +- .../ShareholderRegistry.sol | 7 +- .../ShareholderRegistryBase.sol | 16 ++-- .../ShareholderRegistrySnapshot.sol | 4 +- contracts/Voting/IVoting.sol | 3 +- contracts/Voting/Voting.sol | 5 +- contracts/Voting/VotingBase.sol | 2 +- contracts/Voting/VotingSnapshot.sol | 2 +- contracts/extensions/DAORoles.sol | 2 +- contracts/extensions/HasRole.sol | 2 +- contracts/extensions/ISnapshot.sol | 2 +- contracts/extensions/Roles.sol | 2 +- contracts/extensions/Snapshottable.sol | 2 +- contracts/mocks/ERC20Mock.sol | 2 +- contracts/mocks/HasRoleMock.sol | 2 +- contracts/mocks/NeokingdomTokenMock.sol | 2 +- contracts/mocks/NeokingdomTokenV2Mock.sol | 2 +- contracts/mocks/NewTelediskoTokenMock.sol | 2 +- contracts/mocks/ResolutionExecutorMock.sol | 2 +- contracts/mocks/ResolutionManagerV2Mock.sol | 2 +- contracts/mocks/ShareholderRegistryMock.sol | 2 +- contracts/mocks/TokenMock.sol | 2 +- contracts/mocks/VotingMock.sol | 2 +- echidna/proxies/InternalMarketProxy.sol | 2 +- echidna/proxies/RedemptionControllerProxy.sol | 2 +- echidna/proxies/TokenMock.sol | 2 +- hardhat.config.ts | 2 +- test/GovernanceToken.ts | 40 ++++++++++ test/GovernanceTokenSnapshot.ts | 4 + test/Integration.ts | 77 +++++++++++++++++++ test/ResolutionManager.ts | 8 ++ test/ShareholderRegistrySnapshot.ts | 8 ++ 45 files changed, 235 insertions(+), 97 deletions(-) diff --git a/contracts/GovernanceToken/GovernanceToken.sol b/contracts/GovernanceToken/GovernanceToken.sol index 1b421ac..33a69ff 100644 --- a/contracts/GovernanceToken/GovernanceToken.sol +++ b/contracts/GovernanceToken/GovernanceToken.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; @@ -46,6 +46,10 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { string memory name, string memory symbol ) public initializer { + require( + address(roles) != address(0), + "GovernanceToken: 0x0 not allowed" + ); _initialize(name, symbol); _setRoles(roles); } @@ -154,6 +158,15 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { uint256 amount ) public virtual onlyRole(Roles.RESOLUTION_ROLE) { _mint(to, amount); + + if ( + _shareholderRegistry.isAtLeast( + _shareholderRegistry.CONTRIBUTOR_STATUS(), + to + ) + ) { + _redemptionController.afterMint(to, amount); + } } /** @@ -292,16 +305,6 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { super._afterTokenTransfer(from, to, amount); _voting.afterTokenTransfer(from, to, amount); - if ( - from == address(0) && - _shareholderRegistry.isAtLeast( - _shareholderRegistry.CONTRIBUTOR_STATUS(), - to - ) - ) { - _redemptionController.afterMint(to, amount); - } - // Invariants require( balanceOf(from) >= _vestingBalance[from], @@ -342,7 +345,12 @@ contract GovernanceToken is Initializable, HasRole, GovernanceTokenSnapshot { * @param amount Amount of external tokens to wrap. */ function _wrap(address from, uint amount) internal virtual { - tokenExternal.transferFrom(from, address(this), amount); + require( + tokenExternal.transferFrom(from, address(this), amount), + "GovernanceToken: transfer failed" + ); + require(amount > 0, "GovernanceToken: attempt to wrap 0 tokens"); + uint256 settlementTimestamp = block.timestamp + settlementPeriod; depositedTokens[from].push( DepositedTokens(amount, settlementTimestamp) diff --git a/contracts/GovernanceToken/GovernanceTokenBase.sol b/contracts/GovernanceToken/GovernanceTokenBase.sol index 7b06617..fe33fcc 100644 --- a/contracts/GovernanceToken/GovernanceTokenBase.sol +++ b/contracts/GovernanceToken/GovernanceTokenBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import "../RedemptionController/IRedemptionController.sol"; @@ -69,7 +68,10 @@ abstract contract GovernanceTokenBase is ERC20Upgradeable, IGovernanceToken { //} function _unwrap(address from, address to, uint amount) internal virtual { - tokenExternal.transfer(to, amount); + require( + tokenExternal.transfer(to, amount), + "GovernanceToken: transfer failed" + ); super._burn(from, amount); } diff --git a/contracts/GovernanceToken/GovernanceTokenSnapshot.sol b/contracts/GovernanceToken/GovernanceTokenSnapshot.sol index 10f7364..0bab57e 100644 --- a/contracts/GovernanceToken/GovernanceTokenSnapshot.sol +++ b/contracts/GovernanceToken/GovernanceTokenSnapshot.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "./IGovernanceToken.sol"; diff --git a/contracts/GovernanceToken/IGovernanceToken.sol b/contracts/GovernanceToken/IGovernanceToken.sol index b405745..e910116 100644 --- a/contracts/GovernanceToken/IGovernanceToken.sol +++ b/contracts/GovernanceToken/IGovernanceToken.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import "../extensions/ISnapshot.sol"; diff --git a/contracts/InternalMarket/InternalMarket.sol b/contracts/InternalMarket/InternalMarket.sol index 29f472a..9a5bb9e 100644 --- a/contracts/InternalMarket/InternalMarket.sol +++ b/contracts/InternalMarket/InternalMarket.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; @@ -23,13 +22,18 @@ contract InternalMarket is Initializable, HasRole, InternalMarketBase { /** * @dev Initializes the contract with the given roles and internal token. * @param roles DAORoles instance containing custom access control roles. - * @param tokenInternal_ Reference to governance token. + * @param governanceToken Reference to governance token. */ function initialize( DAORoles roles, - IGovernanceToken tokenInternal_ + IGovernanceToken governanceToken ) public initializer { - _initialize(tokenInternal_, 7 days); + require( + address(roles) != address(0) && + address(governanceToken) != address(0), + "InternalMarket: 0x0 not allowed" + ); + _initialize(governanceToken, 7 days); _setRoles(roles); } diff --git a/contracts/InternalMarket/InternalMarketBase.sol b/contracts/InternalMarket/InternalMarketBase.sol index 833639d..fec0a3a 100644 --- a/contracts/InternalMarket/InternalMarketBase.sol +++ b/contracts/InternalMarket/InternalMarketBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; diff --git a/contracts/NeokingdomToken/INeokingdomToken.sol b/contracts/NeokingdomToken/INeokingdomToken.sol index 2cbce67..aeea5a4 100644 --- a/contracts/NeokingdomToken/INeokingdomToken.sol +++ b/contracts/NeokingdomToken/INeokingdomToken.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; diff --git a/contracts/NeokingdomToken/NeokingdomToken.sol b/contracts/NeokingdomToken/NeokingdomToken.sol index f27b126..e2dd53e 100644 --- a/contracts/NeokingdomToken/NeokingdomToken.sol +++ b/contracts/NeokingdomToken/NeokingdomToken.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.9; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol"; diff --git a/contracts/RedemptionController/IRedemptionController.sol b/contracts/RedemptionController/IRedemptionController.sol index b281aad..9b23419 100644 --- a/contracts/RedemptionController/IRedemptionController.sol +++ b/contracts/RedemptionController/IRedemptionController.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; diff --git a/contracts/RedemptionController/RedemptionController.sol b/contracts/RedemptionController/RedemptionController.sol index 834c0d3..155c6cc 100644 --- a/contracts/RedemptionController/RedemptionController.sol +++ b/contracts/RedemptionController/RedemptionController.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./RedemptionControllerBase.sol"; @@ -36,6 +35,10 @@ contract RedemptionController is * @param roles The addresses of DAORoles for this contract. */ function initialize(DAORoles roles) public initializer { + require( + address(roles) != address(0), + "RedemptionController: 0x0 not allowed" + ); _setRoles(roles); _initialize(); } diff --git a/contracts/RedemptionController/RedemptionControllerBase.sol b/contracts/RedemptionController/RedemptionControllerBase.sol index 69883ea..40e7f03 100644 --- a/contracts/RedemptionController/RedemptionControllerBase.sol +++ b/contracts/RedemptionController/RedemptionControllerBase.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./IRedemptionController.sol"; diff --git a/contracts/ResolutionManager/ResolutionManager.sol b/contracts/ResolutionManager/ResolutionManager.sol index d0e2b70..0ef7e19 100644 --- a/contracts/ResolutionManager/ResolutionManager.sol +++ b/contracts/ResolutionManager/ResolutionManager.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import { Roles } from "../extensions/Roles.sol"; @@ -26,6 +26,13 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { IGovernanceToken governanceToken, IVoting voting ) public initializer { + require( + address(roles) != address(0) && + address(shareholderRegistry) != address(0) && + address(governanceToken) != address(0) && + address(voting) != address(0), + "ResolutionManager: 0x0 not allowed" + ); _setRoles(roles); _initialize(shareholderRegistry, governanceToken, voting); } @@ -167,13 +174,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { * @param resolutionId The id of the resolution to approve. */ function approveResolution(uint256 resolutionId) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can approve" - ); _approveResolution(resolutionId); } @@ -182,13 +182,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { * @param resolutionId The id of the resolution to reject. */ function rejectResolution(uint256 resolutionId) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can reject" - ); _rejectResolution(resolutionId); } @@ -209,13 +202,6 @@ contract ResolutionManager is Initializable, ResolutionManagerBase, HasRole { address[] memory executionTo, bytes[] memory executionData ) external virtual { - require( - _shareholderRegistry.isAtLeast( - _shareholderRegistry.MANAGING_BOARD_STATUS(), - _msgSender() - ), - "Resolution: only managing board can update" - ); _updateResolution( resolutionId, dataURI, diff --git a/contracts/ResolutionManager/ResolutionManagerBase.sol b/contracts/ResolutionManager/ResolutionManagerBase.sol index 4d0e873..819e139 100644 --- a/contracts/ResolutionManager/ResolutionManagerBase.sol +++ b/contracts/ResolutionManager/ResolutionManagerBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ShareholderRegistry/IShareholderRegistry.sol"; import "../GovernanceToken/IGovernanceToken.sol"; @@ -267,7 +266,7 @@ abstract contract ResolutionManagerBase { bool isNegative, address[] memory executionTo, bytes[] memory executionData - ) internal virtual onlyPending(resolutionId) { + ) internal virtual onlyPending(resolutionId) exists(resolutionId) { emit ResolutionUpdated(msg.sender, resolutionId); Resolution storage resolution = resolutions[resolutionId]; diff --git a/contracts/ShareholderRegistry/IShareholderRegistry.sol b/contracts/ShareholderRegistry/IShareholderRegistry.sol index 7243337..b9236f1 100644 --- a/contracts/ShareholderRegistry/IShareholderRegistry.sol +++ b/contracts/ShareholderRegistry/IShareholderRegistry.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/ISnapshot.sol"; diff --git a/contracts/ShareholderRegistry/ShareholderRegistry.sol b/contracts/ShareholderRegistry/ShareholderRegistry.sol index 88d9ada..42265d2 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistry.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistry.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "./ShareholderRegistrySnapshot.sol"; @@ -29,6 +28,10 @@ contract ShareholderRegistry is string memory name, string memory symbol ) public initializer { + require( + address(roles) != address(0), + "ShareholderRegistry: 0x0 not allowed" + ); _initialize(name, symbol); _setRoles(roles); } diff --git a/contracts/ShareholderRegistry/ShareholderRegistryBase.sol b/contracts/ShareholderRegistry/ShareholderRegistryBase.sol index fcf0b88..9df6737 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistryBase.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistryBase.sol @@ -1,11 +1,8 @@ // SPDX-License-Identifier: MIT - -// TODO: update _statuses when account has no shares -// TODO: check who can move shares - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; +import "@openzeppelin/contracts/utils/Address.sol"; import "../Voting/IVoting.sol"; contract ShareholderRegistryBase is ERC20Upgradeable { @@ -40,6 +37,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable { } function _setStatus(bytes32 status, address account) internal virtual { + require( + !Address.isContract(account), + "ShareholderRegistry: cannot set status for smart contract" + ); require( status == 0 || isAtLeast(SHAREHOLDER_STATUS, account), "ShareholderRegistry: address has no tokens" @@ -83,7 +84,10 @@ contract ShareholderRegistryBase is ERC20Upgradeable { ) internal view virtual returns (bool) { return balance > 0 && - // shareholder < investor < contributor < managing board + // investor < contributor < managing board + // TODO: shareholder is currently equivalent to investor. + // We need to verify with the lawyer whether we can remove it + // completely from the smart contracts. (status == INVESTOR_STATUS || status == SHAREHOLDER_STATUS || status == accountStatus || diff --git a/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol b/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol index 6940493..84ee78b 100644 --- a/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol +++ b/contracts/ShareholderRegistry/ShareholderRegistrySnapshot.sol @@ -1,7 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts v4.4.1 (token/ERC20/extensions/ERC20Snapshot.sol) - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./ShareholderRegistryBase.sol"; import "../extensions/Snapshottable.sol"; diff --git a/contracts/Voting/IVoting.sol b/contracts/Voting/IVoting.sol index b078a5a..cab918a 100644 --- a/contracts/Voting/IVoting.sol +++ b/contracts/Voting/IVoting.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/ISnapshot.sol"; diff --git a/contracts/Voting/Voting.sol b/contracts/Voting/Voting.sol index d12cd71..000df4f 100644 --- a/contracts/Voting/Voting.sol +++ b/contracts/Voting/Voting.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; @@ -18,6 +18,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole { * @param roles Instance of a DAORoles contract. */ function initialize(DAORoles roles) public initializer { + require(address(roles) != address(0), "Voting: 0x0 not allowed"); _setRoles(roles); } @@ -95,7 +96,7 @@ contract Voting is VotingSnapshot, Initializable, HasRole { /** * @notice Hook called on every governance token transfer. - * @dev Only the governance token can call this method. + * @dev Called by GovernanceToken and ShareholderRegistry. * @param from The sender's address. * @param to The receiver's address. * @param amount The amount transferred. diff --git a/contracts/Voting/VotingBase.sol b/contracts/Voting/VotingBase.sol index 2cb6667..fc37a67 100644 --- a/contracts/Voting/VotingBase.sol +++ b/contracts/Voting/VotingBase.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; import "../ShareholderRegistry/IShareholderRegistry.sol"; diff --git a/contracts/Voting/VotingSnapshot.sol b/contracts/Voting/VotingSnapshot.sol index 2b6b407..35c3ac1 100644 --- a/contracts/Voting/VotingSnapshot.sol +++ b/contracts/Voting/VotingSnapshot.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "../extensions/Snapshottable.sol"; diff --git a/contracts/extensions/DAORoles.sol b/contracts/extensions/DAORoles.sol index bd8eece..7d2d9c8 100644 --- a/contracts/extensions/DAORoles.sol +++ b/contracts/extensions/DAORoles.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.16; import "@openzeppelin/contracts/access/AccessControl.sol"; diff --git a/contracts/extensions/HasRole.sol b/contracts/extensions/HasRole.sol index 161fc65..2c760d6 100644 --- a/contracts/extensions/HasRole.sol +++ b/contracts/extensions/HasRole.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol"; import "@openzeppelin/contracts/utils/Strings.sol"; diff --git a/contracts/extensions/ISnapshot.sol b/contracts/extensions/ISnapshot.sol index c98c272..0b639ad 100644 --- a/contracts/extensions/ISnapshot.sol +++ b/contracts/extensions/ISnapshot.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; interface ISnapshot { function snapshot() external returns (uint256); diff --git a/contracts/extensions/Roles.sol b/contracts/extensions/Roles.sol index 257c19e..5ca5b28 100644 --- a/contracts/extensions/Roles.sol +++ b/contracts/extensions/Roles.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; library Roles { bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE"); diff --git a/contracts/extensions/Snapshottable.sol b/contracts/extensions/Snapshottable.sol index 96eca8f..3bd7017 100644 --- a/contracts/extensions/Snapshottable.sol +++ b/contracts/extensions/Snapshottable.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; diff --git a/contracts/mocks/ERC20Mock.sol b/contracts/mocks/ERC20Mock.sol index bf5cd38..61e1b11 100644 --- a/contracts/mocks/ERC20Mock.sol +++ b/contracts/mocks/ERC20Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/HasRoleMock.sol b/contracts/mocks/HasRoleMock.sol index 6e5bdf7..65912c8 100644 --- a/contracts/mocks/HasRoleMock.sol +++ b/contracts/mocks/HasRoleMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../extensions/HasRole.sol"; diff --git a/contracts/mocks/NeokingdomTokenMock.sol b/contracts/mocks/NeokingdomTokenMock.sol index 8209f4b..71ffc0f 100644 --- a/contracts/mocks/NeokingdomTokenMock.sol +++ b/contracts/mocks/NeokingdomTokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract GovernanceTokenMock { mapping(address => uint256) mockResult_balanceOfAt; diff --git a/contracts/mocks/NeokingdomTokenV2Mock.sol b/contracts/mocks/NeokingdomTokenV2Mock.sol index 07e1d30..b65d5f3 100644 --- a/contracts/mocks/NeokingdomTokenV2Mock.sol +++ b/contracts/mocks/NeokingdomTokenV2Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../GovernanceToken/GovernanceToken.sol"; import "../extensions/Roles.sol"; diff --git a/contracts/mocks/NewTelediskoTokenMock.sol b/contracts/mocks/NewTelediskoTokenMock.sol index 2a7aa38..fe4d3cb 100644 --- a/contracts/mocks/NewTelediskoTokenMock.sol +++ b/contracts/mocks/NewTelediskoTokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/utils/Arrays.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/ResolutionExecutorMock.sol b/contracts/mocks/ResolutionExecutorMock.sol index 6f97a8e..a9dabb8 100644 --- a/contracts/mocks/ResolutionExecutorMock.sol +++ b/contracts/mocks/ResolutionExecutorMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract ResolutionExecutorMock { event MockExecutionSimple(uint256 a); diff --git a/contracts/mocks/ResolutionManagerV2Mock.sol b/contracts/mocks/ResolutionManagerV2Mock.sol index 4e0c5c2..4f60444 100644 --- a/contracts/mocks/ResolutionManagerV2Mock.sol +++ b/contracts/mocks/ResolutionManagerV2Mock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ResolutionManager/ResolutionManager.sol"; import "../extensions/Roles.sol"; diff --git a/contracts/mocks/ShareholderRegistryMock.sol b/contracts/mocks/ShareholderRegistryMock.sol index 17c1609..015eee0 100644 --- a/contracts/mocks/ShareholderRegistryMock.sol +++ b/contracts/mocks/ShareholderRegistryMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../ShareholderRegistry/IShareholderRegistry.sol"; import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; diff --git a/contracts/mocks/TokenMock.sol b/contracts/mocks/TokenMock.sol index c97b39b..c6f0b2c 100644 --- a/contracts/mocks/TokenMock.sol +++ b/contracts/mocks/TokenMock.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; diff --git a/contracts/mocks/VotingMock.sol b/contracts/mocks/VotingMock.sol index 0cef53c..8634780 100644 --- a/contracts/mocks/VotingMock.sol +++ b/contracts/mocks/VotingMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; contract VotingMock { event AfterTokenTransferCalled(address from, address to, uint256 amount); diff --git a/echidna/proxies/InternalMarketProxy.sol b/echidna/proxies/InternalMarketProxy.sol index 361d61a..cf31924 100644 --- a/echidna/proxies/InternalMarketProxy.sol +++ b/echidna/proxies/InternalMarketProxy.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "./TokenMock.sol"; import "../../contracts/InternalMarket/InternalMarket.sol"; diff --git a/echidna/proxies/RedemptionControllerProxy.sol b/echidna/proxies/RedemptionControllerProxy.sol index 2f1c9d9..213e626 100644 --- a/echidna/proxies/RedemptionControllerProxy.sol +++ b/echidna/proxies/RedemptionControllerProxy.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "../../contracts/RedemptionController/RedemptionController.sol"; diff --git a/echidna/proxies/TokenMock.sol b/echidna/proxies/TokenMock.sol index 0ddea86..33cdbb4 100644 --- a/echidna/proxies/TokenMock.sol +++ b/echidna/proxies/TokenMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.16; +pragma solidity 0.8.16; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; diff --git a/hardhat.config.ts b/hardhat.config.ts index ddcb214..88acca2 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -44,7 +44,7 @@ const config: HardhatUserConfig = { solidity: { compilers: [ { - version: "0.8.19", + version: "0.8.16", settings: { optimizer: { enabled: true, diff --git a/test/GovernanceToken.ts b/test/GovernanceToken.ts index 6e79026..52b39fe 100644 --- a/test/GovernanceToken.ts +++ b/test/GovernanceToken.ts @@ -94,6 +94,8 @@ describe("GovernanceToken", () => { shareholderRegistry.isAtLeast .whenCalledWith(contributorStatus, contributor2.address) .returns(true); + neokingdomToken.transfer.returns(true); + neokingdomToken.transferFrom.returns(true); }); afterEach(async () => { @@ -101,6 +103,8 @@ describe("GovernanceToken", () => { redemption.afterMint.reset(); daoRoles.hasRole.reset(); shareholderRegistry.isAtLeast.reset(); + neokingdomToken.transfer.reset(); + neokingdomToken.transferFrom.reset(); neokingdomToken.mint.reset(); }); @@ -253,6 +257,19 @@ describe("GovernanceToken", () => { ); }); + it("should fail when the transfer fails", async () => { + neokingdomToken.transferFrom.returns(false); + await expect(governanceToken.wrap(contributor.address, 1)).revertedWith( + "GovernanceToken: transfer failed" + ); + }); + + it("should fail wrapping 0 tokens", async () => { + await expect( + governanceToken.connect(contributor).wrap(contributor.address, 0) + ).revertedWith("GovernanceToken: attempt to wrap 0 tokens"); + }); + it("should transfer external token to itself", async () => { await governanceToken.wrap(contributor.address, 41); expect(neokingdomToken.transferFrom).calledWith( @@ -420,6 +437,22 @@ describe("GovernanceToken", () => { expect(balanceAfter).equal(balanceBefore.add(42)); }); + it("should not mint an equivalent amount of neok tokens to the governance contract", async () => { + neokingdomToken.mint.reset(); + await governanceToken.wrap(contributor.address, 42); + await timeTravel(7); + await governanceToken.settleTokens(contributor.address); + + expect(neokingdomToken.mint).to.not.have.been.called; + }); + + it("should not call RedemptionController.afterMint", async () => { + await governanceToken.wrap(contributor.address, 42); + await timeTravel(7); + await governanceToken.settleTokens(contributor.address); + expect(redemption.afterMint).not.called; + }); + it("should not mint an equivalent amount of neok tokens to the governance contract", async () => { await governanceToken.wrap(contributor.address, 42); await timeTravel(7); @@ -450,6 +483,13 @@ describe("GovernanceToken", () => { ).revertedWith("ERC20: burn amount exceeds balance"); }); + it("should fail when the transfer fails", async () => { + neokingdomToken.transfer.returns(false); + await expect( + governanceToken.unwrap(contributor.address, contributor.address, 1) + ).revertedWith("GovernanceToken: transfer failed"); + }); + it("should transfer external token to 'to' address", async () => { await governanceToken.mint(contributor.address, 41); await governanceToken.unwrap( diff --git a/test/GovernanceTokenSnapshot.ts b/test/GovernanceTokenSnapshot.ts index 5216609..65ab744 100644 --- a/test/GovernanceTokenSnapshot.ts +++ b/test/GovernanceTokenSnapshot.ts @@ -115,11 +115,15 @@ describe("GovernanceTokenSnapshot", () => { beforeEach(async () => { snapshotId = await network.provider.send("evm_snapshot"); daoRoles.hasRole.returns(true); + neokingdomToken.transfer.returns(true); + neokingdomToken.transferFrom.returns(true); }); afterEach(async () => { await network.provider.send("evm_revert", [snapshotId]); daoRoles.hasRole.reset(); + neokingdomToken.transfer.reset(); + neokingdomToken.transferFrom.reset(); }); describe("snapshot logic", async () => { diff --git a/test/Integration.ts b/test/Integration.ts index 659dede..2960394 100644 --- a/test/Integration.ts +++ b/test/Integration.ts @@ -42,6 +42,7 @@ const INITIAL_USDC = 1000; describe("Integration", async () => { let snapshotId: string; let offerDurationDays: number; + let settlementPeriod = 7; let redemptionStartDays: number; let redemptionWindowDays: number; let redemptionMaxDaysInThePast: number; @@ -1130,6 +1131,82 @@ describe("Integration", async () => { ); }); + describe("least authority audit proof of fix (july 2023)", async () => { + it("issue A: Deposited Tokens Can Be Redeemed", async () => { + await _makeContributor(user1, 10); + await _makeContributor(user2, 10); + + // user2 offers 10 tokens + await internalMarket.connect(user2).makeOffer(e(10)); + await timeTravel(offerDurationDays, true); + + // user2 transfers 10 tokens to user1 + await internalMarket.connect(user2).withdraw(user1.address, e(10)); + + // user1 deposit their tokens + await internalMarket.connect(user1).deposit(e(10)); + await timeTravel(settlementPeriod); + await governanceToken.settleTokens(user1.address); + + // user2 offer all tokens, hoping to redeem all of them... + await internalMarket.connect(user1).makeOffer(e(20)); + await timeTravel(redemptionStartDays, true); + await expect(internalMarket.connect(user1).redeem(e(20))).revertedWith( + "Redemption controller: amount exceeds redeemable balance" + ); + + // ...but they can only redeem those that were minted directly to them + await internalMarket.connect(user1).redeem(e(10)); + expect(await tokenMock.balanceOf(user1.address)).equal( + e(INITIAL_USDC + 10) + ); + }); + + it("Issue B: Unsettled Deposits Can Be Locked", async () => { + await _makeContributor(user1, 10); + await _makeContributor(user2, 0); + + // user1 offers token, no one buys + await internalMarket.connect(user1).makeOffer(e(10)); + await timeTravel(offerDurationDays, true); + + // tokens are transferred to user 2 + await internalMarket.connect(user1).withdraw(user2.address, e(10)); + + // user 2 makes 2 deposits, one of which with 0 tokens, which + // could lock the previous deposit forever + await internalMarket.connect(user2).deposit(e(9)); + await expect(internalMarket.connect(user2).deposit(e(0))).revertedWith( + "GovernanceToken: attempt to wrap 0 tokens" + ); + + await timeTravel(settlementPeriod, true); + await governanceToken.settleTokens(user2.address); + + expect(await governanceToken.balanceOf(user2.address)).equal(e(9)); + }); + + it("Issue C: Missing Modifier Preventing the Update of Non-Existent Resolutions", async () => { + const nonExistingResolutionId = 999; + await expect( + resolutionManager + .connect(managingBoard) + .updateResolution(nonExistingResolutionId, "", 0, false, [], []) + ).revertedWith("Resolution: does not exist"); + }); + + it("issue D: the status of internalMarket or shareholderRegistry can be set to contributor status", async () => { + await expect( + shareholderRegistry.setStatus( + contributorStatus, + internalMarket.address + ) + ).revertedWith( + "ShareholderRegistry: cannot set status for smart contract" + ); + }); + }); + it("redemption edge cases", async () => { await _makeContributor(user1, 10); await _makeContributor(user2, 0); diff --git a/test/ResolutionManager.ts b/test/ResolutionManager.ts index 8ecf6ee..58a08be 100644 --- a/test/ResolutionManager.ts +++ b/test/ResolutionManager.ts @@ -306,6 +306,14 @@ describe("Resolution", async () => { ).revertedWith("Resolution: already rejected"); }); + it("doesn't allow the managing board to update a non-existing resolution", async () => { + await expect( + resolution + .connect(managingBoard) + .updateResolution(42, "updated test", 6, true, [], []) + ).revertedWith("Resolution: does not exist"); + }); + it("doesn't allow anyone else to update a resolution", async () => { shareholderRegistry.isAtLeast .whenCalledWith(managingBoardStatus, user1.address) diff --git a/test/ShareholderRegistrySnapshot.ts b/test/ShareholderRegistrySnapshot.ts index df4a749..3c16a89 100644 --- a/test/ShareholderRegistrySnapshot.ts +++ b/test/ShareholderRegistrySnapshot.ts @@ -104,6 +104,14 @@ describe("Shareholder Registry", () => { ).revertedWith("ShareholderRegistry: address has no tokens"); }); + it("should fail if address is a contract", async () => { + await expect( + registry.setStatus(CONTRIBUTOR_STATUS, registry.address) + ).revertedWith( + "ShareholderRegistry: cannot set status for smart contract" + ); + }); + it("should be callable only by a resolution", async () => { await expect( registry.connect(alice).setStatus(CONTRIBUTOR_STATUS, alice.address)