Skip to content

Commit

Permalink
Add support for reentrancy guard on MaticX contract
Browse files Browse the repository at this point in the history
  • Loading branch information
evercoinx committed Sep 13, 2024
1 parent 40557da commit 82d742d
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 22 deletions.
65 changes: 45 additions & 20 deletions contracts/MaticX.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ contract MaticX is

bytes32 public constant PREDICATE_ROLE = keccak256("PREDICATE_ROLE");
bytes32 public constant BOT = keccak256("BOT");
uint256 private constant NOT_ENTERED = 1;
uint256 private constant ENTERED = 2;

address private validatorRegistry;
address private stakeManager;
Expand All @@ -45,6 +47,17 @@ contract MaticX is
mapping(address => WithdrawalRequest[]) private userWithdrawalRequests;
address public override fxStateRootTunnel;
address private polToken;
uint256 private reentrancyGuardStatus;

/**
* @dev Enables guard from reentrancy calls.
*/
modifier nonReentrant() {
require(reentrancyGuardStatus != ENTERED, "ReentrancyGuard: reentrant call");
reentrancyGuardStatus = ENTERED;
_;
reentrancyGuardStatus = NOT_ENTERED;
}

/**
* @dev Initializes the current contract.
Expand Down Expand Up @@ -105,6 +118,22 @@ contract MaticX is
emit SetupBotAdmin();
}

/**
* @dev Initializes version 2 of the current contract.
* @param _polToken - Address of the POL token
*/
function initializeV2(address _polToken) external override onlyRole(DEFAULT_ADMIN_ROLE) {
require(_polToken != address(0), "Zero POL token address");

polToken = _polToken;
reentrancyGuardStatus = NOT_ENTERED;

IERC20Upgradeable(_polToken).safeApprove(
stakeManager,
type(uint256).max
);
}

////////////////////////////////////////////////////////////
///// ///
///// ***Staking Contract Interactions*** ///
Expand All @@ -120,7 +149,7 @@ contract MaticX is
*/
function submit(
uint256 _amount
) external override whenNotPaused returns (uint256) {
) external override nonReentrant whenNotPaused returns (uint256) {
return _submit(msg.sender, _amount, false);
}

Expand All @@ -133,7 +162,7 @@ contract MaticX is
*/
function submitPOL(
uint256 _amount
) external override whenNotPaused returns (uint256) {
) external override nonReentrant whenNotPaused returns (uint256) {
return _submit(msg.sender, _amount, true);
}

Expand All @@ -145,6 +174,7 @@ contract MaticX is
* @param _pol - If the POL flow should be used
* @return Amount of MaticX shares generated
*/
// slither-disable-next-line reentrancy-benign
function _submit(
address depositSender,
uint256 _amount,
Expand Down Expand Up @@ -192,7 +222,9 @@ contract MaticX is
* @dev Registers a user's request to withdraw Matic tokens.
* @param _amount - Amount of Matic tokens that is requested to withdraw
*/
function requestWithdraw(uint256 _amount) external override whenNotPaused {
function requestWithdraw(
uint256 _amount
) external override nonReentrant whenNotPaused {
_requestWithdraw(_amount, false);
}

Expand All @@ -202,7 +234,7 @@ contract MaticX is
*/
function requestWithdrawPOL(
uint256 _amount
) external override whenNotPaused {
) external override nonReentrant whenNotPaused {
_requestWithdraw(_amount, true);
}

Expand All @@ -211,6 +243,7 @@ contract MaticX is
* @param _amount - Amount of POL that is requested to withdraw
* @param _pol - If the POL flow should be used
*/
// slither-disable-next-line reentrancy-no-eth
function _requestWithdraw(uint256 _amount, bool _pol) internal {
require(_amount > 0, "Invalid amount");

Expand Down Expand Up @@ -314,7 +347,7 @@ contract MaticX is
* user.
* @param _idx - Array index of the user's withdrawal request
*/
function claimWithdrawal(uint256 _idx) external override whenNotPaused {
function claimWithdrawal(uint256 _idx) external override nonReentrant whenNotPaused {
_claimWithdrawal(msg.sender, _idx, false);
}

Expand All @@ -323,7 +356,7 @@ contract MaticX is
* user.
* @param _idx - Array index of the user's withdrawal request
*/
function claimWithdrawalPOL(uint256 _idx) external override whenNotPaused {
function claimWithdrawalPOL(uint256 _idx) external override nonReentrant whenNotPaused {
_claimWithdrawal(msg.sender, _idx, true);
}

Expand Down Expand Up @@ -375,7 +408,7 @@ contract MaticX is
*/
function withdrawRewards(
uint256 _validatorId
) external override whenNotPaused returns (uint256) {
) external override nonReentrant whenNotPaused returns (uint256) {
return _withdrawRewards(_validatorId, false);
}

Expand All @@ -385,7 +418,7 @@ contract MaticX is
*/
function withdrawValidatorsReward(
uint256[] calldata _validatorIds
) external override whenNotPaused returns (uint256[] memory) {
) external override nonReentrant whenNotPaused returns (uint256[] memory) {
uint256[] memory rewards = new uint256[](_validatorIds.length);
for (uint256 i = 0; i < _validatorIds.length; i++) {
rewards[i] = _withdrawRewards(_validatorIds[i], false);
Expand All @@ -399,7 +432,7 @@ contract MaticX is
*/
function withdrawValidatorsRewardPOL(
uint256[] calldata _validatorIds
) external override whenNotPaused returns (uint256[] memory) {
) external override nonReentrant whenNotPaused returns (uint256[] memory) {
uint256[] memory rewards = new uint256[](_validatorIds.length);
for (uint256 i = 0; i < _validatorIds.length; i++) {
rewards[i] = _withdrawRewards(_validatorIds[i], true);
Expand Down Expand Up @@ -442,7 +475,7 @@ contract MaticX is
*/
function stakeRewardsAndDistributeFees(
uint256 _validatorId
) external override whenNotPaused onlyRole(BOT) {
) external override nonReentrant whenNotPaused onlyRole(BOT) {
_stakeRewardsAndDistributeFees(_validatorId, false);
}

Expand All @@ -453,7 +486,7 @@ contract MaticX is
*/
function stakeRewardsAndDistributeFeesPOL(
uint256 _validatorId
) external override whenNotPaused onlyRole(BOT) {
) external override nonReentrant whenNotPaused onlyRole(BOT) {
_stakeRewardsAndDistributeFees(_validatorId, true);
}

Expand Down Expand Up @@ -510,7 +543,7 @@ contract MaticX is
uint256 _fromValidatorId,
uint256 _toValidatorId,
uint256 _amount
) external override whenNotPaused onlyRole(DEFAULT_ADMIN_ROLE) {
) external override nonReentrant whenNotPaused onlyRole(DEFAULT_ADMIN_ROLE) {
require(
IValidatorRegistry(validatorRegistry).validatorIdExists(
_fromValidatorId
Expand Down Expand Up @@ -733,14 +766,6 @@ contract MaticX is

polToken = _address;
emit SetPOLToken(_address);

require(
IERC20Upgradeable(polToken).approve(
stakeManager,
type(uint256).max
),
"Unable to approve POL token on StakeManager"
);
}

////////////////////////////////////////////////////////////
Expand Down
2 changes: 2 additions & 0 deletions contracts/interfaces/IMaticX.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ interface IMaticX is IERC20Upgradeable {

event SetPOLToken(address _address);

function initializeV2(address _polToken) external;

function setupBotAdmin() external;

function submit(uint256 _amount) external returns (uint256);
Expand Down
2 changes: 1 addition & 1 deletion slither.config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"compile_force_framework": "hardhat",
"detectors_to_exclude": "calls-loop,constable-states,naming-convention,pragma,reentrancy-benign,reentrancy-events,reentrancy-no-eth,solc-version,unused-return",
"detectors_to_exclude": "calls-loop,constable-states,naming-convention,pragma,solc-version,unused-return",
"include_paths": "(MaticX.sol)",
"filter_paths": "(@openzeppelin/|lib/|mocks/|state-transfer/|tunnel/)"
}
2 changes: 1 addition & 1 deletion test/MaticX.forking.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe("MaticX (Forking)", function () {
await maticX
.connect(manager)
.setFxStateRootTunnel(fxStateRootTunnel.address);
await maticX.connect(manager).setPOLToken(pol.address);
await maticX.connect(manager).initializeV2(pol.address);

const defaultAdminRole = await maticX.DEFAULT_ADMIN_ROLE();

Expand Down

0 comments on commit 82d742d

Please sign in to comment.