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.

Sign up for GitHub

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

Jump to bottom

fix wrong display of other sighash types + p2sh redeemScript when <=4 bytes + consistency with esplora backend + tapscript fixes #1502

Merged
merged 4 commits into from
Apr 12, 2022
Merged

fix wrong display of other sighash types + p2sh redeemScript when <=4 bytes + consistency with esplora backend + tapscript fixes #1502

merged 4 commits into from
Apr 12, 2022

Conversation

antonilol
Copy link
Contributor

@antonilol antonilol commented Apr 3, 2022 •
edited

tx: f06e7c91f741c0320a6aed477aa4b702c4b592b3ffa6089f51f90d9cab9e99e1

more info on sighash types: https://en.bitcoin.it/wiki/OP_CHECKSIG#How_it_works

before:
image

after:
image

other bug:
tx: 7d5e417287b32a166127042cd8fc2e2bf6205cc718c333e6f37778c358fb5307

before:
image

after:
image

just found out this fixes #1260

@cla-bot cla-bot bot added the cla-signed label Apr 3, 2022
@antonilol antonilol changed the title fix wrong display of other sighash types and p2sh redeemScript when <4 bytes fix wrong display of other sighash types and p2sh redeemScript when <=4 bytes Apr 3, 2022
@antonilol
Completely rewrote convertScriptSigAsm
dfb5ba5 
it now gives identical output to esplora, tested with the following TXs (testnet):

88710a9a6751827490f260e307757543f533c0f18bcd6865794713d263d5f5a4
446b2aad074de94efa28a1e82d2e6016dcb8a8ca38aca1a5a8eac6ef54e56a2e
4cfc410092e9514c14f48b61e20d2d3baf540ae7e981a821dd8c05dd4b7cd591
4b55dde38173174ab09e5571ebffffca798ba11143d28b9770600ff376dc778a
@antonilol antonilol marked this pull request as ready for review April 3, 2022 19:43
@antonilol
Copy link
Contributor Author

a small change was needed to get this working: convertScriptSigAsm now expects a script in hex, not asm anymore. i changed this were it is used

@antonilol
Copy link
Contributor Author

there were also some inconsistencies between output script and redeemScript/witnessScript because the output script came in ASM from bitcoin core, and the redeemScript/witnessScript was converted to ASM with bitcoinjs-lib, which output different things for the same input. Now every script is passed in as hex and converted to ASM with bitcoinjs-lib

@antonilol antonilol mentioned this pull request Apr 3, 2022
Closed
@antonilol antonilol changed the title fix wrong display of other sighash types and p2sh redeemScript when <=4 bytes fix wrong display of other sighash types + p2sh redeemScript when <=4 bytes + consistency with esplora backend Apr 3, 2022
@antonilol antonilol changed the title fix wrong display of other sighash types + p2sh redeemScript when <=4 bytes + consistency with esplora backend fix wrong display of other sighash types + p2sh redeemScript when <=4 bytes + consistency with esplora backend + tapscript fixes Apr 4, 2022
@antonilol
Copy link
Contributor Author

53d68a3 adds OP_CHECKSIGADD (new tapscript opcode) to convertScriptSigAsm. this will only be visible now with bitcoind
it is already added to rust-bitcoin, but its version has to be bumped before it will be visible on mempool.space
PR to master (merged): https://github.com/rust-bitcoin/rust-bitcoin/pull/644/files#diff-1145b219d5b2128f8d2cc7b8b47c087b5cc1bc51e5f505443f66a73f0dd81a0cR421
PR to bump version: rust-bitcoin/rust-bitcoin#785

@antonilol
Copy link
Contributor Author

antonilol commented Apr 4, 2022 •
edited

taproot script spend with OP_CHECKSIGADD i made: 7a58768481f63f310119a5a8b02bd94613cddd3f380a108c5134f1be6a774d0b (testnet)

hmmm the script is not there on mempool.space (dont know if this also will come with rust-bitcoin v0.28)

image

bitcoind backend:

image

@antonilol antonilol force-pushed the script-display-2 branch 2 times, most recently from ab0f6a0 to 27d6ffc Compare April 9, 2022 22:14
@antonilol antonilol mentioned this pull request Apr 9, 2022
Merged
@softsimon softsimon self-requested a review April 12, 2022 18:28
softsimon
softsimon approved these changes Apr 12, 2022
View reviewed changes
Copy link
Member

@softsimon softsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK

@softsimon softsimon merged commit cb89422 into mempool:master Apr 12, 2022
@antonilol antonilol deleted the script-display-2 branch April 12, 2022 18:54
@antonilol antonilol mentioned this pull request May 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@softsimon softsimon softsimon approved these changes

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

local hosted mempool wrong script opcodes
2 participants
@antonilol @softsimon