dbXEN security record

In the `mintDBXENFT` function, `cycleAccruedFees` is updated every time the function is called, regardless of whether `currentCycleMem` equals `currentStartedCycle`. ```solidity function mintDBXENFT() { ... if(redeemed) { ... } else { setUpNewCycle(); ... } cycleAccruedFees[currentCycleMem] = cycleAccruedFees[currentCycleMem] + fee; ... } function stake(){ ... if(currentCycleMem == currentStartedCycle) { cycleAccruedFees[currentCycleMem] += stakeFee; } else { pendingFees += stakeFee; } ... } ``` However, in the `stake` function, `cycleAccruedFees` is only updated if `currentCycleMem` equals `currentStartedCycle`. Otherwise, the fee is added to `pendingFees`. This inconsistency might result in a portion of fees collected being missed. If the `pendingFees` mechanism is not in place, the update for `cycleAccruedFees[currentCycleMem]` does not take into account the fees that were collected in previous cycles. In consequence, some ethers will be locked in the contract.

The `unstake()` function probably prevents `DBXeNFT` holders to unstake `DXN` from `DBXeNFT`. This issue is caused by `Arithmetic over/underflow` in line#518. ```solidity=502 function unstake(uint256 tokenId, uint256 amount) external nonReentrant onlyNFTOwner(dbxenft, tokenId, msg.sender) { calculateCycle(); updateCycleFeesPerStakeSummed(); updateDBXeNFT(tokenId); require( amount > 0, "DBXeNFT: Amount is zero." ); require( amount <= dbxenftWithdrawableStake[tokenId], "DBXeNFT: Amount greater than withdrawable stake" ); uint256 powerDecrease = calcExtraPower(baseDBXeNFTPower[tokenId], amount); dbxenftWithdrawableStake[tokenId] -= amount; dbxenftPower[tokenId] -= powerDecrease; if (lastStartedCycle == currentStartedCycle) { pendingStakeWithdrawal += powerDecrease; } else { summedCyclePowers[currentCycle] -= powerDecrease; } ``` The `dbxenftPower` can only be updated with `extraPower` after the `lastStartedCycle` is updated. This means that users have to wait for another active cycle to unstake all their `DXN` tokens. ```solidity function updateDBXeNFT(uint256 tokenId) internal { if ( currentCycle > lastStartedCycleMem && lastFeeUpdateCycle[tokenId] != lastStartedCycleMem + 1 ) { ... uint256 stakedDXN = pendingDXN[tokenId]; if(stakedDXN != 0) { ... pendingDXN[tokenId] = 0; dbxenftPower[tokenId] += extraPower; } lastFeeUpdateCycle[tokenId] = lastStartedCycleMem + 1; } ```

The "DBXen" Contract enables the deposit of `DXN` tokens to participate in the sharing of native token fees. The `cycleFeesPerStakeSummed` value acts as an accumulated index to determine the reward earned by the user between the time of staking and the time the reward is issued. However, there is a concern with the current implementation, as the `cycleFeesPerStakeSummed` value is discrete and only updates for `lastStartedCycle + 1`. This means that if multiple cycles occur without a batch burn, the `lastStartedCycle` will remain unchanged, and the `cycleFeesPerStakeSummed` value for the remaining cycles will be 0. ```solidity=469 cycleFeesPerStakeSummed[lastStartedCycle + 1] = cycleFeesPerStakeSummed[previousStartedCycle] + feePerStake; ``` For instance, if the new cycle starts at 0, then `cycleFeesPerStakeSummed[1]` will have a value, but cycles 2 and 3 will be 0.  This will lead to an incorrect calculation in the `accAccruedFees` update. If the user stakes in an inactive cycle, such as cycle 2 or 3, the value of `cycleFeesPerStakeSummed[accFirstStake[account]]` will always be 0. ```solidity=548 if (lastStartedCycle + 1 > accFirstStake[account]) { accAccruedFees[account] = accAccruedFees[account] + ( (accStakeCycle[account][accFirstStake[account]] * (cycleFeesPerStakeSummed[lastStartedCycle + 1] - cycleFeesPerStakeSummed[accFirstStake[account]] ) ) ) / SCALING_FACTOR; } ```

When the `claimXen()` function is called, it will attempt to invoke the external function `xenft.bulkClaimMintReward()` of the `XENFT` contract. However, the `xenft` variable in the `DBXeNFTFactory` contract is defined as an interface, and the `bulkClaimMintReward()` function is marked as a view function in this interface, which means that it is not allowed to modify any state. ```solidity=619 xenft.bulkClaimMintReward(xenftId, msg.sender); ``` ```solidity interface IXENFT is IERC721{ function vmuCount(uint256 tokenId) external view returns (uint256); function mintInfo(uint256 tokenId) external view returns (uint256); function bulkClaimMintReward(uint256 tokenId, address dest) external view ; } ``` As a result, when the `claimXen()` function tries to call `bulkClaimMintReward()`, it will fail and cause the function to revert due to the calling of a view function that attempts to modify state.

If an owner mints a `DBXeNFT` by locking a redeemed `XeNFT`, they won't be able to stake `DXN` on the newly wrapped `DBXeNFT`. This is because the `mintDBXENFT()` function does not initialize `dbxenftEntryPower[dbxenftId]` for the claimed `XeNFT`, causing the `stake()` function to be reverted at line #447. ```solidity=372 if(redeemed) { baseDBXeNFTPower[dbxenftId] = 1e18; dbxenftPower[dbxenftId] = 1e18; if(currentCycleMem != 0) { lastFeeUpdateCycle[dbxenftId] = lastStartedCycle + 1; } if(currentCycleMem == currentStartedCycle) { summedCyclePowers[currentCycleMem] += 1e18; } else { pendingPower += 1e18; } } else { setUpNewCycle(); dbxenftEntryPower[dbxenftId] = estimatedReward; tokenEntryCycle[dbxenftId] = currentCycleMem; totalEntryPowerPerCycle[currentCycleMem] += estimatedReward; if(currentCycleMem != 0) { lastFeeUpdateCycle[dbxenftId] = lastStartedCycle + 1; } } ``` ```solidity=440 function stake(uint256 amount, uint256 tokenId) external payable nonReentrant onlyNFTOwner(dbxenft, tokenId, msg.sender) { calculateCycle(); updateCycleFeesPerStakeSummed(); updateDBXeNFT(tokenId); require(amount > 0, "DBXen: amount is zero"); uint256 tokenEntryPowerMem = dbxenftEntryPower[tokenId]; require(tokenEntryPowerMem != 0, "DBXeNFT does not exist"); ```

The `calcStakeFee` function uses integer division to calculate the staking fee from the `dxnAmount`. When the `dxnAmount` is less than 1000, integer division results in a `stakeFee` of 0 due to truncation of the fractional part. A malicious actor could potentially exploit this to bypass paying the staking fee. ```solidity function stake() { ... uint256 stakeFee = calcStakeFee(amount); require(msg.value >= stakeFee, "Value less than staking fee"); ... } function calcStakeFee(uint256 dxnAmount) internal pure returns(uint256 stakeFee){ stakeFee = dxnAmount / 1000; } ``` This issue allows an attacker to gain additional power by calling the `stake` function multiple times with an amount of 999 or less, thus bypassing the staking fee entirely.

In the `stake()` function, it would record the stake amount in the future cycle because `cycleToSet` is set incorrectly to current cycle plus one. ```solidity=452 uint256 currentCycleMem = currentCycle; if(currentCycleMem == currentStartedCycle) { cycleAccruedFees[currentCycleMem] += stakeFee; } else { pendingFees += stakeFee; } uint256 cycleToSet = currentCycleMem + 1; if ( (cycleToSet != dbxenftFirstStake[tokenId] && cycleToSet != dbxenftSecondStake[tokenId]) ) { if (dbxenftFirstStake[tokenId] == 0) { dbxenftFirstStake[tokenId] = cycleToSet; } else if (dbxenftSecondStake[tokenId] == 0) { dbxenftSecondStake[tokenId] = cycleToSet; } } dbxenftStakeCycle[tokenId][cycleToSet] += amount; pendingDXN[tokenId] += amount; ```

The `tokenSecondStakeMem` is indeed assigned the value of `dbxenftSecondStake[tokenId]` right before the check, and thus the `dbxenftSecondStake[tokenId]` can be safely replaced with tokenSecondStakeMem in the if-statement. ```solidity=862 uint256 tokenSecondStakeMem = dbxenftSecondStake[tokenId]; if (dbxenftSecondStake[tokenId] != 0) { if (currentCycle > tokenSecondStakeMem) { ``` This change will optimize the code slightly by reducing one SLOAD operation which is quite expensive in terms of gas cost.

There are some cheaper require statements performed after some updates. ```solidity=441 calculateCycle(); updateCycleFeesPerStakeSummed(); updateDBXeNFT(tokenId); require(amount > 0, "DBXen: amount is zero"); ``` ```solidity=503 calculateCycle(); updateCycleFeesPerStakeSummed(); updateDBXeNFT(tokenId); require( amount > 0, "DBXeNFT: Amount is zero." ); ```

The auditing team has the following questions about the project's design: ### Could the team give more details of the variable `dbxenftFirstStake` and variable `dbxenftSecondStake`? The logic for `dbxenftFirstStake` and `dbxenftSecondStake` in the `updateDBXeNFT` function is almost the same. Also, in a subsequent cycle after the first stake, `updateDBXeNFT` is indeed called first before a new stake operation can be made. So, in normal situations, `dbxenftFirstStake` should have already been dealt with (and set to 0 if the current cycle is greater than the stake cycle), before a new stake is processed. Therefore, one `dbxenftFirstStake` could be enough to track the `amount` to update `dbxenftWithdrawableStake`. We would like to discuss with the team to better understand the purpose of `dbxenftFirstStake` and `dbxenftSecondStake`. ### Could the team share more information regarding the mechanism of initial token allocation mechanism of `DXN`? All of the DBXen tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the anonymous deployer can distribute tokens without obtaining the consensus of the community. Any compromise to the deployer account that holds undistributed tokens may allow the attacker to steal and sell tokens on the market, resulting in severe damage to the project. According to the comment in the `DBXenERC20` contract, the entire amount is minted by the main dbxen contract, and the `DBXen.sol` is the contract's owner. But we are uncertain about the specific implementation of the `DBXen` contract. We would like to discuss with the team to better understand the basic functions of the `DBXent` contract.