[Bug] No constant `BLOCK_TIME` with Bullshark
Closed this issue · 6 comments
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:
- Validators propose batch with best effort, the actual commit inverted is affected by network condition and the number of validators.
- 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.
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.
@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 constantANCHOR_TIME
to make thecoinbase_reward
stable and predictable. The difficulty adjustment is no longer to makeblock_time
stable (like the previous POW version) but to makeANCHOR_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, setANCHOR_TIME
as 10 minutes. RemoveNUM_BLOCKS_PER_EPOCH
. -
Within the anchor round, keep
coinbase_target
andcoinbase_reward
as constant. Only updatecoinbase_target
at the anchor boundary. Adjusting thecoinbase_target
according to the previous time taken to reachcoinbase_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.
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).
As @raychu86 says, this is a known issue, and I don't see us blocking mainnet for it. Marking as such. Feel free to advocate.
The coinbase reward structure has been updated and will no longer go to 0 at year 10. Which should mitigate concerns regarding token emissions.
In addition, there is not much we can do regarding the time intervals between blocks due to the inherent nature of the Bullshark BFT protocol, so the unpredictable nature is something we must accept with our constant values here acting as estimates. After mainnet launch and if the need arises, the validators/governance process can revisit adjusting these values to match the new network conditions.
Closing this Issue for now, especially since some of the possible mitigations are also related to a separate finding - #2426.
@randomsleep, we greatly value your input, so feel free to reopen if you have any strong concerns.
We've considered updating the retargeting algorithm such that it is dependent on block height rather than block time. At a high level, the existing algorithm can be adopted by using a timestamp proportional to the anchor_time
times the block_height
since the last coinbase target.
Such a solution would need to be thoroughly validated and approved through governance.