Closed issmall closed 9 months ago
Thanks, @issmall. You're absolutely right; the comparison method currently used is incorrect and needs correction.
That said, unless the Buffers were duplicated, their references should have remained consistent. Could you elaborate on how you came across this error?
I plan to review the PR shortly. At an initial look, there could be issues related to it passing the linter tests. Also, it's noteworthy that the witnesses
type is undefined | Buffer
, which isn't suitable as arguments for Buffer.compare
.
Did the provided fix resolve your specific problem? While your identification of this issue is indeed valid and needs fixing, I want to ensure that we're also addressing any underlying problem you might be facing.
Hi @issmall , can take this a look?
It works.
I'm currently working on developing a self-custody platform based on this project. A few months ago, when I was researching Miniscript, I discovered this project and tried to write a demo. At that time, I could successfully create a wallet, sign transactions, and send them to the blockchain. However, last week, while testing, I encountered persistent errors. Initially, I thought it was a problem with my own calls, but later, I traced the issue to this specific location.
This project is fantastic!
It works.
Do you mean specifically the PR I just submitted? https://github.com/bitcoinerlab/descriptors/pull/23
This project is fantastic!
Thank you for your kind words, @issmall. It means the world to me!
Yes, I tested the fix in my project, it works well
During development, I noticed that
this.getWitnessScript()
andinput.witnessScript
have the same values, but using!==
here returnstrue
. Perhaps it should be evaluated usingBuffer.compare
: https://github.com/bitcoinerlab/descriptors/blob/5945e7804304c672c769fbcc7f2b6ec6af5f2c9f/src/descriptors.ts#L772.