Cleaned up last usages of nHashType
ClosedPublic
Actions

Authored by jasonbcox on Jan 17 2018, 19:31.

Details

Reviewers

deadalnix
schancel

Group Reviewers

Restricted Project

Commits

Summary

Final cleanup work to complete T61.

Test Plan

make check

Repository

rABC Bitcoin ABC

Branch

master

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1646
Build 1646: arc lint + arc unit

deadalnix requested changes to this revision.Jan 17 2018, 21:47

deadalnix added inline comments.

src/test/sighash_tests.cpp
145	Keep the raw integer in here. You don't want `SigHashType` involved at all on this codepath. This ensures no information is lost in the process.

This revision now requires changes to proceed.Jan 17 2018, 21:47

jasonbcox marked an inline comment as done.Jan 17 2018, 22:05

jasonbcox added inline comments.

src/test/sighash_tests.cpp
145	Fair point. I reverted all changes in this function, since it didn't make sense to keep sigHashType and nHashType side-by-side. Not to mention it wouldn't make sense to call RandomTransaction() using sigHashType for the same reason as you stated for SignatureHashOld().

jasonbcox marked 2 inline comments as done.Jan 17 2018, 22:05

reverted changes around SignatureHashOld() to ensure no information loss

deadalnix requested changes to this revision.Jan 17 2018, 23:48

deadalnix added inline comments.

src/test/sighash_tests.cpp
134 ↗	(On Diff #2569)	Arguably, you should add SigHashType sigHashType(nHashType);
137 ↗	(On Diff #2569)	sigHashType = sigHashType.withForkId(true);
146 ↗	(On Diff #2569)	Remove constructor.