Implement sighash cache in CHECKMULTISIG #14079
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
@jl2012 Would it be possible to generate some benchmarks to quantify the performance effect? :-) |
|
Awesome! I thought we already did that! |
| The last travis run for this pull request was 253 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened. |
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
it is not necessary
Unless there's a performance penalty it doesn't matter? Or is there another reason to cache it?
| }; | ||
|
|
||
| bool SignatureExtractorChecker::CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const | ||
| bool SignatureExtractorChecker::CheckSig(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion, uint256* sighash_copy, int* sighash_copy_type) const |
There was a problem hiding this comment.
nit, unused parameters could be commented.
|
|
||
| uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->txdata); | ||
| uint256 sighash; | ||
| if (sighash_copy_type && sighash_copy && *sighash_copy_type == nHashType) |
|
Agree with @promag; I would like to see benchmark results indicating whether this actually has any effects. |
|
Closing this as "Up for grabs". If anyone is interested in picking this up, as mentioned multiple times above, the best place to start is likely writing some benchmarks that would demonstrate any performance improvements. |
7 participants
Currently
CHECKMULTISIGrecalculates the sighash every time it verifies a key-signature pair. It is not necessary because sighash for the samenHashTypeis always the same inside aCHECKMULTISIG. Therefore, we could reuse a sighash as long as thenHashTypeis not changed.Note that in
CHECKMULTISIG, the trimming ofscriptCode(withCODESEPARATORandFindAndDelete()) is done before the signature checking loop. So thescriptCodeis always a constant in the sameCHECKMULTISIGoperation. However, this is not guaranteed across differentCHECKSIGorCHECKMULTISIGso a cross-opcode sighash cache could not be done in the same way.