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.
jtimon cross-referenced this on Aug 13, 2014 from issue Strictly hash what is going to be signed by jtimon
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.
jtimon cross-referenced this on Aug 13, 2014 from issue libbitcoinscript by TheBlueMatt
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.
laanwj cross-referenced this on Oct 2, 2014 from issue Remove core dependencies from CScript by theuni
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
sipa cross-referenced this on Nov 4, 2014 from issue Abstract out CTransaction-specific signing into SignatureCreator by sipa
jtimon referenced this in commit f4b779b5ab on Nov 5, 2014
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#4692 libbitcoinscript #4695 Strictly hash what is going to be signed #4754 Make script more modular #4755 Move code from script to wallet #4890 Make signature caching optional, and move it out.#4981 Remove core dependencies from CScript #5208 Abstract out CTransaction-specific signing into SignatureCreator