-
Notifications
You must be signed in to change notification settings - Fork 146
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
Failed to sign PSBT transaction with Bare MultiSig UTXO #3897
Comments
This is likely an error in how the PSBT is built, not an issue with the wallet. Can you create a demo of your code so we can look at it? |
Closing this issue bc likely not a bug with the wallet, but user error in building the PSBT passed to us. Will look to providing better docs for apps to use this feature. |
It's truly not a normal multisig, but a must format for src20 transactions. And we can sign the transaction through Unisat. |
Happy to help, but we need more information from you. We requested some demo code to understand what tx you are working with here. Can you provide? This is an error coming from the signing lib we use, which you can reference here: https://github.com/paulmillr/scure-btc-signer |
It's not convenient to provide all the code. you can check the bare multi-sig UTXO format from https://mempool.space/tx/5b05ce32451f80ba0beb8887a0ba901c3f1c553d594e72fce7d4d50c5b557c48. We are trying to integrate Hiro to our platform(https://openstamp.io/), but we are stuck. |
|
Check out 'SRC-20 BTC Transaction Specifications |
I am currently assigned/working on another issue with PSBTs, but I can reopen this and look at it after. |
@btcopenstamp can you provide a hex of the PSBT you are trying to sign, or a gist of the decoded PSBT? I am doing a big refactor of our PSBT implementation. I'd like to get a fix for this included in it if I can. It would help if I can quickly reproduce this on my end. |
Hi. Sorry for the late response. You can try to create a TX with bare multi-sig output on your own for testing. First, you need to create a normal PSBT TX with inputs and outputs, then add the following 'bareMultiSigLockingScript' as a bare multi-sig output. bitcoinjs-lib example:
Here is an unsigned PSBT with bare multi-sig output: 70736274ff0100fd550102000000014d477394b42190a29f892a48b61eaac7b4b6b26721f89185aea105d60d322cf70000000000fdffffff04708e010000000000160014bf4ea576eca42fc46447afd294172352e79b88751c0300000000000069512102a92801734e48efdbcd3075c9d8a28393b2be3ea16019e0f2265c130e02cddef22103261b400c8a36ddb3972e0675e039e9f6b3d7212f45d019f08c5e8733226eff222102222222222222222222222222222222222222222222222222222222222222222253ae1c0300000000000069512102db89151a5a4ff5bacc8d63b961ec397c6a6619466c83cadc302c1445303ab7f32103cd77827da4cb2ac4278a612733e089da7c6897aaf5dc0aafdfb152ffff0a2daa2102222222222222222222222222222222222222222222222222222222222222222253ae2803000000000000160014b3322b56e4e79264a91ed9f85d608af0a70f78db000000000001011f2803000000000000160014b3322b56e4e79264a91ed9f85d608af0a70f78db010304810000000000000000 |
Hi @fbwoolf . How's the issue going? Is there an ETA to fix this? |
@btcopenstamp if bitcoinjs-lib accepts your tx it doesn't mean it's valid. The checks are there for a reason. If you are using basic multisig, it must be wrapped in P2SH / P2WSH / P2SH-P2WSH. If you are using taproot multisig, it must be wrapped in P2TR. |
Hi @paulmillr. Thanks for the reminder. We are using bare multisig which is not a basic multisig. Bare Multisig Locking Script Format: OP_M <pub_key_1> <pub_key_2> ... <pub_key_N> OP_N OP_CHECKMULTISIG |
@fbwoolf works for me to merge! |
This is bare / basic multisig. And it must be wrapped into p2sh / p2wsh. |
@markmhx it doesn't seem like a good idea to merge it bc the implementation is outdated on the app side? ^ |
Please check out https://mempool.space/tx/757b329dcdd3fea73d7b82d4e3ae22bd8b7ac73e0a46f833ee67fa380ba31bdf It used the pubkey of bare multi-sig output to store data, not normal(P2SH...) multisig to hold BTC. |
It's doable but it should not be done. It's written here: https://learnmeabitcoin.com/technical/p2ms. |
I'm not sure why I need to repeat it 5 times. You are doing something that has been discouraged since p2sh was released in 2011. This is bad behavior that should not be encouraged and we definitely should not add it as "default" behavior. If you need this, you should be fine with maintaining a fork. |
Yeah. I got your point. But that's how counterparty and btc stamps protocol work, it's not what we want to do that way. So I think there's no need to discuss why we need to use that kind of multisig here. Thank you for your thoughtful and detailed explanation. |
@kyranjamie wanted to make you aware of this ^ if you haven't been tracking the issue. |
@fbwoolf I'm not following the details here, so it's up to you and @kyranjamie whether your PR seems appropriate to merge. It does sound like @paulmillr is cautioning against it. Let's perhaps discuss briefly as a team this coming week for shared context? |
By the way, since src20 transfer function is related with bare multisig output, if Hiro would like to support Src20 transfer in the future, Hiro also needs to compose src20 transfer tx with bare multisig outputs. |
There's a debate going on about Bare MultiSig on the protocol level here, due to a proposed change that would have nodes filter them out from the mempool by default (intended to prevent stamps, which are viewed by the change's proponents as network spam): bitcoin/bitcoin#28217 |
Hi @fbwoolf @markmhendrickson , Sorry for the late feedback. I am not sure whether Leather is gonna support signing bare multi-sig since the debate @markmhendrickson mentioned before. I just tried the current Leather version, but it seems #4168 does not solve the problem. And like I said before, the test package on #4060 works fine for this issue, but #4060 still on open status. |
Thanks for following up, @btcopenstamp. Are you still getting the same error as originally reported in this issue when trying in the latest version? Or is there a different behavior? |
Right, bc the fix was for address type being 'unknown`, makes sense it didn't fix for you. |
Understood. I assumed issue #4186 had resolved this. So it appears we're still hindered by paulmillr/scure-btc-signer#55. However, Paulmillr suggests avoiding the ms address type. If Leather doesn't offer a workaround, it might be impossible to support SRC20 trading via Leather on OpenStamp. If that's the case, we could consider closing this issue. |
Please also check the latest comment at #4215 |
@fbwoolf @markmhendrickson With SRC20 gaining popularity, many Leather users are moving to Unisat for its bare multi-sig output support. We suggest that the Leather team reconsider adding this feature. |
Our platform is currently developing a SRC20 marketplace and plans to integrate with the Hiro wallet. During our testing, we found that Hiro cannot sign PSBT transactions that include Bare MultiSig outputs (similar to the 2nd and 3rd outputs in this transaction: https://mempool.space/tx/5b05ce32451f80ba0beb8887a0ba901c3f1c553d594e72fce7d4d50c5b557c48). The error we encountered is shown in the following screenshot.
Could you please assist us in understanding what may be causing this issue?
By the way, we have tested with the UniSat wallet and it is able to sign these transactions correctly. However, our first choice is still Hiro, so we hope that you can assist us in resolving this issue. Thanks.
The text was updated successfully, but these errors were encountered: