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

contract refactor #46

Merged
merged 5 commits into from
Jul 31, 2024
Merged

contract refactor #46

merged 5 commits into from
Jul 31, 2024

Conversation

YouStillAlive
Copy link
Member

closes #40

Copy link

github-actions bot commented Jul 31, 2024

Methods

Symbol Meaning
Execution gas for this method does not include intrinsic gas overhead
Cost was non-zero but below the precision setting for the currency display (see options)
Min Max Avg Calls usd avg
BNBPartyFactory
       createParty(string,string) 5,668,459 5,882,147 5,728,542 29 3.36
       joinParty(address,uint256) 153,302 5,642,318 1,818,734 20 1.07
       leaveParty(address,uint256,uint256) 174,469 191,569 180,169 3 0.11
       setNonfungiblePositionManager(address,address) 27,399 68,928 66,158 15 0.04
       setSwapRouter(address) 24,119 46,259 44,782 15 0.03
       withdrawFee() - - 31,792 2 0.02
       withdrawPartyLPFee(address[]) 173,835 173,880 173,858 2 0.10
NonfungiblePositionManager
       approve(address,uint256) 46,052 46,383 46,273 3 0.03
SwapRouter
       exactInput((bytes,address,uint256,uint256,uint256)) 132,707 142,139 138,995 3 0.08
       multicall(bytes[]) - - 160,866 2 0.09

Deployments

Min Max Avg Block % usd avg
BNBPartyFactory 3,306,916 3,306,928 3,306,927 2.5 % 1.94
MockNonfungibleTokenPositionDescriptor - - 111,537 0.1 % 0.07
NonfungiblePositionManager 5,171,500 5,171,524 5,171,519 4 % 3.04
SwapRouter 2,201,078 2,201,090 2,201,088 1.7 % 1.29
UniswapV3Factory 5,437,097 5,437,109 5,437,108 4.2 % 3.19

Solidity and Network Config

Settings Value
Solidity: version 0.8.24
Solidity: optimized true
Solidity: runs 200
Solidity: viaIR false
Block Limit 130,000,000
L1 Gas Price 1 gwei
Token Price 587.17 usd/bnb
Network BINANCE
Toolchain hardhat

Copy link

github-actions bot commented Jul 31, 2024

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

incorrect-equality

Impact: Medium
Confidence: High

function _createFLP(address _token) internal returns (address liquidityPool) {
(address tokenA, address tokenB, uint160 sqrtPrice) = _getTokenPairAndPrice(_token);
uint256 amount0;
uint256 amount1;
if (IERC20(tokenA).balanceOf(address(this)) == party.initialTokenAmount) {
amount0 = party.initialTokenAmount;
} else {
amount1 = party.initialTokenAmount;
}
IERC20(_token).approve(
address(BNBPositionManager),
party.initialTokenAmount
);
liquidityPool = _createLP(
BNBPositionManager,
tokenA,
tokenB,
amount0,
amount1,
sqrtPrice,
party.partyLpFee
);
isParty[liquidityPool] = true;
}

function withdrawFee() external onlyOwner {
if (address(this).balance == 0) {
revert ZeroAmount();
}
payable(msg.sender).transfer(address(this).balance);
emit TransferOutBNB(msg.sender, address(this).balance);
}

reentrancy-no-eth

Impact: Medium
Confidence: Medium

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

uninitialized-local

Impact: Medium
Confidence: Medium

unused-return

Impact: Medium
Confidence: Medium

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

function _executeSwap(
address tokenIn,
address tokenOut,
address recipient,
uint256 amountOutMinimum,
uint256 amountIn
) internal notZeroAddress(address(swapRouter)) {
ISwapRouter.ExactInputParams memory params = ISwapRouter
.ExactInputParams({
path: abi.encodePacked(tokenIn, party.partyLpFee, tokenOut),
recipient: recipient,
deadline: block.timestamp,
amountIn: amountIn,
amountOutMinimum: amountOutMinimum
});
uint256 value = msg.value > 0 ? amountIn : 0;
swapRouter.exactInput{value: value}(params);
}

function _createFLP(address _token) internal returns (address liquidityPool) {
(address tokenA, address tokenB, uint160 sqrtPrice) = _getTokenPairAndPrice(_token);
uint256 amount0;
uint256 amount1;
if (IERC20(tokenA).balanceOf(address(this)) == party.initialTokenAmount) {
amount0 = party.initialTokenAmount;
} else {
amount1 = party.initialTokenAmount;
}
IERC20(_token).approve(
address(BNBPositionManager),
party.initialTokenAmount
);
liquidityPool = _createLP(
BNBPositionManager,
tokenA,
tokenB,
amount0,
amount1,
sqrtPrice,
party.partyLpFee
);
isParty[liquidityPool] = true;
}

function _collectFee(
address liquidityPool,
INonfungiblePositionManager manager
) internal notZeroAddress(liquidityPool) {
manager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[liquidityPool],
recipient: msg.sender,
amount0Max: type(uint128).max,
amount1Max: type(uint128).max
})
);
}

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

function _getFeeGrowthInsideLastX128(
IUniswapV3Pool pool
)
internal
view
returns (
uint256 feeGrowthInside0LastX128,
uint256 feeGrowthInside1LastX128
)
{
(, feeGrowthInside0LastX128, feeGrowthInside1LastX128, , ) = pool
.positions(
keccak256(
abi.encodePacked(
address(BNBPositionManager),
party.tickLower,
party.tickUpper
)
)
);
}

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

function _createLP(
INonfungiblePositionManager liquidityManager,
address token0,
address token1,
uint256 amount0,
uint256 amount1,
uint160 sqrtPriceX96,
uint24 fee
) internal returns (address liquidityPool) {
// Create LP
liquidityPool = liquidityManager.createAndInitializePoolIfNecessary(
token0,
token1,
fee,
sqrtPriceX96
);
// Mint LP
(lpToTokenId[liquidityPool], , , ) = liquidityManager.mint(
INonfungiblePositionManager.MintParams({
token0: token0,
token1: token1,
fee: fee,
tickLower: party.tickLower,
tickUpper: party.tickUpper,
amount0Desired: amount0,
amount1Desired: amount1,
amount0Min: 0,
amount1Min: 0,
recipient: address(this),
deadline: block.timestamp
})
);
}

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

shadowing-local

Impact: Low
Confidence: High

string memory symbol,

reentrancy-benign

Impact: Low
Confidence: Medium

function _createFLP(address _token) internal returns (address liquidityPool) {
(address tokenA, address tokenB, uint160 sqrtPrice) = _getTokenPairAndPrice(_token);
uint256 amount0;
uint256 amount1;
if (IERC20(tokenA).balanceOf(address(this)) == party.initialTokenAmount) {
amount0 = party.initialTokenAmount;
} else {
amount1 = party.initialTokenAmount;
}
IERC20(_token).approve(
address(BNBPositionManager),
party.initialTokenAmount
);
liquidityPool = _createLP(
BNBPositionManager,
tokenA,
tokenB,
amount0,
amount1,
sqrtPrice,
party.partyLpFee
);
isParty[liquidityPool] = true;
}

function createParty(
string calldata name,
string calldata symbol
)
external
payable
override
nonReentrant
insufficientBNB
notZeroAddress(address(BNBPositionManager))
returns (IERC20 newToken)
{
// create new token
newToken = new ERC20Token(name, symbol, party.initialTokenAmount);
// create First Liquidity Pool
address liquidityPool = _createFLP(address(newToken));
lpToCreator[liquidityPool] = msg.sender;
if (msg.value > party.createTokenFee) {
_executeSwap(address(newToken));
}
emit StartParty(address(newToken), msg.sender, liquidityPool);
}

function _createLP(
INonfungiblePositionManager liquidityManager,
address token0,
address token1,
uint256 amount0,
uint256 amount1,
uint160 sqrtPriceX96,
uint24 fee
) internal returns (address liquidityPool) {
// Create LP
liquidityPool = liquidityManager.createAndInitializePoolIfNecessary(
token0,
token1,
fee,
sqrtPriceX96
);
// Mint LP
(lpToTokenId[liquidityPool], , , ) = liquidityManager.mint(
INonfungiblePositionManager.MintParams({
token0: token0,
token1: token1,
fee: fee,
tickLower: party.tickLower,
tickUpper: party.tickUpper,
amount0Desired: amount0,
amount1Desired: amount1,
amount0Min: 0,
amount1Min: 0,
recipient: address(this),
deadline: block.timestamp
})
);
}

reentrancy-events

Impact: Low
Confidence: Medium

function _unwrapAndSendBNB(address recipient, uint256 unwrapAmount) internal {
address creator = lpToCreator[msg.sender];
WBNB.withdraw(unwrapAmount);
if (recipient == creator) {
_transferBNB(recipient, party.bonusTargetReach + party.bonusPartyCreator);
} else {
_transferBNB(recipient, party.bonusTargetReach);
_transferBNB(creator, party.bonusPartyCreator);
}
}

function _unwrapAndSendBNB(address recipient, uint256 unwrapAmount) internal {
address creator = lpToCreator[msg.sender];
WBNB.withdraw(unwrapAmount);
if (recipient == creator) {
_transferBNB(recipient, party.bonusTargetReach + party.bonusPartyCreator);
} else {
_transferBNB(recipient, party.bonusTargetReach);
_transferBNB(creator, party.bonusPartyCreator);
}
}

function _transferBNB(address recipient, uint256 amount) private {
// Use call to send BNB
(bool success, ) = recipient.call{value: amount}("");
if (!success) {
revert BonusAmountTransferFailed();
}
emit TransferOutBNB(recipient, amount);
}

function _handleLiquidity(address recipient) internal {
IUniswapV3Pool pool = IUniswapV3Pool(msg.sender);
(uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
address token0 = pool.token0();
address token1 = pool.token1();
// Decrease liquidity and collect tokens
(uint256 amount0, uint256 amount1) = BNBPositionManager.decreaseLiquidity(
INonfungiblePositionManager.DecreaseLiquidityParams({
tokenId: lpToTokenId[msg.sender],
liquidity: pool.liquidity(),
amount0Min: 0,
amount1Min: 0,
deadline: block.timestamp
})
);
BNBPositionManager.collect(
INonfungiblePositionManager.CollectParams({
tokenId: lpToTokenId[msg.sender],
recipient: address(this),
amount0Max: uint128(amount0),
amount1Max: uint128(amount1)
})
);
uint256 unwrapAmount = party.bonusTargetReach + party.bonusPartyCreator + party.targetReachFee;
if (token0 == address(WBNB)) {
amount0 -= unwrapAmount;
} else {
amount1 -= unwrapAmount;
}
IERC20(token0).approve(address(positionManager), amount0);
IERC20(token1).approve(address(positionManager), amount1);
// Create new Liquidity Pool
_createLP(positionManager, token0, token1, amount0, amount1, sqrtPriceX96, party.lpFee);
// Send bonuses
_unwrapAndSendBNB(recipient, unwrapAmount);
}

function _unwrapAndSendBNB(address recipient, uint256 unwrapAmount) internal {
address creator = lpToCreator[msg.sender];
WBNB.withdraw(unwrapAmount);
if (recipient == creator) {
_transferBNB(recipient, party.bonusTargetReach + party.bonusPartyCreator);
} else {
_transferBNB(recipient, party.bonusTargetReach);
_transferBNB(creator, party.bonusPartyCreator);
}
}

assembly

Impact: Informational
Confidence: High

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/utils/Address.sol#L146-L158

low-level-calls

Impact: Informational
Confidence: High

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol#L110-L117

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/utils/Address.sol#L95-L98

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/utils/Address.sol#L104-L107

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/utils/Address.sol#L41-L50

function _transferBNB(address recipient, uint256 amount) private {
// Use call to send BNB
(bool success, ) = recipient.call{value: amount}("");
if (!success) {
revert BonusAmountTransferFailed();
}
emit TransferOutBNB(recipient, amount);
}

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/utils/Address.sol#L83-L89

naming-convention

Impact: Informational
Confidence: High

IWBNB public immutable WBNB;

INonfungiblePositionManager public BNBPositionManager; // BNB Party position manager

function setSwapRouter(ISwapRouter _swapRouter) external onlyOwner {

INonfungiblePositionManager _BNBPositionManager,

function WETH9() external view returns (address);

INonfungiblePositionManager _positionManager

https://github.com/bnb-party/BNBParty.Factory/blob/02990fad50a33f683d4d880c965be2c45227cfe4/node_modules/@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol#L89

reentrancy-unlimited-gas

Impact: Informational
Confidence: Medium

function withdrawFee() external onlyOwner {
if (address(this).balance == 0) {
revert ZeroAmount();
}
payable(msg.sender).transfer(address(this).balance);
emit TransferOutBNB(msg.sender, address(this).balance);
}

unused-import

Impact: Informational
Confidence: High

  • ID-39
    The following unused import(s) in contracts/BNBPartyState.sol should be removed:
    -import "./interfaces/IUniswapV3Pool.sol"; (contracts/BNBPartyState.sol#9)

@YouStillAlive YouStillAlive marked this pull request as ready for review July 31, 2024 07:54
@YouStillAlive YouStillAlive merged commit f50bc69 into master Jul 31, 2024
4 checks passed
@YouStillAlive YouStillAlive deleted the issue-40 branch July 31, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split/refactor the contract
2 participants