[ETCM-193] Make target time between blocks configurable #773
Conversation
| * [0.5 min, 1.5 min) => difficulty stays the same (the average should be powTargetTime) | ||
| * [1.5 min, +infinity) => difficulty decreases | ||
| */ | ||
| val LowerBoundExpectedRatio: Long = powTargetTime / 2 |
There was a problem hiding this comment.
thinking about it now, I did too simple assumption here. WDYT guts, should we configure also the deviation of target time or we should assume that deviation is X % of target time?
There was a problem hiding this comment.
Hmm I'm not sure if this deviation will bother us too much, neither if it's easy to change this formulas without screwing up... I wouldn't mind leaving it as is for now
...main/scala/io/iohk/ethereum/consensus/ethash/difficulty/ConfiguredDifficultyCalculator.scala
Outdated
Show resolved
Hide resolved
...main/scala/io/iohk/ethereum/consensus/ethash/difficulty/ConfiguredDifficultyCalculator.scala
Outdated
Show resolved
Hide resolved
| * [0.5 min, 1.5 min) => difficulty stays the same (the average should be powTargetTime) | ||
| * [1.5 min, +infinity) => difficulty decreases | ||
| */ | ||
| val LowerBoundExpectedRatio: Long = powTargetTime / 2 |
There was a problem hiding this comment.
Hmm I'm not sure if this deviation will bother us too much, neither if it's easy to change this formulas without screwing up... I wouldn't mind leaving it as is for now
pslaski
force-pushed
the
etcm-193-target-time-between-blocks-configurable
branch
from
b8f3eff to
ce7df7f
Compare
| * [0.5 min, 1 min) => difficulty stays the same (the average should be powTargetTime) | ||
| * [1 min, +infinity) => difficulty decreases | ||
| */ | ||
| private val LowerBoundExpectedRatio: Long = (powTargetTime / 1.5).toLong |
There was a problem hiding this comment.
A nazi comment: capitalised names are for constants and this is not a constant if it depends on the class parameter
| import io.iohk.ethereum.consensus.difficulty.DifficultyCalculator | ||
| import io.iohk.ethereum.domain.BlockHeader | ||
|
|
||
| class ConfiguredDifficultyCalculator(powTargetTime: Long) extends DifficultyCalculator { |
There was a problem hiding this comment.
Not fond of the name, EthashDiffCalc is also configured. Maybe TargetTimeDiffCalc or simply QaDiffCalc?
There was a problem hiding this comment.
(I don't like QaDiffCalc as this will be used on our testnet and I'm not sure for how long, not just testing purposes)
| val x: BigInt = parentHeader.difficulty / DifficultyBoundDivision | ||
| val c: BigInt = math.max(1 - (timestampDiff / LowerBoundExpectedRatio), FrontierTimestampDiffLimit) | ||
|
|
||
| MinimumDifficulty.max(parentHeader.difficulty + x * c) |
There was a problem hiding this comment.
How did we arrive at this formula? I would have expected it to be parameterised by something like:
- mu - the expected block time
- sigma - std deviation, where we don't change the diff
- delta - rate of change outside of std deviation range
Perhaps something like: https://en.wikipedia.org/wiki/Beta_distribution could be used. The formula we have is eerily similar ETH formula, but I think we are allowed more freedom with this, no?
Disclaimer: these are very rough proposals and are not to be taken word-for-word. But I think something along those lines would give us more control. It's not a blocker if such control is not required.
There was a problem hiding this comment.
This was inspired by what I had done on this regard on the other project so I'll share my thoughts. As you said this formula is the ETH formula with simplifications, I didn't want to deviate too much from it just in case I'd break something (for example, if we are more aggressive with when to increase/decrease difficulty it might lead to an divergent time between blocks).
Either way, the only reason I see for maintaining the current formula is so that this doesn't become a blocker for deploying checkpointing on our testnet, if that isn't at risk we could go for another better one
(of course, if this formula doesn't work as expected we should definitely change it!)
There was a problem hiding this comment.
I would go now with the current formula for three reasons: 1. we need this fast for testnet deployment 2. It is the same formula used in other project and its upgraded version is in mantis now. 3. Developing new formula only for testnet without having in mind using it in mainnet is not worth a time now imho.
Maybe we can think about it in the future? (new task)
There was a problem hiding this comment.
I would try it out on the testnet and only create a follow-up task if we detect there that's not enough (or that it would be good to have something better)
src/main/scala/io/iohk/ethereum/consensus/ethash/difficulty/EthashDifficultyCalculator.scala
Outdated
Show resolved
Hide resolved
pslaski
force-pushed
the
etcm-193-target-time-between-blocks-configurable
branch
4 times, most recently
from
f61f4f5 to
756c797
Compare
pslaski
force-pushed
the
etcm-193-target-time-between-blocks-configurable
branch
from
756c797 to
b20ae80
Compare
Description
Due to OBFT constraints, our internal testnet will have a higher target time between blocks than ETC, so we should make the block rate configurable for changing it on the different environments.
Proposed Solution
Introduced new Difficulty calculator which will try to compute difficulty to target configured PoW time
Important Changes Introduced
pow-target-time(default is null which means that normal ethash calculator will be used)