DDDX is a decentralized staking farm protocol where users can participate as depositors and voters.

1. Voting power of each user is decreasing gradually, the project has to record snapshots at epoch time to distribute rewards.
2. Voting weights(BaseV1-voter) and derived balance(BaseV1-gauge) do not follow the voting power changes in real-time.
3. Calculation of rewards in `BaseV1-gauge` are rough, based on snapshots of amount and perTokenStored that do not correspond in time.
4. Users can cast negative votes for the pools that they do not participate in to intensify user competition.

   
Due to the abovementioned points, the protocol will have a potential distribution issue.

Financial models of blockchain protocols need to be resilient to attacks. It needs to pass simulations and verifications to guarantee the security of the overall protocol. 


Financial Models 

In the contract `BaseV1Minter` the role `initializer` has authority over the functions shown in the diagram below.

Any compromise to the `initializer` account may allow the hacker to take advantage of this authority and bring unpredictable damages to the project.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-BaseV1Minter-initializer.svg)

Centralization Risk in BaseV1-minter.sol

Incorrect return values for ERC-20 functions. A contract compiled with Solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.

ve (BaseV1-minter.sol#10-15) has incorrect ERC20 function interface:ve.transferFrom(address,address,uint256) (BaseV1-minter.sol#14)

File: contracts/BaseV1-minter.sol (Line 14, Contract `ve`)
```Solidity
    function transferFrom(address, address, uint) external;
```



Incorrect ERC-20 Interface

The value of `_ve.totalSupply()` represents for the total voting power rather than the locked supply.

Logical issue of function `_ve.totalSupply`

From the comments, the calculation should be based on `tail_emission * circulating_supply()`. However, it is calculated by `weekly * emission * circulating_supply() / target_base / _token.totalSupply()`.

Logical issue of function calclulate_emission()

In the contract `BaseV1` the role `minter` has authority over the functions shown in the diagram below.

Any compromise to the `minter` account may allow the hacker to take advantage of this authority and mint tokens for arbitrary accounts.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-BaseV1-minter.svg)

Centralization Risk in BaseV1-token.sol

100000e18 tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute tokens without obtaining the consensus of the community.


Initial Token Distribution

In the contract `Bribe` the role `factory` has authority over the functions shown in the diagram below.

Any compromise to the `factory` account may allow the hacker to take advantage of this authority and bring unpredictable damages to the project.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-Bribe-factory.svg)

Centralization Risk in BaseV1-bribes.sol

In the contract `Gauge` the role `voter` has authority over the functions shown in the diagram below.

Any compromise to the `voter` account may allow the hacker to take advantage of this authority and bring unpredictable damages to the project.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-Gauge-voter.svg)

Centralization Risk in BaseV1-gauges.sol

1. The function `deposit()` does not strictly forbid multi-veNFT deposits. But if the user trys to deposit for the second veNFT, he/she cannot bind change the tokenId. And the adjusted part of his/her balance is still based on the initial tokenId. We recommend the team disallow multi-veNFT investments from the process aspect.
   
2. No matther how many tokens the user withdraw from the contract, function `withdrawToken()` will always clear the binding of tokenId to the user. And if tokenId is 0, the function won't clear the binding.

Logical issue about attachments of veNFT

In the contract `BaseV1Factory` the role `pauser` has authority over the functions shown in the diagram below.

Any compromise to the `pauser` account may allow the hacker to take advantage of this authority.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-BaseV1Factory-pauser.svg)

In the contract `BaseV1Factory` the role `pendingPauser` has authority over the functions shown in the diagram below.

Any compromise to the `pendingPauser` account may allow the hacker to take advantage of this authority.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-BaseV1Factory-pendingPauser.svg)



Centralization Risk in BaseV1-core.sol

The protocol is using some algorithms, including in the logic of the functions `swap()` and `getAmountOut`.

The project introduces a new curve: $X^3*Y + Y^3*X = K$ to support efficient stable swaps.

The mathematical verification of these algorithms is not in the scope of this audit. The function logic will be checked based on the requirement documents.

Mathematical verification

The contract `BaseV1-core` is an autonomous smart contract deployed on a blockchain that makes markets between different assets

This class of AMM like `Uniswap` suffers from what is commonly referred to as Impermanent Loss, which we have previously identified as the Gamma component of the associated self-financing trading strategy and which is the risk that LP providers wager against potential fee earnings.


Impermanent Loss 

In the contract `BaseV1Voter` the role `isGauge` has authority over the functions shown in the diagram below.

Any compromise to the `isGauge` account may allow the hacker to take advantage of this authority.

![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-BaseV1Voter-isGauge.svg)



Centralization Risk in BaseV1-voter.sol

The calculation of `listing_fee` is as below:
```solidity
(erc20(base).totalSupply() - erc20(_ve).totalSupply()) / 200
``` 
The first is total supply of erc20 token and the second is 5‰ of the total voting power.
The erc20 base is a third-party protocol. We treat the protocol as black boxes and assume their functional correctness. 

Logical issue on listing_fee()

The sum of _poolWeight is not greater than _weight. Because the sum of values is certainly not greater than sum of their absolute values. So if the weight of the gauge is negative or the address is not gauge, the voting power of the NFT can not be used fully.

Logical issue of _vote()

The function poke() allows the user to update the vote weights according to the new voting power. However, users may only poke when his/her voting power is greater and not poke when his voting power decreases gradually.

The new vote weights can be insistent with the voting power. It increases the difficulty of equitable distribution. 

Logical issue on function poke()

Users can cast negative votes for pools, which allows users to be hostile to each other. Large investors can short other pools' rewards. This makes the investment project unstable.

Logical issue of negative votes

The function does not change any state variable for `msg.sender` or the account, and just emit the event. It's not recommended to skip redundancy check for the (gauge, account, tokenId) relation.

Logical issue of function `attach` and `detach`

It seems that _votes can be a negative number. But only when processing totalWeight, its absolute value is used. Both `weights[_pool]` and `votes[_tokenId][_pool]` directly subtract the value. We would like to confirm the meaning of a negative `_votes` and if the current implementation aligns with the original project design.

Logical issue on _votes in function _reset()

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Improper Usage of `public` and `external` Type

The linked statements do not affect the functionality of the codebase and appear to be either leftovers from test code or older functionality.

Redundant Code Components

Same as what is mentioned in the code comments, the `earned` function just provides the estimation reward amount. The function earned() iterates all the users' checkpoints and roughly uses both sides of `perTokenStred` to calculate the rewards. Each time the user deposit, withdraw, getReward, the functions will generate user checkpoints and supply checkpoints. Each user checkpoint corresponds to a supply checkpoint. So there will be a huge amount of supply checkpoints, it may exist out of gas errors when iterating all the supply checkpoints in function `_updateRewardPerToken()`.

Because derivedBalance varies with the balance of veNFT, and the balance of veNFT is decreasing, users can deposit, withdraw and claim rewards to make the devivedBalance change. However, if the number of samples is not enough, the balance of the user does not reflect the gradually decreasing balance but changes dramatically to another value at a certain time. Those who have not triggered balance changes can maintain a higher share for a longer period of time to get more rewards.

This could potentially lead to a reward distribution issue, insufficient rewards, etc, which are all severe problems.

Logical issue in function earned

The voting power of the user's veNFT is gradually decreasing. This is reflected in ve.sol and ve_dist.sol. 

In the contract `BaseV1-minter`, the vote weights mechanism for distributing pool rewards is seperate from the voting power itself. Users have to manually trigger a poke or re-vote to get a copy of the voting power.

In the contract `BaseV1-gauges`, the `derivedBalance` is Math.min(40% * _balance + totalSupply * _adjusted / _supply, _balance). It means that the user can get a minimum of 40% of the staking power, the remaining 60% is made up by the share of voting power. And it's still a copy of the voting power, not accurate enough to support accurate allocation.

Logical issue of decreasing votingPower

Using strict equality to check a volatile numeric value such as Ether/token balance, time, and block number is risky, because such a value can be manipulated by an attacker.

File: contracts/BaseV1-bribes.sol (Line 163, Function `Bribe.getPriorSupplyIndex`)
```solidity
            if (cp.timestamp == timestamp) {
```


---
File: contracts/BaseV1-bribes.sol (Line 210, Function `Bribe._writeCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && checkpoints[tokenId][_nCheckPoints - 1].timestamp == _timestamp) {
```


---
File: contracts/BaseV1-bribes.sol (Line 221, Function `Bribe._writeRewardPerTokenCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && rewardPerTokenCheckpoints[token][_nCheckPoints - 1].timestamp == timestamp) {
```


---
File: contracts/BaseV1-bribes.sol (Line 233, Function `Bribe._writeSupplyCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && supplyCheckpoints[_nCheckPoints - 1].timestamp == _timestamp) {
```


---
File: contracts/BaseV1-bribes.sol (Line 300, Function `Bribe._batchRewardPerToken`)
```solidity
        if (rewardRate[token] == 0) {
```


---
File: contracts/BaseV1-bribes.sol (Line 334, Function `Bribe._updateRewardPerToken`)
```solidity
        if (rewardRate[token] == 0) {
```


---
File: contracts/BaseV1-bribes.sol (Line 454, Function `Bribe._safeTransfer`)
```solidity
        require(success && (data.length == 0 || abi.decode(data, (bool))));
```


---
File: contracts/BaseV1-core.sol (Line 298, Function `BaseV1Pair.current`)
```solidity
        if (block.timestamp == _observation.timestamp) {
```


---
File: contracts/BaseV1-core.sol (Line 352, Function `BaseV1Pair.mint`)
```solidity
        if (_totalSupply == 0) {
```


---
File: contracts/BaseV1-core.sol (Line 580, Function `BaseV1Pair._safeTransfer`)
```solidity
        require(success && (data.length == 0 || abi.decode(data, (bool))));
```


---
File: contracts/BaseV1-gauges.sol (Line 227, Function `Gauge.getPriorSupplyIndex`)
```solidity
            if (cp.timestamp == timestamp) {
```


---
File: contracts/BaseV1-gauges.sol (Line 274, Function `Gauge._writeCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && checkpoints[account][_nCheckPoints - 1].timestamp == _timestamp) {
```


---
File: contracts/BaseV1-gauges.sol (Line 285, Function `Gauge._writeRewardPerTokenCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && rewardPerTokenCheckpoints[token][_nCheckPoints - 1].timestamp == timestamp) {
```


---
File: contracts/BaseV1-gauges.sol (Line 297, Function `Gauge._writeSupplyCheckpoint`)
```solidity
        if (_nCheckPoints > 0 && supplyCheckpoints[_nCheckPoints - 1].timestamp == _timestamp) {
```


---
File: contracts/BaseV1-gauges.sol (Line 343, Function `Gauge.rewardPerToken`)
```solidity
        if (derivedSupply == 0) {
```


---
File: contracts/BaseV1-gauges.sol (Line 374, Function `Gauge._batchRewardPerToken`)
```solidity
        if (rewardRate[token] == 0) {
```


---
File: contracts/BaseV1-gauges.sol (Line 408, Function `Gauge._updateRewardPerToken`)
```solidity
        if (rewardRate[token] == 0) {
```


---
File: contracts/BaseV1-gauges.sol (Line 509, Function `Gauge.withdraw`)
```solidity
        if (amount == balanceOf[msg.sender]) {
```


---
File: contracts/BaseV1-gauges.sol (Line 580, Function `Gauge._safeTransfer`)
```solidity
        require(success && (data.length == 0 || abi.decode(data, (bool))));
```


---
File: contracts/BaseV1-gauges.sol (Line 587, Function `Gauge._safeTransferFrom`)
```solidity
        require(success && (data.length == 0 || abi.decode(data, (bool))));
```


---
File: contracts/ve.sol (Line 1142, Function `ve.balanceOfNFT`)
```solidity
        if (ownership_change[_tokenId] == block.number) return 0;
```


---
File: contracts/ve.sol (Line 1224, Function `ve._supply_at`)
```solidity
            if (t_i == t) {
```


---
File: contracts/ve.sol (Line 1289, Function `ve.toString`)
```solidity
        if (value == 0) {
```


---
File: contracts/ve.sol (Line 774, Function `ve._checkpoint`)
```solidity
                if (new_locked.end == old_locked.end) {
```


---
File: contracts/ve.sol (Line 825, Function `ve._checkpoint`)
```solidity
                if (t_i == block.timestamp) {
```


---
File: contracts/ve.sol (Line 860, Function `ve._checkpoint`)
```solidity
                if (new_locked.end == old_locked.end) {
```


---
File: contracts/ve_dist.sol (Line 103, Function `ve_dist._checkpoint_token`)
```solidity
                if (since_last == 0 && block.timestamp == t) {
```


---
File: contracts/ve_dist.sol (Line 110, Function `ve_dist._checkpoint_token`)
```solidity
                if (since_last == 0 && next_week == t) {
```


---
File: contracts/ve_dist.sol (Line 204, Function `ve_dist._claim`)
```solidity
        if (week_cursor == 0) {
```


---
File: contracts/ve_dist.sol (Line 214, Function `ve_dist._claim`)
```solidity
        if (week_cursor == 0) week_cursor = (user_point.ts + WEEK - 1) / WEEK * WEEK;
```


---
File: contracts/ve_dist.sol (Line 234, Function `ve_dist._claim`)
```solidity
                if (balance_of == 0 && user_epoch > max_user_epoch) break;
```


---
File: contracts/ve_dist.sol (Line 261, Function `ve_dist._claimable`)
```solidity
        if (week_cursor == 0) {
```


---
File: contracts/ve_dist.sol (Line 271, Function `ve_dist._claimable`)
```solidity
        if (week_cursor == 0) week_cursor = (user_point.ts + WEEK - 1) / WEEK * WEEK;
```


---
File: contracts/ve_dist.sol (Line 291, Function `ve_dist._claimable`)
```solidity
                if (balance_of == 0 && user_epoch > max_user_epoch) break;
```



Dangerous Strict Equality

If the user does not come in at the exact epoch time, the voting power before the epoch time is lost in vain. Especially when the lock-up time is short, it is more obvious.

If the lock is only for one week and just after the last epoch, then the percentage of his/her voting power involved in the current distribution is almost 0.

Logical issue about voting power

Addresses should be checked before assignment or external call to make sure they are not zero addresses.

File: contracts/BaseV1-bribes.sol (Line 90, Function `Bribe.constructor`)
```solidity
        factory = _factory;
```

File: contracts/BaseV1-bribes.sol (Line 91, Function `Bribe.constructor`)
```solidity
        _ve = IBaseV1Voter(_factory)._ve();
```
- `_factory` is not zero-checked before being used.


---
File: contracts/BaseV1-core.sol (Line 147, Function `BaseV1Pair.constructor`)
```solidity
        (token0, token1, stable) = (_token0, _token1, _stable);
```

File: contracts/BaseV1-core.sol (Line 148, Function `BaseV1Pair.constructor`)
```solidity
        fees = address(new BaseV1Fees(_token0, _token1));
```
- `_token0` is not zero-checked before being used.


---
File: contracts/BaseV1-core.sol (Line 45, Function `BaseV1Fees.constructor`)
```solidity
        token0 = _token0;
```
- `_token0` is not zero-checked before being used.


---
File: contracts/BaseV1-core.sol (Line 46, Function `BaseV1Fees.constructor`)
```solidity
        token1 = _token1;
```
- `_token1` is not zero-checked before being used.


---
File: contracts/BaseV1-core.sol (Line 611, Function `BaseV1Factory.setPauser`)
```solidity
        pendingPauser = _pauser;
```
- `_pauser` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 123, Function `Gauge.constructor`)
```solidity
        stake = _stake;
```
- `_stake` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 124, Function `Gauge.constructor`)
```solidity
        bribe = _bribe;
```
- `_bribe` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 125, Function `Gauge.constructor`)
```solidity
        _ve = __ve;
```
- `__ve` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 126, Function `Gauge.constructor`)
```solidity
        voter = _voter;
```
- `_voter` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 602, Function `BaseV1GaugeFactory.createGauge`)
```solidity
        last_gauge = address(new Gauge(_pool, _bribe, _ve, msg.sender));
```
- `_bribe` is not zero-checked before being used.


---
File: contracts/BaseV1-gauges.sol (Line 607, Function `BaseV1GaugeFactory.createGaugeSingle`)
```solidity
        last_gauge = address(new Gauge(_pool, _bribe, _ve, _voter));
```
- `_bribe` is not zero-checked before being used.


---
File: contracts/BaseV1-periphery.sol (Line 76, Function `BaseV1Router01.constructor`)
```solidity
        factory = _factory;
```
- `_factory` is not zero-checked before being used.


---
File: contracts/BaseV1-token.sol (Line 27, Function `BaseV1.setMinter`)
```solidity
        minter = _minter;
```
- `_minter` is not zero-checked before being used.


---
File: contracts/BaseV1-voter.sol (Line 100, Function `BaseV1Voter.constructor`)
```solidity
        factory = _factory;
```
- `_factory` is not zero-checked before being used.


---
File: contracts/BaseV1-voter.sol (Line 102, Function `BaseV1Voter.constructor`)
```solidity
        gaugefactory = _gauges;
```
- `_gauges` is not zero-checked before being used.


---
File: contracts/BaseV1-voter.sol (Line 103, Function `BaseV1Voter.constructor`)
```solidity
        bribefactory = _bribes;
```
- `_bribes` is not zero-checked before being used.


---
File: contracts/BaseV1-voter.sol (Line 121, Function `BaseV1Voter.initialize`)
```solidity
        minter = _minter;
```
- `_minter` is not zero-checked before being used.


---
File: contracts/BaseV1-voter.sol (Line 99, Function `BaseV1Voter.constructor`)
```solidity
        _ve = __ve;
```

File: contracts/BaseV1-voter.sol (Line 101, Function `BaseV1Voter.constructor`)
```solidity
        base = ve(__ve).token();
```
- `__ve` is not zero-checked before being used.


---
File: contracts/ve.sol (Line 407, Function `ve.constructor`)
```solidity
        token = token_addr;
```
- `token_addr` is not zero-checked before being used.


---
File: contracts/ve.sol (Line 926, Function `ve.setVoter`)
```solidity
        voter = _voter;
```
- `_voter` is not zero-checked before being used.


---
File: contracts/ve_dist.sol (Line 345, Function `ve_dist.setDepositor`)
```solidity
        depositor = _depositor;
```
- `_depositor` is not zero-checked before being used.


---
File: contracts/ve_dist.sol (Line 79, Function `ve_dist.constructor`)
```solidity
        token = _token;
```
- `_token` is not zero-checked before being used.


---
File: contracts/ve_dist.sol (Line 80, Function `ve_dist.constructor`)
```solidity
        voting_escrow = _voting_escrow;
```
- `_voting_escrow` is not zero-checked before being used.



Missing Zero Address Validation

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.

Missing Error Messages

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

Missing Emit Events

Since the reward is calculated based on the number of periods(weeks), it's better to allocate rewards to these weeks on average but not accurate to the second. It may result in twice updates in the reward of the weeks and lead to confusion in reward distribution.

Logical issue on _checkpoint_token()

It's hard to distribute rewards based on the timestamp. It's recommended to just distribute rewards for a single week by receiving the week parameter, or by default distribute the next week's rewards to avoid duplication.

Logical issue on the design of _checkpoint_token()

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.

Variables That Could Be Declared as Immutable

It's not recommended to claim based on timestamp. It's better to claim based on the week number and only allow one claim for one call to reduce complexity.

Logical issue on the design of claim mechanism

The codes of function `_claim()` covers function `_claimable`.

Redundant codes

The variable `slope_changes` is a reflection of users' changes of vesting schedules. But users are only required to lock money for whole weeks. It still can not match the update slots of point_history. So the record of supply becomes meaningless because it does not change in time or even not change at all.

Because the whole records are based on strict and accurate calculations. It can be a very severe problem.

Logical issue about updates of point_history

In the contract `ve` the role `voter` has authority over the functions shown in the diagram below.

Any compromise to the `voter` account may allow the hacker to take advantage of this authority and bring unpredictable damages to the project.
![](https://accelerator-tasks-prod.s3.amazonaws.com/cd4cebdaeec043b78dd3e0c06b45eef4/diagrams/centralization-ve-voter.svg)

Centralization Risk in ve.sol

The block recorded in the point history may be inaccurate, since the history block is calculated by `block_slope * past_time`. There will be bias due to the division accuracy and the acquisition time point within the block.

Inaccurate block record

From the code we can see, the team is not fully confident with the control of updating point history. But it represents the total voting power. After all, any actions that make its recording inaccurate should be avoided.

Logical issue on updates of point_history

Two `Transfer` events are emitted in constructor of the contract `ve`. But the corresponding operations are not performed here.

Redundant logs in constructor of contract ve

The voting power decreases from the moment when the lockout begins. The decay rate is the same for NFTs with different durations and the same amount. 

This type of calculation of voting power is not beneficial to the use of voting power. It makes it very hard to calculate the share of one NFT token's voting power in the whole total voting power. So the project has to record all the checkpoints, no matter that of the total voting power or that of one certain NFT, which may generate many unexpected errors due to accuracy, or some errors due to robust processing.

Logical issue on calculation of voting power

The current implementation complicates the logic, the check `new_locked.end > old_locked.end` seems not logical and is actually is a fix for the above judgment `new_locked.end == old_locked.end`. And the code relies on the mechanism that users can only maintain or delay the lock end time.

The check in the processing of `slope_changes[new_locked.end]` should be `new_locked.end != old_locked.end`.

DDDX.io

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

41

0

26

0

13