Metaverse User DAO - Addendum

In the contract `MudAngelRoundReleaseBank` the role `admin` has authority over the functions shown below: * icoDeposits(): deposit `MUD` tokens to investor. * icoFinalised(): mark the `ICO` finished flag, stop `icoDeposit`. Any compromise to the `admin` account may allow the hacker to take advantage of this authority and do the following: * Deposit `MUD` tokens to themselves or block `ICO`. In the contract `MudTeamReleaseBank` the role `admin` has authority over the functions shown below: * teamMemberDeposit(): deposit `MUD` tokens to team members, lock the team's `MUD` tokens to the contract and start to release after 365 days in daily release rate. * icoFinalised(): mark the `ICO` finished flag, stop `icoDeposit`. Any compromise to the `admin` account may allow the hacker to take advantage of this authority and do the following: * Deposit `MUD` tokens to themselves or block `ICO`. In the contract `MudMiningPool` the role `admin` has authority over the functions shown below: * icoDeposit(): deposit `MUD` tokens to the pool. Any compromise to the `admin` account may allow the hacker to take advantage of this authority and do the following: * Refuse deposit `MUD` tokens. In the contract `MudMiningPool` the role `miningDappAddress` has authority over the functions shown below: * miningStart(): start mining process. * miningSettlement(): settle mining tokens. Any compromise to the `miningDappAddress` may allow the hacker to take advantage of this authority and do the following: * Start mining process at any time as they want. * Settle tokens to themselves.

In the function `miningSettlement()` of contract `MudMiningPool`, the `dailyMiningLimit` is just checked whether it should be halved only in the last settlement batch. If current settlement is divided into 5 batches (1,2,3,4,0), and it has been 4 years since the last `lastHalvingTime`, according to the logic, only in the last batch (batchNumber = 0) the `dailyMiningLimit` will be halved. In this case, it should be halved starting from batch #1. Besides, the half time could happen in any batch during a settlement.

Accessing storage variables in a loop can be costly in terms of gas consumption. ```solidity=64 _icoDepositTotal = _icoDepositTotal + balanceArray[i]; ``` --- ```solidity=65 _icoDepositTotal = _icoDepositTotal + balanceArray[i]; ``` --- ```solidity=116 _totalSettlementAmount = _totalSettlementAmount + balanceArray[i]; ```

Performing integer division before multiplication truncates the low bits, losing the precision of calculation. ```solidity=102 dailyMiningLimit = dailyMiningLimit / 2; ``` ```solidity=110 uint256 settlementLimit = dailyMiningLimit * settlementPeriod; ```

According to the `require` conditions in the functions `checkBalance()` and `withdraw()`, `miningDappAddress` is also excluded from this ico. Therefore, the mining address should be checked for `miningDappAddress` in the settlement loop. ```solidity for (uint i = 0; i < addressArray.length; i++) { require(addressArray[i] != admin && addressArray[i] != address(0), "invalid address"); require(balanceArray[i] > 0); _totalSettlementAmount = _totalSettlementAmount + balanceArray[i]; require(_totalSettlementAmount <= settlementLimit, "TotalAmount out of settlement limit!"); // > daily limit, trasaction failed. _minedToken[addressArray[i]] = _minedToken[addressArray[i]] + balanceArray[i]; } ```

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

In the function `miningSettlement()` of contract `MudMiningPool`, the `batchNumber` is not validated. In the same settlement, `batchNumber` should be starting from 1 and increasing by 1 in subsequent batches, then ending at zero. But there is no logic to check whether the input `batchNumber` is passed correctly, for example, duplicate `batchNumber` for the same settlement in different batches.

In the contract `MudAngelRoundReleaseBank`, investors or admin are required to call function `checkBalance()` to check their locked `MUD` token amount after the `lastTIme`. ```solidity=88 require(block.timestamp > bank[addressToCheck].lastTime, "now time should > lastTime"); ``` If admin doesn't deposit MUD for `addressToCheck`, `bank[addressToCheck].lastTime` is equal to 0; once deposited, `bank[addressToCheck].lastTime` is just the time in the past, the condition in `require` statement can be true. The only timestamp it reverts is just the calling time is equal to depositing time, but this happens rarely. Even though it does, the remaining code can cover this and return `(0,0)`.

State variables that never change should be declared as `constant` to save gas. --- ```solidity=22 uint256 settlementPeriod = 7; //7 days ``` - `settlementPeriod` should be declared `constant`.