Differential D1192

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

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

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Maniphest Tasks

T301: Automatic Future Hardfork Replay Protection

Commits

rSTAGING306b4f9c13ca: Implement a flag to trigger replay protection when passed down to SignatureHash
rABC306b4f9c13ca: Implement a flag to trigger replay protection when passed down to SignatureHash

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

Branch

sighashreplay

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2072
Build 2291: Bitcoin ABC Buildbot (legacy)
Build 2290: arc lint + arc unit

Event Timeline

deadalnix created this revision.Mar 12 2018, 20:05

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 12 2018, 20:05

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2054: Diff 3158.Mar 12 2018, 20:24

deadalnix added a child revision: D1193: Add script tests for the replay protection mechanism.Mar 12 2018, 21:24

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.

Add comments

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2068: Diff 3178.Mar 14 2018, 09:53

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

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2069: Diff 3179.Mar 14 2018, 14:02

jasonbcox accepted this revision.Mar 14 2018, 15:04

This revision is now accepted and ready to land.Mar 14 2018, 15:04

Reduce the scope of some variables.

Closed by commit rABC306b4f9c13ca: Implement a flag to trigger replay protection when passed down to SignatureHash (authored by deadalnix). · Explain WhyMar 14 2018, 15:08

This revision was automatically updated to reflect the committed changes.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging started.

Build Bitcoin-ABC :: Staging :: Bitcoin-ABC Staging passed.

Harbormaster completed remote builds in B2072: Diff 3182.Mar 14 2018, 15:29

schancel added a task: T301: Automatic Future Hardfork Replay Protection.Mar 15 2018, 14:50

nakihito mentioned this in D3671: Merge #10777: [tests] Avoid redundant assignments. Remove unused variables.Jul 15 2019, 22:28

Revision Contents
Changeset List

Path

Size

src/

script/

4 lines

interpreter.cpp

8 lines

test/

data/

2005 lines

sighash_tests.cpp

97 lines

Diff 3182

src/script/interpreter.h

Loading...

src/script/interpreter.cpp

Loading...

src/test/data/sighash.json

Loading...

src/test/sighash_tests.cpp

Loading...