Platform: Code4rena
Start Date: 10/03/2022
Pot Size: $75,000 USDT
Total HM: 25
Participants: 54
Period: 7 days
Judge: pauliax
Total Solo HM: 10
Id: 97
League: ETH
Rank: 4/54
Findings: 8
Award: $5,783.05
🌟 Selected for report: 3
🚀 Solo Findings: 1
🌟 Selected for report: minhquanym
1260.6939 USDT - $1,260.69
In the scenario where the transfer fee exceeds the equilibrium fee, the excess gets credited to the incentive pool. The incentive pool fee added is
incentivePool[tokenAddress] = (incentivePool[tokenAddress] + (amount * (transferFeePerc - tokenManager.getTokensInfo(tokenAddress).equilibriumFee))) / BASE_DIVISOR;
Due to an incorrect bracket placement, the current incentive pool amount is divided by BASE_DIVISOR each time.
Add the following lines after L316 in the "Current liquidity < Provided liquidity and pool remains in deficit state after deposit” test case.
// after L316 let currentIncentivePoolAmountAcc = await liquidityPool.incentivePool(tokenAddress); let currentlpFeeAcc = await liquidityProviders.getTotalLPFeeByToken(tokenAddress); let currentGasFeeAcc = await liquidityPool.gasFeeAccumulatedByToken(tokenAddress); let currentReceiverBal = await token.balanceOf(receiver); expect(currentIncentivePoolAmountAcc).to.be.gt(0); let withdrawAmount = BigNumber.from(minTokenCap).add(2); tx = await sendFundsToUser(tokenAddress, withdrawAmount.toString(), receiver, "0"); await tx.wait(); let accountedIncentivePoolFee = (await liquidityPool.incentivePool(tokenAddress)).sub(currentIncentivePoolAmountAcc); let accountedLpFee = (await liquidityProviders.getTotalLPFeeByToken(tokenAddress)).sub(currentlpFeeAcc); let accountedGasFee = (await liquidityPool.gasFeeAccumulatedByToken(tokenAddress)).sub(currentGasFeeAcc); let totalFees = withdrawAmount.sub((await token.balanceOf(receiver)).sub(currentReceiverBal)); expect(totalFees).to.be.eq(accountedIncentivePoolFee.add(accountedLpFee).add(accountedGasFee));
Apart from the incorrect bracket placement, there is a potential loss of 1 wei in certain instances due to precision errors. To ensure every wei is accounted for, calculate the total transfer fee first. The incentive fee is the difference between that and the lpFee.
uint256 transferFeeAmount = (amount * transferFeePerc) / BASE_DIVISOR; uint256 lpFee; uint256 equilibriumFee = tokenManager.getTokensInfo(tokenAddress).equilibriumFee; if (transferFeePerc > equilibriumFee) { lpFee = amount * equilibriumFee / BASE_DIVISOR; incentivePool[tokenAddress] += transferFeeAmount - lpFee; } else { // avoid recalculation lpFee = transferFeeAmount; }
#0 - pauliax
2022-04-28T20:55:03Z
#38
There isn’t a cap placed on the maximum equilibriumFee and maxFee values that can be set. Values that are greater than BASE_DIVISOR will result in subtraction overflow in sendFundsToUser(), thus preventing transfer transactions from execution.
In LiquidityPool.tests.ts, let equilibriumFee = 10_000_000_001; and run the test file. All failing tests should have reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block).
Ensure the values to be set do not exceed BASE_DIVISOR or a smaller limit since they are gas fees to be deducted too.
require(_equilibriumFee <= BASE_DIVISOR, "Equilibrium Fee cannot exceed 100%"); require(_maxFee <= BASE_DIVISOR, "Max Fee cannot exceed 100%");
#0 - ankurdubey521
2022-03-30T15:44:45Z
Duplicate of #8
#1 - pauliax
2022-04-30T17:26:19Z
🌟 Selected for report: hickuphh3
933.8474 USDT - $933.85
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L178-L184 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L115-L125
The totalLiquidity and totalLiquidityByLp mappings are not updated when an address is removed from the isExcludedAddress mapping. While this affects the enforcement of the cap limits and the getMaxCommunityLpPositon() function, the worst impact this has is that the address cannot have liquidity removed / transferred due to subtraction overflow.
In particular, users can be prevented from withdrawing their staked LP tokens from the liquidity farming contract should it become non-excluded.
0xA is excludedsetIsExcludedAddressStatus([0xA, false])totalLiquidityByLp[USDC][0xA] = 0, resulting in subtraction overflow.// insert test case in Withdraw test block of LiquidityFarming.tests.ts it.only('will brick withdrawals by no longer excluding farming contract', async () => { await farmingContract.deposit(1, bob.address); await wlpm.setIsExcludedAddressStatus([farmingContract.address], [false]); await farmingContract.connect(bob).withdraw(1, bob.address); }); // results in // Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block)
The simplest way is to prevent exclusion removals.
function setIsExcludedAddresses(address[] memory _addresses) external onlyOwner { for (uint256 i = 0; i < _addresses.length; ++i) { isExcludedAddress[_addresses[i]] = true; // emit event emit AddressExcluded(_addresses[i]); } }
#0 - pauliax
2022-05-02T12:32:24Z
Great find, but I think it should be of Medium severity because it requires an external condition, the owner should stop excluding the contract, and also in case that happens, function setIsExcludedAddresses can be used to exclude this address again so the funds are not stuck forever in this case.
🌟 Selected for report: hickuphh3
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L178-L184 https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/WhitelistPeriodManager.sol#L83-L99
The totalLiquidity and totalLiquidityByLp mappings are not updated when an address is added to the isExcludedAddress mapping. This affects the enforcement of the cap limits and the getMaxCommunityLpPositon() function, which implicitly assumes that whitelisted addresses will have 0 liquidity, for addresses with non-zero liquidity at the time of addition to the whitelist.
0xA is the sole USDC liquidity provider
totalLiquidity[USDC] = 500totalLiquidity[USDC][0xA] = 500500, ie. perTokenTotalCap[USDC] = 5000xA: setIsExcludedAddressStatus([0xA, true])
getMaxCommunityLpPositon() returns 500 when it should return 0// insert test case in WhitelistPeriodManager.test.ts describe.only("Test whitelist addition", async () => { it('produces deviant behaviour if excluding address with existing liquidity', async () => { await wlpm.setCaps([token.address], [500], [500]); await liquidityProviders.connect(owner).addTokenLiquidity(token.address, 500); await wlpm.setIsExcludedAddressStatus([owner.address], [true]); // 1) returns 500 instead of 0 console.log((await wlpm.getMaxCommunityLpPositon(token.address)).toString()); // 2) bob (or other non-excluded addresses) will be unable to add liquidity await expect(liquidityProviders.connect(bob).addTokenLiquidity(token.address, 1)).to.be.revertedWith('ERR__LIQUIDITY_EXCEEDS_PTTC'); }); });
Check that the address to be excluded is not holding any LP token at the time of exclusion.
// in setIsExcludedAddressStatus() for (uint256 i = 0; i < _addresses.length; ++i) { if (_status[i]) { require(lpToken.balanceOf(_addresses[i]) == 0, 'address has existing liquidity'); } ... }
#0 - ankurdubey521
2022-03-30T15:41:07Z
Duplicate of #72
#1 - pauliax
2022-05-02T13:11:22Z
I think it is a different issue than #72, based on the description it deserves a severity of Medium.
🌟 Selected for report: hickuphh3
Also found by: 0v3rf10w, 0x1f8b, 0xDjango, 0xNazgul, 0xngndev, 0xwags, Cantor_Dust, CertoraInc, Dravee, IllIllI, PPrieditis, Ruhum, TerrierLover, WatchPug, XDms, benk10, berndartmueller, bitbopper, catchup, cmichel, cryptphi, csanuragjain, danb, defsec, gzeon, hagrid, hubble, jayjonah8, kenta, kyliek, minhquanym, rfa, robee, saian, samruna, throttle, ye0lde, z3s
1163.6691 USDT - $1,163.67
Overall, code quality for the Hyphen 2.0 contracts is high. Supporting documentation was adequate in helping to understand the incentive and fee mechanisms for cross-chain transfers.
The contracts in scope have 81.36% statement and 54.91% branch test coverage. Notably, the Liquidity Pool’s permitAndDepositErc20() and permitEIP2612AndDepositErc20() functions that allow users to deposit with signed messages are untested. It will be ideal to write more tests so that better coverage is achieved. Also note that some liquidity farming tests often fail because rewards are continuously accruing, so the actual amount tends to be greater than the expected amount.
One area of concern is the sendFundsToUser() function, where executors are fully trusted to provide the correct information to complete the cross-chain transfer. Any executor that becomes compromised will enable the attacker to fully drain the available liquidity of a pool by submitting multiple malicious transactions.
BASE_DIVISORhttps://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L20
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityProviders.sol#L27
BASE_DIVISOR is 10_000_000_000 in LiquidityPool, but 10**18 in LiquidityProviders. This can easily confuse 3rd parties integrating the token bridge.
Rename either variable. I recommend renaming the instance in LiquidityPool to FEE_DIVISOR.
BASE_DIVISORhttps://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L20
BASE_DIVISOR is defined as 10_000_000_000; with accompanying description // Basis Points * 100 for better accuracy.
This isn’t accurate as 100% = 10,000 basis points, and 10,000 * 100 = 1_000_000, not 10_000_000_000.
Either update the comment to be
uint256 private constant BASE_DIVISOR = 10_000_000_000; // 100 * (Basis Points ^ 2) for better accuracy
or the BASE_DIVISOR itself to be a different value.
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L20
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L350
In relation to L02, there are conflicting definitions of the fee denomination. BASE_DIVISOR says that fees are in Basis points * 100, while the comment in getTransferFees() says they are specified in basis points * 10.
Standardize the fee denomination.
getAmountToTransfer() results in wei lossesIn the scenario where the transfer fee exceeds the equilibrium fee, the excess gets credited to the incentive pool. Disregarding from the fact that a bracket is incorrectly placed causing a massive loss in incentives (raised in separate issue), there are cases where 1 wei is unaccounted for from precision loss in the calculation.
lpFee = (amount * tokenManager.getTokensInfo(tokenAddress).equilibriumFee) / BASE_DIVISOR; // altered for bracket positioning incentivePool[tokenAddress] += (amount * (transferFeePerc - tokenManager.getTokensInfo(tokenAddress).equilibriumFee)) / BASE_DIVISOR;
amount = 337671308498transferFeePerc = 181480242equilibriumFee = 10000000 (0.1%)Calculated amounts are
lpFee = 337671308incentive = 337671308498 * (181480242 - 10000000) / BASE_DIVISOR = 5790395769Total fee calculated = 337671308 + 5790395769 = 6128067077
transferFeeAmount = 337671308498 * 181480242 / BASE_DIVISOR = 6128067078We therefore see 1 wei being unaccounted for.
uint256 transferFeeAmount = (amount * transferFeePerc) / BASE_DIVISOR; uint256 lpFee; uint256 equilibriumFee = tokenManager.getTokensInfo(tokenAddress).equilibriumFee; if (transferFeePerc > equilibriumFee) { lpFee = amount * equilibriumFee / BASE_DIVISOR; incentivePool[tokenAddress] += transferFeeAmount - lpFee; } else { ... }
getMaxCommunityLpPositon()The getMaxCommunityLpPosition() iterates through the LP token supply to obtain the maximum community LP position obtained. Because the supply of NFT tokens is uncapped, there will come a point where this function runs out of gas.
In the worst case, the limit seems to be at about 1250 NFTs where the (N+1)th LP token has more liquidity than the Nth LP token.
it.only("Tries to get max iterations possible for getMaxCommunityLpPositon()", async () => { let MAX_LOOPS = 1250; // summation formula for 1 to MAX_LOOPS let maxCap = MAX_LOOPS * (MAX_LOOPS + 1) / 2; await wlpm.setCaps([token.address], [maxCap], [maxCap]); for (let i = 1; i <= MAX_LOOPS; i++) { console.log(`adding ${i}`); // worst case: every iteration in getMaxCommunityLpPositon() enters if case // by giving next tokenId more liquidity await liquidityProviders.connect(owner).addTokenLiquidity(token.address, i); } console.log('getting max lp position...'); // Runs out of gas // Error: Transaction reverted and Hardhat couldn't infer the reason. Please report this to help us improve Hardhat. await wlpm.getMaxCommunityLpPositon(token.address); });
Have start and end indexes as inputs to cap the number of iterations performed.
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L175
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L186
The comment says to use 0x00 for Ethereum, but the implementation uses NATIVE instead.
use 0x00 for Ethereum → use NATIVE for native token
addSupportedToken() allows zero fees to be set, but changeFee() doesn’tAs per the title, the addSupportedToken() allows for zero equilibriumFee or maxFee to be set, but changeFee() doesn’t.
Either include non-zero checks in addSupportedToken() or remove them in changeFee().
receive() functionIn general, it would be good to restrict native fund transfer senders to prevent users from accidentally sending them. 3 contracts have the receive() function:
LiquidityFarming
LiquidityPool
LiquidityProviders contract when someone adds or increases their native liquidity positions.
LiquidityProviders contract.LiquidityProviders
LiquidityPool:receive() external payable { require(_msgSender() == address(liquidityProviders), "Only liquidityProviders is allowed"); emit EthReceived(_msgSender(), msg.value); }
LiquidityProviders_sendErc20AndGetSentAmount() uses recipient instead of sender balance differenceThe function name implies that the sent amount should be returned, but it uses the amount received by the recipient instead.
uint256 recepientBalance = _token.balanceOf(_to); _token.safeTransfer(_to, _amount); return _token.balanceOf(_to) - recepientBalance;
If a fee-on-transfer token is the reward token, the amount sent vs received would differ.
Decide which value is to be returned and logged. Either update the function to be _sendErc20AndGetReceivedAmount() or change it to use the contract’s balance difference instead.
uint256 senderBalance = _token.balanceOf(address(this)); _token.safeTransfer(_to, _amount); return _token.balanceOf(address(this)) - senderBalance;
FoT token deposits are not supported because _amount is used for internal accounting, but the actual amount received will be less than it due to the fee.
I gave a low severity rating because of the existence of a token whitelist. Referenced from https://github.com/code-423n4/2021-12-sublime-findings/issues/143
Use the actual token balance change instead of _amount, similar to the _sendErc20AndGetSentAmount.
uint256 currentBalance = _token.balanceOf(address(this)); IERC20Upgradeable(_token).safeTransferFrom(_msgSender(), address(liquidityPool), _amount); return _token.balanceOf(address(this)) - currentBalance;
It is unclear what each variable consists of, because there is:
It would be best to explictly state what each variable consists of for clarity.
mapping(address => uint256) public totalReserve; // Supplied Liquidity (SL) + LP Fees mapping(address => uint256) public totalLiquidity; // Supplied Liquidity only // Available liquidity = SL + net deposits and withdrawals from bridging activity mapping(address => uint256) public currentLiquidity; // Available Liquidity + All Fees (LP, Incentive, Gas), updated on every in and out transfer
As per OpenZeppelin’s (OZ) recommendation, “The guidelines are now to make it impossible for anyone to run initialize on an implementation contract, by adding an empty constructor with the initializer modifier. So the implementation contract gets initialized automatically upon deployment.”
Note that this behaviour is also incorporated the OZ Wizard since the UUPS vulnerability discovery: “Additionally, we modified the code generated by the Wizard to include a constructor that automatically initializes the implementation when deployed.”
Add an empty constructor method to the relevant upgradeable contracts.
/// @custom:oz-upgrades-unsafe-allow constructor constructor() initializer {}
There is no limit to the gas fee charged. While it is claimed that “there are no incentives for the executors”, in reality, executors can be indirectly incentivised by inflating the gas price so that they will be credited a higher fee.
The fee can be as high as the bridged amount - transfer fee, leaving nothing for the user. While there is a lot of trust placed on the executor already, it would help to be able to provide a trust-less solution by enforcing a cap on the gas fee.
Limit the maximum gas fee chargeable.
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L103
The format seems to be 2 underscores after ERR, but the line reference above only has 1 underscore: ERR_REWARD_TOKEN_IS_ZERO.
ERR_REWARD_TOKEN_IS_ZERO -> ERR__REWARD_TOKEN_IS_ZERO
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L351
The variable order in the comment do not correspond to that of the implementation. For readability, I recommend that they do.
uint256 numerator = providedLiquidity * equilibriumFee * maxFee; // L(e) * F(e) * F(max)
The fee calculation formula mentions a deep factor d: Value that decides how much deeper the curve looks. Readers may have the impression that this may therefore be a customisable parameter in the contract, but in actual factor, is set to a constant value of 1.
Users / readers should be made aware that the deep factor has been fixed.
#0 - pauliax
2022-05-15T19:58:04Z
"L13: Consider having limit on gas fee charged" could be Medium: https://github.com/code-423n4/2022-03-biconomy-findings/issues/8#issuecomment-1114023886 but is already reported in a separate issue #76
#1 - pauliax
2022-05-15T20:00:23Z
"L10: Deposits don’t work with FoT tokens" should be Medium: https://github.com/code-423n4/2022-03-biconomy-findings/issues/91#issuecomment-1109645407
#2 - pauliax
2022-05-15T20:02:11Z
"L08: Restrict / remove receive() function" should be Medium: #157
#3 - pauliax
2022-05-15T20:05:41Z
"One area of concern is the sendFundsToUser() function, where executors are fully trusted to provide the correct information to complete the cross-chain transfer." could be Medium: https://github.com/code-423n4/2022-03-biconomy-findings/issues/80#issuecomment-1109662505
#4 - pauliax
2022-06-12T07:27:44Z
I think L02, L03, L06, and L11 are non-critical.
🌟 Selected for report: Dravee
Also found by: 0v3rf10w, 0x1f8b, 0xDjango, 0xNazgul, 0xngndev, 0xwags, Cantor_Dust, CertoraInc, IllIllI, Jujic, Kenshin, Kiep, PPrieditis, TerrierLover, Tomio, WatchPug, antonttc, benk10, berndartmueller, bitbopper, csanuragjain, defsec, gzeon, hagrid, hickuphh3, kenta, minhquanym, oyc_109, pedroais, peritoflores, rfa, robee, saian, samruna, sirhashalot, throttle, wuwe1, z3s
70.7008 USDT - $70.70
Foundry. Worked like a charm for fuzzing.
If amount < liquidityDifference, the reward amount calculated is
rewardAmount = (amount * incentivePool[tokenAddress] * 10000000000) / liquidityDifference; rewardAmount = rewardAmount / 10000000000;
The magnification doesn’t work at all because you’re multiplying and dividing by the same amount. It only makes sense if the numerator is represented as a fraction of the denominator, like equilibriumFee being denominated in BASE_DIVISOR.
I ported over the logic to a separate contract and fuzz tested it in foundry to see if the magnification worked.
pragma solidity 0.8.0; contract RoundCalc { function calcRewardAmount(uint256 amount, uint256 incentive, uint256 liquidityDifference) public view returns (uint256 rewardAmount) { rewardAmount = (amount * incentive * 10000000000) / liquidityDifference; rewardAmount = rewardAmount / 10000000000; } function calcRewardAmountNoMagnification(uint256 amount, uint256 incentive, uint256 liquidityDifference) public view returns (uint256 rewardAmount) { rewardAmount = amount * incentive / liquidityDifference; } } import "ds-test/test.sol"; contract RoundCalcTest is DSTest { RoundCalc round; function setUp() public { round = new RoundCalc(); } function testMagnification(uint256 amount, uint256 incentive, uint256 liquidityDifference) public { // reasonably sized caps on incentive and amount (indirectly through liquidityDifference) incentive = bound(incentive, 1, 1e30); liquidityDifference = bound(liquidityDifference, 1, 1e35); // amount < liquidityDifference amount = bound(amount, 1, liquidityDifference); uint256 rewardAmountMagnified = round.calcRewardAmount(amount, incentive, liquidityDifference); uint256 rewardAmount = round.calcRewardAmountNoMagnification(amount, incentive, liquidityDifference); assertEq(rewardAmountMagnified, rewardAmount); } // taken from solmate, thanks @t11s =) function bound( uint256 x, uint256 min, uint256 max ) internal pure returns (uint256 result) { require(max >= min, "MAX_LESS_THAN_MIN"); uint256 size = max - min; if (max != type(uint256).max) size++; // Make the max inclusive. if (size == 0) return min; // Using max would be equivalent as well. // Ensure max is inclusive in cases where x != 0 and max is at uint max. if (max == type(uint256).max && x != 0) x--; // Accounted for later. if (x < min) x += size * (((min - x) / size) + 1); result = min + ((x - min) % size); // Account for decrementing x to make max inclusive. if (max == type(uint256).max && x != 0) result++; } }
Using 100_000 fuzz runs, there was no difference found in the computed values.
Remove the redundant multiplication and division by 10000000000.
rewardAmount = amount * incentivePool[tokenAddress] / liquidityDifference;
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L316
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L318
https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L321
Multiple external calls are made to fetch values from other contracts. It would be more gas efficient to save the result in memory.
TokenConfig memory depositConfig = tokenManager.getDepositConfig(toChainId, NATIVE); require(depositConfig.min <= amount && depositConfig.max >= amount, "Deposit amount not in Cap limit"); TokenConfig memory depositConfig = tokenManager.getDepositConfig(toChainId, tokenAddress); require(depositConfig.min <= amount && depositConfig.max >= amount, "Deposit amount not in Cap limit"); TokenConfig memory transferConfig = tokenManager.getTransferConfig(tokenAddress); require(transferConfig.min <= amount && transferConfig.max >= amount, "Withdraw amnt not in Cap limits"); uint256 equilibriumFee = tokenManager.getTokensInfo(tokenAddress).equilibriumFee; // replace tokenManager.getTokensInfo(tokenAddress).equilibriumFee with equilibriumFee subsequently // alternatively, change getTokensInfo to return (uint256, uint256) instead ITokenManager.TokenInfo memory config = tokenManager.getTokensInfo(tokenAddress); uint256 numerator = providedLiquidity * config.equilibriumFee * config.maxFee; uint256 denominator = config.equilibriumFee * providedLiquidity + (config.maxFee - config.equilibriumFee) * resultingLiquidity;
LiquidityProvidersMajority of the mappings are declared public, but have duplicate accessor methods. You have situations where a mixture of both ways are being called:
uint256 liquidityPoolBalance = liquidityProviders.getCurrentLiquidity(tokenAddress); currentLiquidity = liquidityPoolBalance - liquidityProviders.totalLPFees(tokenAddress) - gasFeeAccumulatedByToken[tokenAddress] - incentivePool[tokenAddress];
Either have the mappings made internal, or remove the accessor methods.
totalSharesMinted[_baseToken] with supplytotalSharesMinted[_baseToken] has already been fetched a couple of lines above. Avoid the duplicate external call.
uint256 supply = totalSharesMinted[_baseToken]; if (supply > 0) { return supply / totalReserve[_baseToken]; } return BASE_DIVISOR;
Since pending can be 0, checking if (pending != 0) before doing subsequent accounting and transfer of the token will potentially save an external call and the unnecessary gas cost of a 0 token transfer.
pending = ((amount * pool.accTokenPerShare) / ACC_TOKEN_PRECISION) - nft.rewardDebt + nft.unpaidRewards; if (pending != 0) { if (rewardTokens[baseToken] == NATIVE) { ... } ... }