Skip to content
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

Add FindAndDelete for Signature hash calculation #570

Closed
sanket1729 opened this issue Feb 15, 2021 · 10 comments
Closed

Add FindAndDelete for Signature hash calculation #570

sanket1729 opened this issue Feb 15, 2021 · 10 comments

Comments

@sanket1729
Copy link
Member

Add support sighash for scripts with OP_CODESEPARATOR.

@sanket1729
Copy link
Member Author

I will do it at some point in the future but is up for grabs if anyone wants to do it, it's up for grabs

@apoelstra
Copy link
Member

lol nobody wants to do this, I'm afraid it's gonna be on you :)

@vss96
Copy link

vss96 commented Sep 20, 2021

I would like to give this a try but I might be out of my depth here 😅

@apoelstra
Copy link
Member

@vss96 that would be welcome! But be aware that our signature hash computation has changed a bit since this issue was opened. In particular, since #628 we have a new structure, util::sighash::SigHashCache which is responsible for computing sighashes.

To implement CODESEPARATOR, you need to change the legacy sighash algorithm to implement FindAndDelete (see the Bitcoin Core source code for what this is); for the segwit and taproot sighashes, the CODESEPARAOR behavior is much simpler and can be found in their respective BIPs (141 for segwit and 341 for taproot, I believe)

@sanket1729
Copy link
Member Author

@vss96 you can reach me on IRC(libera) or on Twitter for more assistance

@tcharding
Copy link
Member

tcharding commented Mar 16, 2022

@vss96 its been a while since you posted so I had a go at this one. Hope you don't mind. I've only done part of it so far, holla at me if you are working on it still.

@vss96
Copy link

vss96 commented Mar 16, 2022

@tcharding Please go ahead and work on it, I never got the time to work on this 😓 .

@tcharding
Copy link
Member

Its late at night but I think I've got this, correct me if I'm wrong

So unless I'm mistaken #879 fully closes this issue. Some hand-wavy supporting evidence; all the tests in bitcoin/bitcoin/src/test/data/sighash.json appear to be in libbtc and going off number of LOC (about 500) #879 adds all of them and they all pass.

@tcharding
Copy link
Member

Shall we close this one then @sanket1729?

ref: #777 (comment)

@tcharding
Copy link
Member

tcharding commented Sep 15, 2022

We already have an implementation of FindAndDelete, its script_find_and_remove - and we are removing it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants