bitcore icon indicating copy to clipboard operation
bitcore copied to clipboard

Remove unneeded argument from toDER method

Open msalcala11 opened this issue 3 years ago • 4 comments

msalcala11 avatar Jun 06 '21 20:06 msalcala11

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 (this.isSchnorr is currently being overwritten to undefined if someone calls .set without isSchnorr included as part of the params obj)

msalcala11 avatar Jun 09 '21 01:06 msalcala11

Oh ok, I was thrown off by the title of the PR being about removing the signingMethod param in the toBuffer/toDER function. That's fine if you're just wanting to change the one line in set. Removing the toDER param would need a little more attention if you want to continue with that.

kajoseph avatar Jun 09 '21 01:06 kajoseph

Well it looks like the author of the original commit encountered this same bug and added the siginingMethod param to the toDER method as quick patch, but addressing the root cause (ensuring that this.isSchnorr does not get wiped out by a .set call without obj.isSchnorr set) makes toDER's signingMethod param unnecessary.

msalcala11 avatar Jun 09 '21 01:06 msalcala11

Yep, that's all good. We'll just need to finish the job instead of leaving dangling params such as in Signature.prototype.toTxFormat, etc.

kajoseph avatar Jun 09 '21 15:06 kajoseph