Don't hash what you're not going to sign #4694

pull jtimon wants to merge 2 commits into bitcoin:master from jtimon:nohash changing 1 files +21 −26

jtimon commented at 2:07 PM on August 13, 2014: contributor

A tiny optimization in place that is not performance critical, but in my opinion the end result is also more readable.
jgarzik commented at 2:15 PM on August 13, 2014: contributor

Having two Solvers() that do different things is ugly. Thanks for cleaning that up.

This probably conflicts with #4692 as will anything that touches script.
jtimon commented at 6:50 PM on August 13, 2014: contributor

Thanks, I hadn't seen that PR and it definitely interests me since I'm almost exclusively working on refactor script lately. I don't mind to rebase all my work on top of that since the hardest part was funding out the changes I wanted to make.
laanwj added the label Improvement on Aug 14, 2014
jtimon commented at 1:11 PM on August 14, 2014: contributor

Dependent on #4692
jtimon force-pushed on Aug 20, 2014
jtimon force-pushed on Aug 27, 2014
jtimon commented at 11:51 PM on August 27, 2014: contributor

Rebased on top of #4754
jtimon force-pushed on Aug 31, 2014
jtimon commented at 2:25 PM on August 31, 2014: contributor

Rebased on top of #4755
jtimon force-pushed on Aug 31, 2014
jtimon force-pushed on Sep 2, 2014
jtimon commented at 10:17 AM on September 2, 2014: contributor

Closing until #4754 is merged.
jtimon closed this on Sep 2, 2014
jtimon reopened this on Sep 8, 2014
jtimon force-pushed on Sep 8, 2014
jtimon force-pushed on Sep 13, 2014
sipa commented at 2:25 AM on September 16, 2014: member

I haven't reviewed fully, but this seems like it could be simplified on top #4890.
jtimon commented at 6:27 PM on September 16, 2014: contributor

I have already a version rebased on top of that, but I don't see the simplification...
sipa commented at 6:30 PM on September 16, 2014: member

You don't need to pass txTo and nIn around - just pass the SIgnatureHasher around instead.
sipa commented at 6:33 PM on September 16, 2014: member

Oh no, sorry - you still need it as the function updates the transaction itself, not just produce the signature. Nevermind.
jtimon force-pushed on Sep 17, 2014
BitcoinPullTester commented at 8:50 PM on September 17, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4694_87784ec8d0720affac6d133bf550a0a6471b530b/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
Don't hash what you're not going to sign 661e047781
jtimon force-pushed on Oct 6, 2014
Refactor SignSignature and higher level Solver d9a2c22a0b
jtimon force-pushed on Oct 6, 2014
jtimon commented at 6:50 PM on October 6, 2014: contributor

Rebased.
jtimon commented at 9:03 PM on October 29, 2014: contributor

Closing until 0.10
jtimon closed this on Oct 29, 2014
jtimon referenced this in commit f4b779b5ab on Nov 5, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#4695 Strictly hash what is going to be signed #5208 Abstract out CTransaction-specific signing into SignatureCreator