From 4a589478c94b468491817a52beca4d88d5dbaee5 Mon Sep 17 00:00:00 2001 From: ungaro Date: Wed, 11 Dec 2024 01:01:46 -0500 Subject: [PATCH 1/2] remove nonreentrant from aggregatetoken override functions --- nest/src/AggregateToken.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nest/src/AggregateToken.sol b/nest/src/AggregateToken.sol index 0bb4d3a..2c9cce9 100644 --- a/nest/src/AggregateToken.sol +++ b/nest/src/AggregateToken.sol @@ -173,7 +173,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { uint256 assets, address receiver, address controller - ) public override(ComponentToken, IComponentToken) nonReentrant returns (uint256 shares) { + ) public override(ComponentToken, IComponentToken) returns (uint256 shares) { if (_getAggregateTokenStorage().paused) { revert DepositPaused(); } @@ -190,7 +190,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { function deposit( uint256 assets, address receiver - ) public override(ERC4626Upgradeable, IERC4626) nonReentrant returns (uint256 shares) { + ) public override(ERC4626Upgradeable, IERC4626) returns (uint256 shares) { if (_getAggregateTokenStorage().paused) { revert DepositPaused(); } @@ -209,7 +209,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { uint256 shares, address receiver, address controller - ) public override(ComponentToken) nonReentrant returns (uint256 assets) { + ) public override(ComponentToken) returns (uint256 assets) { if (_getAggregateTokenStorage().paused) { revert DepositPaused(); } @@ -226,7 +226,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { function mint( uint256 shares, address receiver - ) public override(ERC4626Upgradeable, IERC4626) nonReentrant returns (uint256 assets) { + ) public override(ERC4626Upgradeable, IERC4626) returns (uint256 assets) { if (_getAggregateTokenStorage().paused) { revert DepositPaused(); } @@ -238,7 +238,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { uint256 shares, address receiver, address controller - ) public override(ComponentToken, IComponentToken) nonReentrant returns (uint256 assets) { + ) public override(ComponentToken, IComponentToken) returns (uint256 assets) { return super.redeem(shares, receiver, controller); } From 45c8be09bfb3e41c4e937a64b89eb0b479cf9a27 Mon Sep 17 00:00:00 2001 From: ungaro Date: Wed, 11 Dec 2024 12:17:58 -0500 Subject: [PATCH 2/2] add comments about double reentrancy --- nest/src/AggregateToken.sol | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/nest/src/AggregateToken.sol b/nest/src/AggregateToken.sol index 2c9cce9..dd6bcb7 100644 --- a/nest/src/AggregateToken.sol +++ b/nest/src/AggregateToken.sol @@ -169,6 +169,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { } /// @inheritdoc IComponentToken + /// @dev Do not add reentrancy guard here, as it is already handled in the parent contract function deposit( uint256 assets, address receiver, @@ -182,7 +183,8 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { /** * @inheritdoc ERC4626Upgradeable - * @dev Overridden to add pause check before deposit + * @dev Overridden to add pause check before deposit. Do not add reentrancy guard here, as it is already handled in + * the parent contract * @param assets Amount of assets to deposit * @param receiver Address that will receive the shares * @return shares Amount of shares minted @@ -199,7 +201,8 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { /** * @inheritdoc ComponentToken - * @dev Overridden to add pause check before minting + * @dev Overridden to add pause check before minting. Do not add reentrancy guard here, as it is already handled in + * the parent contract * @param shares Amount of shares to mint * @param receiver Address that will receive the shares * @param controller Address that controls the minting @@ -218,7 +221,8 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { /** * @inheritdoc ERC4626Upgradeable - * @dev Overridden to add pause check before minting + * @dev Overridden to add pause check before minting. Do not add reentrancy guard here, as it is already handled in + * the parent contract * @param shares Amount of shares to mint * @param receiver Address that will receive the shares * @return assets Amount of assets deposited @@ -234,6 +238,7 @@ contract AggregateToken is ComponentToken, IAggregateToken, ERC1155Holder { } /// @inheritdoc IComponentToken + /// @dev Do not add reentrancy guard here, as it is already handled in the parent contract function redeem( uint256 shares, address receiver,