Remove sighash fork value (forkid)
Addresses #432 (closed).
The 'forkid' feature isn't used and could be removed completely. Nobody seems to currently see a use for it, and any chain that forks from Bitcoin Cash would be able to implement its own (maybe better) replay protection scheme if it wanted.
Disambiguation - "ForkID" is used to describe the signature hash type which is used after UAHF, so the majority of references in the code to ForkID will remain - it's necessary to be able to distinguish between sighashes with and without a fork ID. The value of the fork ID itself (referred in the code as a "fork value") is an arbitrary uint32_t
which has always been zero. This MR removes the potential to set this to anything other than zero.
The amount of unused code that can be removed while maintaining current behaviour is quite small, and relates only to the "fork value". The only way to interact with the fork value presently is through a pair of mutation functions in sighash, and these are called nowhere outside of their own tests.
Changes
- Remove
SigHashType
's fork value functions:withForkValue()
getForkValue()
- Remove subsections of test cases concerned with altering and checking fork value:
sighash_tests/sighash_test
sighashtype_tests/sighash_construction_test
Out of scope
There are over 200 lines of code remaining whcih refer to "fork ID", in one form or another in the source. These relate to the sighash type (all post hard fork) and are all retained. To avoid reference to an unused fork id, it may be a good idea to rename these flags in future.
Similarly, the flags SCRIPT_ENABLE_SIGHASH_FORKID
and bitcoinconsensus_SCRIPT_ENABLE_SIGHASH_FORKID
remain as they are used by logic which decides whether a fork ID should be present or not. Again, renaming these in future to something related to the HF could be a good idea.
Test plan
ninja check-all