Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
3esmit opened this issue Oct 12, 2024 · 3 comments
Open

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

3esmit opened this issue Oct 12, 2024 · 3 comments

Comments

@3esmit
Copy link
Contributor

3esmit commented Oct 12, 2024

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:

uint256 accruedMP = (timeDiff * totalStaked * MP_RATE_PER_YEAR) / (365 days * SCALE_FACTOR);

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
Copy link
Collaborator

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

@0x-r4bbit
Copy link
Collaborator

0x-r4bbit commented Oct 24, 2024

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;
    }
    // ... 
}

@0x-r4bbit
Copy link
Collaborator

@3esmit @gravityblast can we reach consensus on what to do here?

I think all suggested solutions to this (theoretical) problem are far from ideal.
I think the impact and likelyhood of this is fairly low.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

2 participants