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

Account and change addresses are not recognized #36

Open
bjarnemagnussen opened this issue Sep 28, 2020 · 3 comments
Open

Account and change addresses are not recognized #36

bjarnemagnussen opened this issue Sep 28, 2020 · 3 comments

Comments

@bjarnemagnussen
Copy link

bjarnemagnussen commented Sep 28, 2020

Currently when sending from a vault the wallet will add the change output, however without adding meta information to the PSBT that would allow hardware wallets to understand it is a change output.

To allow hardware devices to understand it is a change output the following "per-output" types for the change output should be set as defined by BIP-174:
Lily Wallet should set the key PSBT_OUT_WITNESS_SCRIPT with value corresponding to the witness script of the change output, and set keys for PSBT_OUT_BIP32_DERIVATION with fingerprints and derivation paths for the public keys involved in the change output.

Likewise to be done for account addresses if they should also be recognized by the devices.

@KayBeSee
Copy link
Collaborator

Is this supported in bitcoinjs-lib?

@bjarnemagnussen
Copy link
Author

bjarnemagnussen commented Oct 1, 2020

bitcoinjs-lib does support it.

In addition to adding the values to the output data as explained above, it would also be a good idea to add the extended pubkey globally, which would allow Trezor devices to verify that the change outputs are for the exact same cosigners as for the input(s).

I don't know too much JavaScript, but looking into the bitcoinjs-lib I think I found which values must be set.

On the output data:
The values have to be added to the psbt.addOutput method here https://github.com/KayBeSee/lily-wallet/blob/634e8d21ea1203c11e57b43ad50ac00f7ccee764/src/pages/Send/utils.js#L185-L190

The fields to add are witnessScript and bip32Derivation, similar to how they are done for psbt.addInput.

I tried to see if the values can be found on the unusedChangeAddress[0] object, but I don't know enough JavaScript and couldn't figure it out.

But as an example it should look like:

psbt.addOutput({
      script: address.toOutputScript(unusedChangeAddresses[0].address, currentBitcoinNetwork),
      value: spendingUtxosTotal.minus(outputTotal).toNumber(),
      witnessScript: Bytes.from('00', 'hex'),
      bip32Derivation: [{
         masterFingerprint: Buffer.from('abcabc', 'hex'),
         pubkey: Buffer.from('000000', 'hex'),
         path: "m/48'/0'/0'/2'/1/0"
       },{
         masterFingerprint: Buffer.from('defdef', 'hex'),
         pubkey: Buffer.from('111111', 'hex'),
         path: "m/48'/0'/0'/2'/1/0"
       }, {...}]
})

Additionally, to add the extended pubkey globally, you can use psbt.updateGlobal like this:

psbt.updateGlobal({
    globalXpub: [{
      extendedPubkey: Buffer.from('02aa7ed3XPUB1', 'hex'),
      masterFingerprint: Buffer.from('abcabc', 'hex'),
      path: "m/48'/0'/0'/2'"
    }, {
      extendedPubkey: Buffer.from('02aa7ed3XPUB2', 'hex'),
      masterFingerprint: Buffer.from('defdef', 'hex'),
      path: "m/48'/0'/0'/2'"
    }, {...}]
})

@bjarnemagnussen
Copy link
Author

I was trying to hard-code values and see if my Trezor would recognize the change output, which never was the case. For Electrum it is not a problem but when I used a PSBT Electrum made and sent it to the Trezor with the HWI that Lily Wallet uses the Trezor did again not recognize the change output.

It seems that setting extended public keys in the global is not supported by the bitcoin-core HWI does yet, see bitcoin-core/HWI#355. Hopefully this will change soon.

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

No branches or pull requests

2 participants