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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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

deadalnix added inline comments.

src/test/sighash_tests.cpp
145 ↗	(On Diff #2566)	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 ↗	(On Diff #2566)	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.