bip174: Add test vector for global xpub #810

pull NicolasDorier wants to merge 1 commits into bitcoin:master from NicolasDorier:bips/global-xpub-test-vector changing 1 files +4 −0

NicolasDorier commented at 2:03 PM on July 25, 2019: contributor

The global xpub by @achow101 is missing test vectors. This has already created a mismatch of implementation between NBitcoin (and thus BTCPay) and Coldcard.

One of our implementation is buggy, this test vector comes from NBitcoin.
NicolasDorier commented at 2:03 PM on July 25, 2019: contributor

Ping @peter-conalgo
doc-hex commented at 3:12 PM on July 25, 2019: none

Coldcard doesn't like the inputs' UTXO's in this file. They don't look like legit transactions.

I would prefer a vector that was based on the other examples already in the BIP. Perhaps add the XPUB data to one of the multisig examples.

Also, pet peeve: there is no way to refer to the various vectors. They need section numbers, or symbolic names or something.
achow101 commented at 3:28 PM on July 25, 2019: member

I will write tests for this and implement serialization into Core.
achow101 commented at 4:22 PM on July 25, 2019: member

Bitcoin Core does not like the psbt provided in this PR. Even when using the master branch (which should still be able to deserialize this), it is unable to deserialize it.

edit: You have 0 input txs as UTXOs, that's not allowed.
NicolasDorier commented at 4:35 PM on July 25, 2019: contributor

will make a better PSBT (the one that failed on coldcard was a proper mainnet PSBT)
NicolasDorier force-pushed on Jul 25, 2019
NicolasDorier commented at 4:49 PM on July 25, 2019: contributor

I updated the test vector, it should be correct now. (with bitcoind decoding it correctly)
bip174: Add test vector for global xpub d832c51e82

in bip-0174.mediawiki:592 in ca0bcf8567 outdated

 586 | @@ -587,6 +587,10 @@ must create this PSBT:
 587 |  * Bytes in Hex: <pre>70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f000000000000000000</pre>
 588 |  * Base64 String: <pre>cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAAAAAA=</pre>
 589 |  
 590 | +If global `PSBT_GLOBAL_XPUB` is supported, it might create this PSBT:
 591 | +* Bytes in Hex: <pre>70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f000000004f01043587cf0000000000000000001b7b7ddcd9b3a9dc2589f063b560519c5ac7624a0c766ae2486d6cef83e267c203dfee30349d7c7cf78c0ec4517bd893028a6c2a880341fc4b98be8a823d3bc52704d90c6a4f0000000000</pre>
 592 | +* Base64 String: <pre>cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAATwEENYfPAAAAAAAAAAAAG3t93NmzqdwlifBjtWBRnFrHYkoMdmriSG1s74PiZ8ID3+4wNJ18fPeMDsRRe9iTAopsKogDQfxLmL6Kgj07xScE2QxqTwAAAAAA</pre>

achow101 commented at 5:36 PM on July 25, 2019:

This isn't correct. The xpub in this is useless since all of the keys have hardened steps. This would be better as a serialization test above in the "The following are valid PSBTs" section.

NicolasDorier commented at 2:26 AM on July 26, 2019:

ok fixing it one moment. This was also not the problem when I tried on mainnet with coldcard.

NicolasDorier force-pushed on Jul 26, 2019
NicolasDorier commented at 2:36 AM on July 26, 2019: contributor

@achow101 fixed, this is normally a correct one (except if my parsing is wrong)
NicolasDorier commented at 12:26 PM on September 2, 2019: contributor

Please can someone review this?
luke-jr added the label Proposed BIP modification on Sep 19, 2019
luke-jr commented at 9:53 PM on September 19, 2019: member

Only @achow101's ACK/NACK matters here.
achow101 commented at 10:20 PM on September 19, 2019: member

ACK d832c51e82a321e4721cb9e8cc3e69e8e20aa57f
luke-jr merged this on Sep 20, 2019
luke-jr closed this on Sep 20, 2019

Contributors

Labels

Proposed BIP modification