Skip to content

Conversation

@domob1812
Copy link

@domob1812 domob1812 commented Nov 4, 2020

This refactors BIP30-style fork activation (signalling with block versoins): Instead of a function IsSuperMajority, we now have a utility class ActivationManager that takes care of both the state of individual forks (for now, just a TestDummy fork is defined) as well as of enforcing minimal block versions as the network updates.

Strictly speaking this is by itself already a soft fork that activates automatically: With this logic, the network will enforce any arbitrary block version as minimum that has been seen in at least 950 of the last 1'000 blocks; previously that was only the case for specifically scheduled block versions corresponding to implemented forks. This is irrelevant in practice, though.

Since DIVI started the network already with block version 4 and various BIPs enforced from the start (BIP16, BIP30, BIP34, BIP66), we can also remove obsolete / always-on "conditional" activation for those to simplify the code.

@domob1812
Copy link
Author

I've successfully synced DIVI from scratch on mainnet with this PR now (so the various changes did not affect compatibility especially to the very first blocks).

@galpHub
Copy link

galpHub commented Nov 4, 2020

Looks ok. Is there a reason you added the commit for the staking vault checks in this PR ? I'm a bit confused by that

@domob1812
Copy link
Author

Looks ok. Is there a reason you added the commit for the staking vault checks in this PR ? I'm a bit confused by that

The main reason is that I believe both of these changes to be quick to review and push, and so I combined them. If you prefer to review & push the checks as a separate PR, I'm happy to spit it up.

@domob1812
Copy link
Author

I've removed the coinstakes checking commit from #38 now.

@domob1812 domob1812 force-pushed the activation-manager branch 3 times, most recently from bc05a32 to afc688a Compare November 11, 2020 12:57
@domob1812 domob1812 force-pushed the activation-manager branch 3 times, most recently from f61f174 to a4ef70a Compare November 24, 2020 13:49
DIVI has some of the softforks of Bitcoin (BIP16, BIP30, BIP34, BIP66)
on all the time since genesis.  This change removes the conditional
activation for them (which is not necessary).

Since this also makes all other BIP30 things obsolete (in particular,
IsSuperMajority and the related constants in chain params), those can
be removed as well.
This adds the new ActivationState utility class.  It is used to
abstract fork activation away from other parts of the code.  An
instance is based on a CBlockIndex representing the chain tip being
validated, and it can be queried for the state of various forks
with IsActive.

Activation happens by block timestamp, as this is totally straight-forward
and more or less the simplest form of activation possible.  This also
makes it independent of the block context (unlike e.g. median time)
and allows us to theoretically use fork activation even for things like
changing the serialisation format of blocks.

For now, no fork is defined apart from a dummy one used in testing.
In the future, we will define real forks with this framework (e.g. the
staking-vault changes).
@domob1812
Copy link
Author

We'll use activation based on a timestamp instead to simplify this, as in #55.

@domob1812 domob1812 closed this Nov 24, 2020
@domob1812 domob1812 deleted the activation-manager branch November 24, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants