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

fix: audit 12.09 #13

Merged
merged 21 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
0020e3b
ref: move vault-related contracts & tests to test/vault
nksazonov Sep 12, 2024
34af09c
fix(LiteVault): use call instead of transfer when withdrawing ETH
nksazonov Sep 12, 2024
379f996
fix(LiteVault): pass authorizer in constructor, zero address checks f…
nksazonov Sep 12, 2024
dd3f1b4
fix(BrokerToken): add zero address check for beneficiary in constructor
nksazonov Sep 12, 2024
10172c3
feat: add IAuthorizable interface, emit AuthorizerChanged event
nksazonov Sep 12, 2024
069750e
fix(LiteVault): checks-effects-interactions pattern
nksazonov Sep 12, 2024
6b02a67
nit(LiteVault): remove override modifiers
nksazonov Sep 12, 2024
ba2fe32
ref: make `balanceOf` and `deposit` external`
nksazonov Sep 12, 2024
39ac5cc
fix: pin submodules to a tag
nksazonov Sep 12, 2024
c0b4e2c
chore(LiteVault): use named parameters in mapping
nksazonov Sep 12, 2024
596dc6a
chore(TimeRangeAuthorizer): make time vars immutable
nksazonov Sep 12, 2024
4bb9138
feat: add Ownable2Step instead of Ownable
nksazonov Sep 12, 2024
0e927dc
conf: specify evm version to be cancun
nksazonov Sep 19, 2024
5523486
feat: withdrawal grace period after authorizer has changed
nksazonov Sep 19, 2024
bf6d895
chore: lock solidity version to 0.8.26
nksazonov Sep 19, 2024
221c1dd
test: comply to style guide
nksazonov Sep 20, 2024
68447b0
test: introduce named imports to compy with style guide
nksazonov Sep 20, 2024
3ae51e0
docs: add devdocs for _isWithdrawalGracePeriodActive
nksazonov Sep 20, 2024
b4f68b6
ref: unlock solidity version for interfaces
nksazonov Sep 20, 2024
47fae05
style: pin solidity version to 0.8.26 for BrokerToken
nksazonov Sep 23, 2024
9114873
style: unpin solidity version for tests
nksazonov Sep 23, 2024
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: 2 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
tag = v1.9.2
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
tag = v5.0.2
3 changes: 2 additions & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ src = 'src'
out = 'out'
libs = ['lib']
test = 'test'
solc = "0.8.24"
solc = "0.8.26"
evm_version = "cancun"
optimizer = true
optimizer_runs = 1000000
gas_price = 1000000000
Expand Down
2 changes: 1 addition & 1 deletion lib/forge-std
2 changes: 1 addition & 1 deletion lib/openzeppelin-contracts
9 changes: 9 additions & 0 deletions src/BrokerToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol";
contract BrokerToken is ERC20Permit {
uint8 private immutable _decimals;

/**
* @notice Error thrown when the address supplied with the function call is invalid.
*/
error InvalidAddress();

/**
* @dev Simple constructor, passing arguments to ERC20Permit and ERC20 constructors.
* Mints the supply to the deployer.
Expand All @@ -22,6 +27,10 @@ contract BrokerToken is ERC20Permit {
ERC20Permit(name)
ERC20(name, symbol)
{
if (beneficiary == address(0)) {
revert InvalidAddress();
}

_decimals = decimals_;
_mint(beneficiary, supply);
}
Expand Down
22 changes: 22 additions & 0 deletions src/interfaces/IAuthorizable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import {IAuthorize} from "./IAuthorize.sol";

/**
* @title IAuthorizable
* @notice Interface for a contract that is using Authorize logic.
*/
interface IAuthorizable {
/**
* @notice Emitted when the authorizer contract is changed.
* @param newAuthorizer The address of the new authorizer contract.
*/
event AuthorizerChanged(IAuthorize indexed newAuthorizer);

/**
* @dev Sets the authorizer contract.
* @param newAuthorizer The address of the authorizer contract.
*/
function setAuthorizer(IAuthorize newAuthorizer) external;
}
2 changes: 1 addition & 1 deletion src/interfaces/IAuthorize.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
pragma solidity 0.8.26;

/**
* @title IAuthorize
Expand Down
12 changes: 11 additions & 1 deletion src/interfaces/IVault.sol
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
pragma solidity 0.8.26;
nksazonov marked this conversation as resolved.
Show resolved Hide resolved

/**
* @title IVault
* @notice Interface for a vault contract that allows users to deposit, withdraw, and check balances of tokens and ETH.
*/
interface IVault {
/**
* @notice Error thrown when the address supplied with the function call is invalid.
*/
error InvalidAddress();

/**
* @notice Emitted when a user deposits tokens or ETH into the vault.
* @param user The address of the user that deposited the tokens.
Expand Down Expand Up @@ -35,6 +40,11 @@ interface IVault {
*/
error InsufficientBalance(address token, uint256 required, uint256 available);

/**
* @notice Error thrown when the transfer of Eth fails.
*/
error NativeTransferFailed();

/**
* @dev Returns the balance of a specified token for a user.
* @param user The address of the user.
Expand Down
54 changes: 42 additions & 12 deletions src/vault/LiteVault.sol
Original file line number Diff line number Diff line change
@@ -1,40 +1,48 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
pragma solidity 0.8.26;

import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/access/Ownable2Step.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

import "../interfaces/IAuthorize.sol";
import "../interfaces/IAuthorizable.sol";
import "../interfaces/IVault.sol";

/**
* @title LiteVault
* @notice A simple vault that allows users to deposit and withdraw tokens.
*/
contract LiteVault is IVault, ReentrancyGuard, Ownable {
contract LiteVault is IVault, IAuthorizable, ReentrancyGuard, Ownable2Step {
/// @dev Using SafeERC20 to support non fully ERC20-compliant tokens,
/// that may not return a boolean value on success.
using SafeERC20 for IERC20;

// Mapping from user to token to balances
mapping(address => mapping(address => uint256)) internal _balances;
uint64 public constant WITHDRAWAL_GRACE_PERIOD = 3 days;

mapping(address user => mapping(address token => uint256 balance)) internal _balances;

IAuthorize public authorizer;
uint64 public latestSetAuthorizerTimestamp;

/**
* @dev Constructor sets the initial owner of the contract.
*/
constructor(address owner) Ownable(owner) {}
constructor(address owner, IAuthorize authorizer_) Ownable(owner) {
if (address(authorizer_) == address(0)) {
revert InvalidAddress();
}
authorizer = authorizer_;
}

/**
* @dev Returns the balance of the specified token for a user.
* @param user The address of the user.
* @param token The address of the token. Use address(0) for ETH.
* @return The balance of the specified token for the user.
*/
function balanceOf(address user, address token) public view override returns (uint256) {
function balanceOf(address user, address token) external view returns (uint256) {
return _balances[user][token];
}

Expand All @@ -57,22 +65,28 @@ contract LiteVault is IVault, ReentrancyGuard, Ownable {
* @param newAuthorizer The address of the authorizer contract.
*/
function setAuthorizer(IAuthorize newAuthorizer) external onlyOwner {
if (address(newAuthorizer) == address(0)) {
revert InvalidAddress();
}

authorizer = newAuthorizer;
latestSetAuthorizerTimestamp = uint64(block.timestamp);
emit AuthorizerChanged(newAuthorizer);
}

/**
* @dev Deposits tokens or ETH into the vault.
* @param token The address of the token to deposit. Use address(0) for ETH.
* @param amount The amount of tokens or ETH to deposit.
*/
function deposit(address token, uint256 amount) public payable override {
function deposit(address token, uint256 amount) external payable nonReentrant {
if (token == address(0)) {
if (msg.value != amount) revert IncorrectValue();
_balances[msg.sender][address(0)] += amount;
} else {
if (msg.value != 0) revert IncorrectValue();
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
_balances[msg.sender][token] += amount;
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
}

emit Deposited(msg.sender, token, amount);
Expand All @@ -83,23 +97,39 @@ contract LiteVault is IVault, ReentrancyGuard, Ownable {
* @param token The address of the token to withdraw. Use address(0) for ETH.
* @param amount The amount of tokens or ETH to withdraw.
*/
function withdraw(address token, uint256 amount) external override nonReentrant {
function withdraw(address token, uint256 amount) external nonReentrant {
uint256 currentBalance = _balances[msg.sender][token];
if (currentBalance < amount) {
revert InsufficientBalance(token, amount, currentBalance);
}
if (!authorizer.authorize(msg.sender, token, amount)) {
if (
!_isWithdrawalGracePeriodActive(
latestSetAuthorizerTimestamp, uint64(block.timestamp), WITHDRAWAL_GRACE_PERIOD
) && !authorizer.authorize(msg.sender, token, amount)
) {
revert IAuthorize.Unauthorized(msg.sender, token, amount);
}

_balances[msg.sender][token] -= amount;

if (token == address(0)) {
payable(msg.sender).transfer(amount);
/// @dev using `call` instead of `transfer` to overcome 2300 gas ceiling that could make it revert with some AA wallets
(bool success,) = msg.sender.call{value: amount}("");
if (!success) {
revert NativeTransferFailed();
}
} else {
IERC20(token).safeTransfer(msg.sender, amount);
}

emit Withdrawn(msg.sender, token, amount);
}

function _isWithdrawalGracePeriodActive(uint64 latestSetAuthorizerTimestamp_, uint64 now_, uint64 gracePeriod)
internal
pure
returns (bool)
{
return latestSetAuthorizerTimestamp_ + gracePeriod > now_;
}
}
6 changes: 3 additions & 3 deletions src/vault/TimeRangeAuthorizer.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
pragma solidity 0.8.26;

import "../interfaces/IAuthorize.sol";

Expand All @@ -15,8 +15,8 @@ contract TimeRangeAuthorizer is IAuthorize {
*/
error InvalidTimeRange(uint256 startTimestamp, uint256 endTimestamp);

uint256 public startTimestamp;
uint256 public endTimestamp;
uint256 public immutable startTimestamp;
uint256 public immutable endTimestamp;

/**
* @dev Constructor sets the forbidden time range.
Expand Down
21 changes: 21 additions & 0 deletions src/vault/test/TestLiteVault.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

import {IAuthorize} from "../../interfaces/IAuthorize.sol";
import {LiteVault} from "../LiteVault.sol";

contract TestLiteVault is LiteVault {
constructor(address owner, IAuthorize authorizer_) LiteVault(owner, authorizer_) {}

function workaround_setBalance(address user, address token, uint256 amount) external {
_balances[user][token] = amount;
}

function exposed_isWithdrawalGracePeriodActive(
uint64 latestSetAuthorizerTimestamp_,
uint64 now_,
uint64 gracePeriod
) external pure returns (bool) {
return _isWithdrawalGracePeriodActive(latestSetAuthorizerTimestamp_, now_, gracePeriod);
}
}
11 changes: 8 additions & 3 deletions test/BrokerToken.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,21 @@ contract BrokerTokenTest is Test {
token = new BrokerToken("Canary", "CANARY", DECIMALS, TOKEN_SUPPLY, beneficiary);
}

function test_nameAndSymbol() public view {
function test_constructor_revert_ifBeneficiaryIsZero() public {
vm.expectRevert(abi.encodeWithSelector(BrokerToken.InvalidAddress.selector));
new BrokerToken("Canary", "CANARY", DECIMALS, TOKEN_SUPPLY, address(0));
}

function tes_constructort_nameAndSymbol() public view {
assertEq(token.name(), "Canary");
assertEq(token.symbol(), "CANARY");
}

function test_decimals() public view {
function test_constructor_decimals() public view {
assertEq(token.decimals(), DECIMALS);
}

function test_supplyMintedToBeneficiary() public view {
function test_constructor_supplyMintedToBeneficiary() public view {
assertEq(token.balanceOf(beneficiary), TOKEN_SUPPLY);
}
}
16 changes: 16 additions & 0 deletions test/vault/CostlyReceiver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;

contract CostlyReceiver {
mapping(uint256 seed => uint256 value) public costlyValues;

receive() external payable {
/// @dev This is a costly operation that will consume a lot of gas.
costlyValues[block.timestamp] = 1;
}

fallback() external payable {
/// @dev This is a costly operation that will consume a lot of gas.
costlyValues[block.timestamp] = 1;
}
}
Loading