Skip to content

Commit

Permalink
feat(test): refactored test for rateLimits
Browse files Browse the repository at this point in the history
  • Loading branch information
KristenPire committed Jul 1, 2024
1 parent b2b225c commit 36bfc41
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 76 deletions.
2 changes: 1 addition & 1 deletion script/configureToken.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract All is Script {
token.addAdminAccount(admin);
console.log("Admin account added successfully.");

token.addMinterAndBurner(system, allowance);
token.setLimitCap( allowance);

token.addAdminAccount(devKey);
token.setLimits(system, allowance, 0);
Expand Down
8 changes: 8 additions & 0 deletions script/deployAll.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ elif [ "$1" == "--gnosis-chiado" ]; then
echo $VERIFIER_URL
forge script script/deploy.s.sol:AllControllerGnosis --rpc-url $RPC_URL --broadcast --verify $VERIFIER_URL -vvvv --legacy
exit 0
elif [ "$1" == "--gnosis-mainnet" ]; then
echo "Deploying to Gnosis Mainnet..."
RPC_URL=$GNOSIS_MAINNET_RPC
ETHERSCAN_API_KEY=$GNOSISSCAN_API
VERIFIER_URL="--verifier-url $GNOSISSCAN_URL --chain-id $GNOSIS_MAINNET_CHAIN_ID"
echo $VERIFIER_URL
forge script script/deploy.s.sol:All --rpc-url $RPC_URL --broadcast --etherscan-api-key $ETHERSCAN_API_KEY --verify $VERIFIER_URL -vvvv --legacy
exit 0
elif [ "$1" == "--polygon-amoy" ]; then
echo "Deploying to Polygon Amoy..."
RPC_URL=$POLYGON_AMOY_RPC
Expand Down
4 changes: 3 additions & 1 deletion src/ControllerToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ contract ControllerToken is Token {
address to,
uint256 amount
) external onlyFrontend onlySystemAccount(caller) returns (bool) {
_useMinterLimits(_msgSender(), amount);
if (RateLimitsUpgradeable.mintingCurrentLimitOf(caller) < amount)
revert IXERC20_NotHighEnoughLimits(); // This is a double verification. Since there is the same condition but with a different eventType I don't know where I should verify.
_useMinterLimits(caller, amount);
_mint(to, amount);

return true;
Expand Down
56 changes: 24 additions & 32 deletions src/RateLimitsUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ contract RateLimitsUpgradeable {
uint256 ratePerSecond;
uint256 maxLimit;
uint256 currentLimit;
uint256 limitCap; // Should we cap this ? it is so that an admin can set a current higher than the daily but only up to a certain amount.
}

/**
Expand All @@ -31,6 +30,7 @@ contract RateLimitsUpgradeable {

struct RateLimitsStorage {
mapping(address => Limits) _limits;
uint256 limitCap; // Should we cap this ? it is so that an admin can set a current higher than the daily but only up to a certain amount.
}

/**
Expand Down Expand Up @@ -63,29 +63,17 @@ contract RateLimitsUpgradeable {
}

/**
* @notice set the maxlimit of a minter
* @param account The address of the minter who is being changed
* @param newMax The new limitCap
* @notice set the limitCap
* @param newCap The new limitCap
*/
function _setMinterMaxLimit(address account, uint256 newMax) internal {
RateLimitParameters storage m = _getRateLimitsStorage()
._limits[account]
.mint;
function _setLimitCap(uint256 newCap) internal {
RateLimitsStorage storage $ = _getRateLimitsStorage();

m.limitCap = newMax;
$.limitCap = newCap;
}

/**
* @notice set the maxlimit of a burner
* @param account The address of the burner who is being changed
* @param newMax The new limitCap
*/
function _setBurnerMaxLimit(address account, uint256 newMax) internal {
RateLimitParameters storage b = _getRateLimitsStorage()
._limits[account]
.burn;

b.limitCap = newMax;
function _getLimitCap() internal view returns(uint256 limitCap) {
limitCap = _getRateLimitsStorage().limitCap;
}

/**
Expand Down Expand Up @@ -121,32 +109,34 @@ contract RateLimitsUpgradeable {
}

/**
* @notice Resets the minting limit for a given account to a specified value
* @notice sets the minting limit for a given account to a specified value
* @param account The address of the minter whose limit is to be reset
* @param limit The new current limit to be set for minting
*/
function _resetMinterCurrentLimit(address account, uint256 limit) internal {
RateLimitParameters storage m = _getRateLimitsStorage()
function _setMinterCurrentLimit(address account, uint256 limit) internal {
RateLimitsStorage storage $ = _getRateLimitsStorage();
RateLimitParameters storage m = $
._limits[account]
.mint;

if (limit > m.limitCap) revert RateLimit_LimitsTooHigh();
if (limit > $.limitCap) revert RateLimit_LimitsTooHigh();

m.currentLimit = limit;
m.timestamp = block.timestamp;
}

/**
* @notice Resets the burning limit for a given account to a specified value
* @notice sets the burning limit for a given account to a specified value
* @param account The address of the burner whose limit is to be reset
* @param limit The new current limit to be set for burning
*/
function _resetBurnerCurrentLimit(address account, uint256 limit) internal {
RateLimitParameters storage b = _getRateLimitsStorage()
function _setBurnerCurrentLimit(address account, uint256 limit) internal {
RateLimitsStorage storage $ = _getRateLimitsStorage();
RateLimitParameters storage b = $
._limits[account]
.burn;

if (limit > b.limitCap) revert RateLimit_LimitsTooHigh();
if (limit > $.limitCap) revert RateLimit_LimitsTooHigh();

b.currentLimit = limit;
b.timestamp = block.timestamp;
Expand All @@ -158,11 +148,12 @@ contract RateLimitsUpgradeable {
* @param newDailyLimit The updated limit we are setting to the minter
*/
function _changeMinterLimit(address account, uint256 newDailyLimit) internal {
RateLimitParameters storage m = _getRateLimitsStorage()
RateLimitsStorage storage $ = _getRateLimitsStorage();
RateLimitParameters storage m = $
._limits[account]
.mint;

if (newDailyLimit > m.limitCap) revert RateLimit_LimitsTooHigh();
if (newDailyLimit > $.limitCap) revert RateLimit_LimitsTooHigh();

uint256 oldLimit = m.maxLimit;
uint256 currentLimit = mintingCurrentLimitOf(account);
Expand All @@ -187,11 +178,12 @@ contract RateLimitsUpgradeable {
address account,
uint256 newDailyLimit
) internal {
RateLimitParameters storage b = _getRateLimitsStorage()
RateLimitsStorage storage $ = _getRateLimitsStorage();
RateLimitParameters storage b = $
._limits[account]
.burn;

if (newDailyLimit > b.limitCap) revert RateLimit_LimitsTooHigh();
if (newDailyLimit > $.limitCap) revert RateLimit_LimitsTooHigh();

uint256 oldLimit = b.maxLimit;
uint256 currentLimit = burningCurrentLimitOf(account);
Expand Down
64 changes: 50 additions & 14 deletions src/Token.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ contract Token is
string memory name,
string memory symbol,
address _validator
) public virtual initializer { // Those line replaces the inheritance call in the constructor, as we are using the upgradeable pattern.
) public virtual initializer {
// Those line replaces the inheritance call in the constructor, as we are using the upgradeable pattern.
// In the upgradeable pattern, the 'initialize' function replaces the constructor. It's not called automatically.
// The proxy contract calls this function using delegatecall.
// This results in storing all new variables from ERC20, Ownable, etc., in the proxy's storage.
Expand All @@ -72,6 +73,8 @@ contract Token is
) internal override onlyOwner {}

function mint(address to, uint256 amount) public onlySystemAccounts {
if (RateLimitsUpgradeable.mintingCurrentLimitOf(_msgSender()) < amount)
revert IXERC20_NotHighEnoughLimits(); // This is a double verification. Since there is the same condition but with a different eventType I don't know where I should verify.
_useMinterLimits(_msgSender(), amount);
_mint(to, amount);
}
Expand Down Expand Up @@ -149,6 +152,41 @@ contract Token is
return super.transferFrom(from, to, amount);
}

/**
* @notice Sets the maximum an admin can set the minting limit to
* @param limitCap the maximum value a minting limit can be set to
*/
function setLimitCap(uint256 limitCap) public onlyOwner {
_setLimitCap(limitCap);
}

function getLimitCap() public view returns(uint256){
return _getLimitCap();
}
/*
* @notice Updates the daily limit of a minter
* @param minter The address of the minter we are setting the limit too
* @param limit The updated limit we are setting to the minter
*/
function setMintingLimit(
address minter,
uint256 limit
) public onlyAdminAccounts {
_changeMinterLimit(minter, limit);
}

/**
* @notice sets the minting limit for a given account to a specified value
* @param minter The address of the minter whose limit is to be reset
* @param limit The new current limit to be set for minting
*/
function setMintingCurrentLimit(
address minter,
uint256 limit
) public onlyAdminAccounts {
_setMinterCurrentLimit(minter, limit);
}

/**
* @notice Returns the max limit of a minter
*
Expand Down Expand Up @@ -245,15 +283,8 @@ contract Token is
)
);
}


function addMinterAndBurner(address account, uint256 limitCap) external onlyOwner {
addSystemAccount(account);
_setMinterMaxLimit(account, limitCap);
_setBurnerMaxLimit(account, limitCap);
}

// IXERC20

function setLockbox(address) external pure {
revert("Not Implemented");
}
Expand All @@ -270,8 +301,11 @@ contract Token is
uint256 mintingLimit,
uint256 burningLimit
) external onlyAdminAccounts {
if (_getLimitCap() < mintingLimit) revert IXERC20_LimitsTooHigh();
_changeMinterLimit(bridge, mintingLimit);
_changeBurnerLimit(bridge, burningLimit);

emit BridgeLimitsSet(mintingLimit, burningLimit, bridge);
}

/**
Expand All @@ -281,12 +315,14 @@ contract Token is
* @param amount The amount of tokens being burned
*/
function burn(address user, uint256 amount) external onlySystemAccounts {
if (_msgSender() != user){
_spendAllowance(user, _msgSender(), amount);
}
if (RateLimitsUpgradeable.burningCurrentLimitOf(_msgSender()) < amount)
revert IXERC20_NotHighEnoughLimits(); // This is a double verification. Since there is the same condition but with a different eventType I don't know where I should verify.
if (_msgSender() != user) {
_spendAllowance(user, _msgSender(), amount);
}

_useBurnerLimits(_msgSender(), amount);
_burn(user, amount);
_useBurnerLimits(_msgSender(), amount);
_burn(user, amount);
}

}
Expand Down
2 changes: 1 addition & 1 deletion test/Blacklist.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract BlackListValidatorTest is Test {
token.addAdminAccount(admin);
assertTrue(token.isSystemAccount(system));
assertTrue(token.isAdminAccount(admin));
token.addMinterAndBurner(system, 2e18);
token.setLimitCap( 2e18);
vm.startPrank(admin);
token.setLimits(system, 2e18, 2e18);
vm.stopPrank();
Expand Down
2 changes: 1 addition & 1 deletion test/ControllerToken.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ contract ControllerTokenTest is Test {
token.addAdminAccount(admin);
assertTrue(token.isSystemAccount(system));
assertTrue(token.isAdminAccount(admin));
token.addMinterAndBurner(system, 3e18);
token.setLimitCap(3e18);
vm.startPrank(admin);
token.setLimits(system, 3e18, 3e18);
vm.stopPrank();
Expand Down
2 changes: 1 addition & 1 deletion test/Deployment.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ contract DeploymentTest is Test {
token.addAdminAccount(admin);
assertTrue(token.isSystemAccount(system));
assertTrue(token.isAdminAccount(admin));
token.addMinterAndBurner(system, amount * 3);
token.setLimitCap(amount * 3);
vm.startPrank(admin);
token.setLimits(system, amount * 3, amount * 3);
vm.stopPrank();
Expand Down
2 changes: 1 addition & 1 deletion test/ERC20.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ contract ERC20TokenTest is Test {
token.addAdminAccount(admin);
assertTrue(token.isSystemAccount(system));
assertTrue(token.isAdminAccount(admin));
token.addMinterAndBurner(system, 1e18);
token.setLimitCap( 1e18);
vm.startPrank(admin);
token.setLimits(system, 1e18, 1e18);
vm.stopPrank();
Expand Down
26 changes: 13 additions & 13 deletions test/Mintable.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ contract MintableTokenTest is Test {
}

function test_owner_can_set_max_mint_allowance() public {
token.addMinterAndBurner(system, 1000);
assertEq(token.mintingMaxLimitOf(system), 1000);
token.setLimitCap(1000);
assertEq(token.getLimitCap(), 1000);
}

/*
function test_non_owner_cannot_set_max_mint_allowance() public {
vm.startPrank(user1);
vm.expectRevert(
Expand All @@ -58,34 +57,34 @@ contract MintableTokenTest is Test {
user1
)
);
token.setMaxMintAllowance(1000);
token.setLimitCap(1000);
vm.stopPrank();
}*/
}

function test_admin_can_set_mint_allowance() public {
test_owner_can_set_max_mint_allowance();
token.addAdminAccount(admin);
vm.startPrank(admin);
token.setLimits(system, 500, 500);
token.setMintingLimit(system, 500);
vm.stopPrank();

assertEq(token.mintingCurrentLimitOf(system), 500);
}
/*

function test_admin_cannot_set_mint_allowance_above_max_mint_allowance()
public
{
test_owner_can_set_max_mint_allowance();
token.addAdminAccount(admin);
vm.startPrank(admin);
vm.expectRevert("MintAllowance: cannot set allowance higher than max");
token.setMintAllowance(system, 1500);
vm.expectRevert(abi.encodeWithSignature("RateLimit_LimitsTooHigh()"));
token.setMintingLimit(system, 1500);
vm.stopPrank();
}
*/

function test_non_admin_cannot_set_mint_allowance() public {
vm.expectRevert("SystemRole: caller is not an admin account");
token.setMintAllowance(user1, 500);

token.setMintingLimit(user1, 500);
}

function test_system_account_can_mint_tokens() public {
Expand All @@ -108,7 +107,8 @@ contract MintableTokenTest is Test {
token.addSystemAccount(system);
address user = vm.addr(userPrivateKey);
vm.startPrank(system);
vm.expectRevert("MintAllowance: not allowed to mint more than allowed");

vm.expectRevert(abi.encodeWithSignature("IXERC20_NotHighEnoughLimits()"));
token.mint(user, 1000);
vm.stopPrank();
}
Expand Down
Loading

0 comments on commit 36bfc41

Please sign in to comment.