vacp2p / staking-reward-streamer

MIT License
0 stars 0 forks source link

Staking too low balance can cause MP to be zero. #52

Open 3esmit opened 1 month ago

3esmit commented 1 month ago

Issue:

Due to the limitations of Solidity/EVM, which lacks floating-point arithmetic, small staking values combined with frequent MP-accruing function calls can result in zero MP generation. This occurs when accruedMP rounds down to zero, depending on the stake size and time interval between calls. The frequency of calls and the amount staked influence this bug.

As defined in:

https://github.com/vacp2p/staking-reward-streamer/blob/74522424e6e2c9ae45cc1fb8150ae33fa123f24a/src/RewardsStreamerMP.sol#L162

The function to calculate MP is:

\text{accruedMP} = \frac{\text{timeDiff} \cdot \text{totalStaked} \cdot \text{MP\_RATE\_PER\_YEAR}}{365 \, \text{days} \cdot \text{SCALE\_FACTOR}}

When the function is called too frequently, especially for small stakes, accruedMP may round down to zero.

Objective:

To address this, we need to determine the smallest values for which accruedMP = 1.

If we want to keep the minimal stake as 1, we can start by isolating timeDiff to calculate the minimal interval between calls:

\text{timeDiff} = \frac{\text{accruedMP} \cdot 365 \, \text{days} \cdot \text{SCALE\_FACTOR}}{\text{totalStaked} \cdot \text{MP\_RATE\_PER\_YEAR}}

Setting totalStaked = 1 and accruedMP = 1, we get:

x = \frac{1 \cdot 31536000 \cdot 10^{18}}{1 \cdot 10^{18}} = 31,536,000

This result suggests an unrealistic time frame for MP accrual at minimal stake, undermining the "streaming rewards" concept.

Alternative Solution:

A better approach is to calculate the minimal stake required to avoid this issue by isolating totalStaked:

\text{totalStaked} = \frac{\text{accruedMP} \cdot 365 \, \text{days} \cdot \text{SCALE\_FACTOR}}{\text{timeDiff} \cdot \text{MP\_RATE\_PER\_YEAR}}

Using the "minimal block time" of 12 seconds[1] as timeDiff and setting accruedMP = 1, we find:

x = \frac{1 \cdot 31536000 \cdot 10^{18}}{12 \cdot 10^{18}} = 2,628,000 

Therefore, to avoid this bug, the minimal staking value should be 2,628,000 wei. This represents a fraction (0.000000000002628) of an 18-decimal token.

Conclusion:

While the exact impact of this bug is not fully clear, it could disrupt results in specific cases. Setting a minimal stake of 2,628,000 wei should prevent zero MP accruals. This value represents an insignificant fraction (0.000000000002628) of an 18-decimal token and would make no financial difference for users. Token standards typically use 18 decimals precisely to address such cases, ensuring that even very small values can be handled accurately without losing precision.

Additionally, the entire contract should be thoroughly reviewed to ensure similar issues don't occur wherever division operations are used.

[1]: In Ethereum, time is divided into 12-second slots. Each slot selects a validator to propose a block. Under ideal conditions, a block is produced every 12 seconds. Source

0x-r4bbit commented 1 month ago

There should be a test proving this if we end up requiring a min balance of the said amount.

0x-r4bbit commented 3 weeks ago

Using the "minimal block time" of 12 seconds[1] as timeDiff and setting accruedMP = 1, we find:

We need to keep in mind here that the protocol will not be deployed on mainnet but rather on some L2 where the block time might be significantly shortly, meaning we'd need a higher value for the minimum stake allowance.

However that value might be different for different networks. One thing we can do instead, is to throttle the MP accrual update function, such that it'd only allow for accruing at some minimal time interval that is big enough such that we don't run into scenarios where MP get rounded down to 0.

Something along the lines of

uint256 public constant MIN_UPDATE_INTERVAL = 1 minutes;
mapping(address => uint256) public lastUpdateTime;

function _updateAccountMP(address accountAddress) internal {
    Account storage account = accounts[accountAddress];

    if (block.timestamp - account.lastMPUpdateTime < MIN_UPDATE_INTERVAL) {
        return;
    }
    // ... 
}