-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Don't hash what you're not going to sign #4694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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. |
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. |
Dependent on #4692 |
Rebased on top of #4754 |
Rebased on top of #4755 |
d24bba7
to
28f3f4d
Compare
Closing until #4754 is merged. |
3744560
to
1b5938e
Compare
I haven't reviewed fully, but this seems like it could be simplified on top #4890. |
I have already a version rebased on top of that, but I don't see the simplification... |
You don't need to pass txTo and nIn around - just pass the SIgnatureHasher around instead. |
Oh no, sorry - you still need it as the function updates the transaction itself, not just produce the signature. Nevermind. |
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4694_87784ec8d0720affac6d133bf550a0a6471b530b/ for binaries and test log. |
Rebased. |
Closing until 0.10 |
A tiny optimization in place that is not performance critical, but in my opinion the end result is also more readable.