-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Questions regarding SegWit, PSBT and RBF #1616
Comments
I'll respond in a bit. Thanks for your questions. |
That will have to be done with your app's logic. Then convert then into xpubs. Currently bitcoinjs-lib only accepts xpub. You can do this by using
We have implemented all of the signer responsibilities. So if you manage the private keys using us, it's as simple as running one of the sign methods. For hardware support it's a little more tricky, as you need to extrapolate some extra info from the PSBT and hand it over to the hardware wallet directly... See this issue for some ideas on how to implement: #1517
PSBT has the RBF is not too difficult, but it is not consensus code... so including it in this library is not logical. However, currently RBF is defined as "Any transaction where all input sequences are equal to or less than |
Thank you for your response. I'll look into it and let you know how it goes. |
You can use https://github.com/Anderson-Juhasc/bip84 we solved this problem (analogous to xpub/ypub/zpub) for anyone who is interested using my work here to consume bip84 which will work with legacy/bech32 address types: https://github.com/syscoin/syscoinjs-lib Note it will also work with Bitcoin and assumes a blockbook backend as well. Motivation was to do kind of like web3js for Bitcoin type environments. We are actually integrating this into a fork of Metamask so it can call out metamask once you request signing a transaction which again can be applicable to Bitcoin. This is where it will derive based on bip84 vs bip44 https://github.com/syscoin/syscoinjs-lib/blob/master/utils.js#L72 where you pass in the bip number depending on if you are doing legacy or bech32 (bip84) where it will use ZPub/ZPrv otherwise will use XPub/XPrv and testnet is always YPub/YPrv, but note the library needs this pull request accepted for it to work OOTB: Anderson-Juhasc/bip84#5 My library syscoinjslib also deals with RBF you can see it from the test fixtures how RBF would work which does what @junderw says by setting the sequence flag: https://github.com/syscoin/syscoinjs-lib/blob/master/test/fixtures/index.js#L815 Some of the nifty features we do are pertaining to just Bitcoin should you wish to use:
Anyways all the work we do is with Bitcoin compatibility in mind, this should all work with Bitcoin blockbook and transaction funding, whilst using XPub/YPub/Zpub abstracting the complexity in an SDK design with HD change address strategy built-in. If you think the code will work I would be happy to help in any way you need. Forgive my coding style as I am more of a C/C++ guy but PR's are welcome. Hopefully it can help you. |
This is not needed for Bitcoin since value is always dealt with using integers, and the largest possible UTXO value is 23.3% the size of MAX_SAFE_INTEGER.
|
Yup, agreed unless you are doing some calculations with sums and stuff. I put it in there because we have an asset layer which can extend to int64 but also incase people build statistics for whatever reason into it. |
@sidhujag Thanks for sharing your code. I still didn't have time to look into this, but if I have any issues while implementing it, I'll also look at your code. We have our own library with a shared interface between all coins we support. This allows us to easily integrate them into our apps, without having any protocol specific code in our apps. |
So I finally had time to look into this. I'm very close, but there are still a few things missing.
That works.
Got that working as well, thanks.
This is where I still have some problems. We are only using bitcoinjs, no hardware wallets involved. I fail to construct a standard segwit => segwit transaction and broadcast it to the network. Our use-case is that we have an online part with only the extended public key, and an offline part that has the extended private key. We do have access to the full mnemonic on offline signer, but I'm not sure if that is needed (ideally it is not and we only use the extended private key). When I try that, the node rejects my transaction with the following error:
Preparing transaction
Signing method
Transaction: There are a few things I don't understand yet about PSBT:
Thanks again for your help! |
After a quick glance, you're using the wrong index for addInput. It is not the index of some input, it is the index within the tx pointed to by hash of the output being used/spent in this input. "I want to spend the "index"th output of the tx at "hash"" is what you're saying with the info. |
index starts at 0 |
Ok, that makes sense. I actually did use the This is what the blockbook API returns for my zpub key:
EDIT: I also updated the code above to use |
If you are using an HD key you MUST include bip32Derivation and you MUST use a sign*HD method. Since an HD key fills all the interfaces for a non-HD key, when you use
Finalizing takes all the PSBTInput info and uses it to create the finalScriptSig and finalScriptWitness attributes, then deletes all unneeded info for that input. This step was added exactly for the use case you describe; If I pass out a multisig PSBT, and I get back 5 PSBTs each with one different signature each, I can merge the PSBTs easily if the signatures are separated out into their own logical section (partialSig), but if someone tried to make an attempt at "creating an incomplete scriptSig / witness" before I get the PSBTs, then I will have a hard time merging them if one of the signers has a bad implementation. So finalizing step being an explicit separate step is to logically separate the actions of data gathering and final input creation. It makes the protocol MUCH more flexible.
Lines 166 to 186 in 533d6c2
psbt.txInputs[0].sequence // this will give the sequence of the first input. You can get the tx info from here. As far as the other info, you can access it directly from the psbt.data.inputs (ie. value from witnessUtxo etc. exactly how you input the data you can access it) |
Thanks so much for your help. I was now able to do a transaction now, but I still had to hardcode some of the things. For example, the "path" in the So what I now had to do was use the mnemonic in the sign method to get the root key. Then the derivation path was correctly applied and everything worked. Is it somehow possible to work only with the xpub/xprv keys without accessing the root key?
What is the default Thanks a lot for your help. I think I'm done for today, but I can hopefully finish the proof of concept over the next few days. |
There is no good reason to store only a derived xprv. You should have a mnemonic, which means you have the root. bip32Derivation is defined in PSBT BIP174
|
jfyi you should look at my code if you need help, its all there. Let me know if you can't figure it out still. Its bip44/84 compliant as well. |
Hi all. In our library, which is targeted towards offline signing, we currently only support legacy bitcoin addresses and are using bitgo-utxo-lib.
I now want to improve our library and add segwit support. This library here seems to be more up to date, so I'm planning on switching.
My goal is the following:
bc1
segwit address generation/sending/signing, with all address types as possible recipientsWould this be considered the best practice for a bitcoin wallet at the moment?
While trying to implement this, I ran into a couple of issues:
How to deal with xPub, yPub and zPub
I usually use https://iancoleman.io/bip39/ as a reference. I noticed that when I select the "BIP44", "BIP49" and "BIP84" tabs, the the extended public keys have a different prefix (
xPub
,yPub
,zPub
for mainnet andtPub
,uPub
,vPub
for testnet).At first I ignored it, but when I sent the
xPub
to blockbook to get my UTXOs, I got an empty array back with myxPub
, but it worked with thezPub
key.I read some discussions around this (#927, #1334, trezor/connect#98), but I'm not sure what the current state is. It was mentioned that it was not in the BIP49 standard, but now it seems to be there: https://github.com/bitcoin/bips/blob/master/bip-0049.mediawiki#extended-key-version
So what is the proper way to handle this?
PSBT signing support
We currently get the UTXOs from an API on an online device, select the inputs we want and then send this in a custom JSON structure to our signer. The signer takes the values and creates a transaction using the
TransactionBuilder
and signs them, completely offline.It seems to me that the best way going forward would be to use the PSBT standard to do that. I read the BIP174, but it's not clear to me what different cases need to be handled by the signer.
In our online wallet, when preparing the transaction, we will only have segwit addresses as the origin. But our signer should be able to also handle PSBTs that come from Electrum/Coldcard/etc. So using the example here, would I be able to sign every possible valid transaction that can be given to the signer via PSBT? (Legacy addresses, segwit addresses, custom scripts, timelock, etc). Or is there some additional work required to handle all the different cases?
RBF (Replace by fee)
It seems that this was requested by a couple of people, but a long time ago the decision was made to not include it in this library #521.
Are there any plans to reconsider this? As far as I've seen, this seems like one of the features people see as a standard in bitcoin wallets.
If this is not planned, are there any other projects/examples that handle this and are compatible with this library?
Thanks a lot for taking the time to answer. I spent days reading about all of these issues, but it's hard to find concrete and up-to-date answers because the ecosystem is constantly evolving. I hope you can shed some light on this so I know what I should try to implement 😃.
BTW: The library and apps that will use this are completely open source, so once I have it working I would be willing to create some example scripts that can be added to this library so other people can learn from them as well.
The text was updated successfully, but these errors were encountered: