Remove unneeded argument from toDER method#3185
Remove unneeded argument from toDER method#3185msalcala11 wants to merge 1 commit intobitpay:masterfrom
Conversation
23da28c to
4d1f38f
Compare
There was a problem hiding this comment.
I like the direction you're going here (using the class var as the source of truth), but there are some dangling parameters around the code base. For example, in Signature.prototype.toTxFormat and in MultiSigInput.prototype._createSignatures (and the calling stack of the latter).
For reference, here is the commit where a lot of this was implemented: e00577a
If you want to really go ham, I think the signingMethod param can be scrubbed out of a lot of the codebase since the sighash.js verify function can have the signingMethod param removed (and all implementations) since the signing method is implied by sig.isSchnorr), but that's a little beyond the scope of this MR so I'll leave that up to you whether or not you want to follow that rabbit hole.
Again, I like where you're going with this change. I'd like to see this pattern used more throughout the rest of the code.
|
Thanks for sharing the original commit and additional context. Would be great to eventually do a more thorough refactor and remove more unneeded params from the codebase. This PR is merely meant to be a quick fix for a bug I encountered ( |
|
Oh ok, I was thrown off by the title of the PR being about removing the |
|
Well it looks like the author of the original commit encountered this same bug and added the |
|
Yep, that's all good. We'll just need to finish the job instead of leaving dangling params such as in |
No description provided.