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

[Bug] No constant BLOCK_TIME with Bullshark #2427

Open
feezybabee opened this issue Apr 7, 2024 · 3 comments
Open

[Bug] No constant BLOCK_TIME with Bullshark #2427

feezybabee opened this issue Apr 7, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@feezybabee
Copy link

https://hackerone.com/reports/2315256

Summary:

In Bullshark the commit interval is not guaranteed. However,BLOCK_TIME is defined as 10 in SnarkVM.

Proof-of-Concept (PoC)

In Bullshark, the commit interval is not guaranteed:

  1. Validators propose batch with best effort, the actual commit inverted is affected by network condition and the number of validators.
  2. If a leader is expired in an even round, it needs to wait for at least another two rounds to commit a subdag.

This means that the commit interval in Bullshark is not constant. Therefore, the real block_interval may vary.

It means that we cannot directly calculate block height from a timestamp. This has a direct impact on calculating coinbase_reward in anchor_block_reward_at_height function. It seems that BLOCK_TIME should be removed.

Impact

In anchor_block_reward_at_height function. We calculate block_height_at_year_10 using the defined BLOCK_TIME:

    // Calculate the block height at year 10.
    let block_height_at_year_10 = block_height_at_year(block_time, 10) as u128;

However, the real block time is unstable and unpredictable. We originally expected the coinbase_reward to decrease to 0 in 10 years. If the real average block time is 5, the coinbase_reward will decrease to 0 in the 5th year.

@feezybabee feezybabee added the bug Something isn't working label Apr 7, 2024
@raychu86
Copy link
Contributor

raychu86 commented May 1, 2024

This is a known concept of our BFT protocol, as blocks time can't necessarily be fixed due to the inherent properties of certificate generation and commits.

These rewards are based on estimates and are not necessarily required to be exactly matching with the observed network block times. In addition, we already have a concept of minimum block times (based on minimum elapsed timestamp between certificates), that we can tune to ensure blocks aren't generated too quickly.

@randomsleep
Copy link
Contributor

@raychu86 The coinbase_reward and ’zero date‘ will fluctuate greatly due to the real block time. For example, if the real block_time doubles, the coinbase_reward will decrease to 0 in 20 years, not 10 years. The actual block_time is affected by many factors (validator count, network condition, etc) and is difficult to estimate.

I would suggest (as described in HackerOne report 2315260):

  • Remove BLOCK_TIME. We still need constant ANCHOR_TIME to make the coinbase_reward stable and predictable. The difficulty adjustment is no longer to make block_time stable (like the previous POW version) but to make ANCHOR_TIME stable.

  • Set longer ANCHOR_TIME (>> average_block_time) to avoid the influence of unstable block_time. Not too long because we need to dynamically match the computation power of the whole network. For example, set ANCHOR_TIME as 10 minutes. Remove NUM_BLOCKS_PER_EPOCH.

  • Within the anchor round, keep coinbase_target and coinbase_reward as constant. Only update coinbase_target at the anchor boundary. Adjusting the coinbase_target according to the previous time taken to reach coinbase_target (like how Bitcoin adjusts the block difficulty). limit the max factor (e.g. to 2) and min factor (e.g. 0.5) to avoid the impact of Bullshark in extreme network conditions. Also, the CoinbasePuzzle should only be updated at anchor boundary.

In this way, we can keep the ANCHOR_TIME stable. The coinbase_reward at each anchor round is in proportion to current_anchor_round_index/total_number_of_anchor_round_in_10_year. Then the total reward will be predictable.

@raychu86
Copy link
Contributor

raychu86 commented May 17, 2024

Update: We are still tracking this issue and plan to run some additional modeling/testing before implementing a change (if a change is deemed necessary).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants