Page MenuHomePhabricator

Implement a flag to trigger replay protection when passed down to SignatureHash
ClosedPublic

Authored by deadalnix on Mar 12 2018, 20:05.

Details

Summary

This will allow to enable replay protection at any point in time.

Test Plan

Extend sighash_test to test for sighash generation with various sets of flags instead of just one.

Depends on D1190, D1191

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Mar 12 2018, 20:05
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2018, 20:05
jasonbcox requested changes to this revision.Mar 13 2018, 21:47
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/script/interpreter.cpp
1354 ↗(On Diff #3158)

Add a comment explaining the intention here. It looks like it's intended to scramble values greater than 1-byte in size, but I'm not sure.

src/test/sighash_tests.cpp
149 ↗(On Diff #3158)

Shouldn't this one be shold? Seems backwards looking at the function name.

151 ↗(On Diff #3158)

If you don't want to name the above variable shold, then maybe call this one shnoReplay to make it clearer?

This revision now requires changes to proceed.Mar 13 2018, 21:47
deadalnix requested review of this revision.Mar 14 2018, 09:28
deadalnix added inline comments.
src/test/sighash_tests.cpp
149 ↗(On Diff #3158)

No, this is the hash of the reference implementation.

151 ↗(On Diff #3158)

It is the old (aka pre fork) way of computing the hash. It should match the reference implementation.

deadalnix updated this revision to Diff 3178.Mar 14 2018, 09:31

Add comments

deadalnix updated this revision to Diff 3179.Mar 14 2018, 13:41

Add comment explaining the replay protection in SignatureHash and add extra tests for it.

jasonbcox accepted this revision.Mar 14 2018, 15:04
This revision is now accepted and ready to land.Mar 14 2018, 15:04
deadalnix updated this revision to Diff 3182.Mar 14 2018, 15:08

Reduce the scope of some variables.

This revision was automatically updated to reflect the committed changes.