Page MenuHomePhabricator

Add support for having different forkid values in SigHashType
ClosedPublic

Authored by deadalnix on Mar 12 2018, 16:56.

Details

Summary

This implements support for different fork id values as per https://github.com/bitcoincashorg/spec/blob/master/replay-protected-sighash.md .

Depends on D1189

Test Plan

Updated untitests to check the new feature.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
replayprotection
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2056
Build 2259: Bitcoin ABC Buildbot (legacy)
Build 2258: arc lint + arc unit

Event Timeline

jasonbcox added inline comments.
src/script/sighashtype.h
51

Do we need to communicate out to the other clients that the second byte of sighash will contain the forkid version? If there's a spec, please link it in the summary. I found these meeting notes that mention the change, but no implementation details: https://github.com/bitcoincashorg/workgroups/blob/master/wg-hardforks/summaries/20180202%20-%20Meeting%20Summary.md

src/test/script_sighashtype_tests.cpp
33

Does the range of supported forkIds need to be so large?

38

Any reason to not simply iterate all possible values of forkId?

src/script/sighashtype.h
51

They should already know if they read https://github.com/bitcoincashorg/spec/blob/master/replay-protected-sighash.md

And if they did not, then tough luck, I guess.

src/test/script_sighashtype_tests.cpp
33

It is a common address space between all the shitfork of bitcoin, so yes, it needs to be.

38

Because the range is very large, as you noted.

This revision is now accepted and ready to land.Mar 13 2018, 21:09
This revision was automatically updated to reflect the committed changes.