None for witness_lookup bad for PSBT.create()

[howech]

Passing in None to the witness_lookup in psbt_helper::create_multisig_psbt does the wrong thing if you end up passing in a P2WSH address as an output. It seems that an empty dictionary behaves correctly as an empty witness_lookup object.

[mflaxman]

Good catch, thanks @howech!

Let me submit a PR to add test coverage so I can merge this in (and make a new release for pypi).

[mflaxman]

On second thought, this PR looks good as-is.

However, I'm confused why both this PR branch and the old main branch pass test-coverage as-is. There are many instances in test_psbt_helper.py of create_multisig_psbt() being called without an issue both before your PR (current main branch) and after it.

@howech was there a bug/issue that made you catch this? I agree with the change but am confused how it wasn't previously a problem (and that this change didn't break test coverage). witness_lookup only matters in this code path here, so I'm thinking it has something to do with how the pubkey_lookup or tx_lookup is constructed here (or more precisely, what information is fed into the construction logic).

Separately, for some reason github didn't fire off any CI tests on your PR. I'm not sure why that is. But, I manually ran the tests on your PR branch and they do indeed pass. And I just tested that Github CI is indeed working, so I'm going to keep my eyes on that but I think it's unrelated to your PR and sometime github CI just doesn't run.

[humanumbrella]

Hi @mflaxman - yes, this was in response to a bug we found.

Trying to build a psbt with an output to this address: tb1q9hj5j7mh9f7t6cwdvz34nj6pyzva5ftj2ecarcdqph5wc3n49hyqchh3cg (from here)

fails with AttributeError("'NoneType' object has no attribute 'get'",)

[mflaxman]

Thanks @humanumbrella! Can you give me a full snippet that generates that error? I just tried generating a PSBT using tb1q9hj5j7mh9f7t6cwdvz34nj6pyzva5ftj2ecarcdqph5wc3n49hyqchh3cg and it works fine:

from buidl import *

# modified from https://github.com/buidl-bitcoin/buidl-python/blob/629d8f9d098eb9ac1c8fe7fa5d530bfaa43cd786/buidl/test/test_psbt_helper.py#L81

public_key_records = [
    # action x12
    [
        "e0c595c5",
        "tpubDBnspiLZfrq1V7j1iuMxGiPsuHyy6e4QBnADwRrbH89AcnsUEMfWiAYXmSbMuNFsrMdnbQRDGGSM1AFGL6zUWNVSmwRavoJzdQBbZKLgLgd",
        "m/45'/0",
    ],
    # agent x12
    [
        "838f3ff9",
        "tpubDAKJicb9Tkw34PFLEBUcbnH99twN3augmg7oYHHx9Aa9iodXmA4wtGEJr8h2XjJYqn2j1v5qHLjpWEe8aPihmC6jmsgomsuc9Zeh4ZushNk",
        "m/45'/0",
    ],
]

input_dict = {
    "quorum_m": 1,
    "path_dict": {
        # xfp: root_path
        "e0c595c5": "m/45'/0/0/0",
        "838f3ff9": "m/45'/0/0/0",
    },
    "prev_tx_dict": {
        "hex": "02000000000101380bff9db676d159ad34849079c77e0d5c1df9c841b6a6640cba9bfc15077eea0100000000feffffff02008312000000000017a914d96bb9c5888f473dbd077d77009fb49ba2fda24287611c92a00100000017a9148722f07fbcf0fc506ea4ba9daa811d11396bbcfd870247304402202fe3c2f18e1486407bf0baabd2b3376102f0844a754d8e2fb8de71b39b3f76c702200c1fe8f7f9ef5165929ed51bf754edd7dd3e591921979cf5b891c841a1fd19d80121037c8fe1fa1ae4dfff522c532917c73c4884469e3b6a284e9a039ec612dca78eefd29c1e00",
        "hash_hex": "4412d2a7664d01bb784a0a359e9aacf160ee436067c6a42dca355da4817ca7da",
        "output_idx": 0,
        "output_sats": 1213184,
    },
}

output_dict = {
    "sats": 999500,
    "address": "tb1q9hj5j7mh9f7t6cwdvz34nj6pyzva5ftj2ecarcdqph5wc3n49hyqchh3cg",
}

psbt_obj = create_multisig_psbt(
    public_key_records=public_key_records,
    input_dicts=[input_dict,],
    output_dicts=[output_dict,],
    fee_sats=213684,
    script_type="p2sh",
)

print(psbt_obj)

[humanumbrella]

hi @mflaxman -- don't have access to everything with me right now as I'm out of town, but I believe this was all that was needed to make the error happen (could be wrong, if this isn't enough, can figure out more tomorrow)

xpubs = [('b8ce6f0f', 'tpubDEaV1ww4hcTUiRfuGqt5s2UgxQ5Nc4XejS7gL9iAMNWr5dgRoz9CjvohtqQArpRwfEkFCKra5E3AAWoAbGdnEpWKVM43skmauoA1y3ikteQ', "m/45'/1'/1'/6"), ('c22fe2ac', 'tpubDF17mBZYUi36PMxKJDNCbDAQHkvn4B44ACTFGRCFcTXjJth5c9KehzXwNM2iU7DKbmjdtu2e2fGeg9PQccCmYrr1Hn1R2cg6KJXUFfC258w', "m/45'/1'/0'/16"), ('deadd916', 'tpubDF7U4FTHkZgjopMgwobKuSxLQQSrTcxmzgG4pYztMEJBZHubuJfQXKe5dK3VPWtCzaNW4wFWjir77srSvHBENWsj9696AFRcVLyEPw6yqU4', "m/45'/1'/2'/6")]

inputs = [{'prev_tx_dict': {'hex': '02000000000107e1c76d4b7e9415fd82597a9906857a5c28c3c76adf479e7432287593ecc0fe99000000001716001433b7afc8bad5f5349e2e6d419cc8b5b4734805effeffffff2f9e91eebe40e05791e486e492bc980477478c31b0b088adc75127b0cf8d603f0000000017160014cefea4d47717441eada491d5ddf81f1205246e33feffffff0013630847984d436c60199d88093d6b335217e989cb6d8c091dab01c15eacad00000000171600147d83ab70b4b3b51dbefa459c3ce4cfb9663d2989feffffffa17c7d257bee51c3b113693d1de673b39346916db18f035eb2a1379a7504866a000000001716001412357d9a9789fd316b58f1f6ff8fa5f76b25f15bfeffffff0f792f582c72bbe8c0fb3743a67316e969579ccccabbe57fff6b78e2ba26784b0000000017160014a40186c4e562c89ce5449c55d6557d70d2b5a6e2feffffffd6f4b17c8dbf7628d02cf55fdbb016ce0eb7fb3df40e193e9788b76f6a9d1ca600000000171600147363016ddbfde7f7e00814fc5d0508b86a89fcaefeffffffaa2b74590fe84c761ae44e633c91f94193b9daeff465d8f9109de5a9697642d00000000017160014512195bc9e2a7caecdebee967b217076f8300a2afeffffff0100c2eb0b0000000017a914c6162b3fcb88b82981ab6d7867f064981adaf90f870247304402200a3c17eb15cceed617d8f8ce15046dae2e147f14f6366ef21e2a171437073d2602203e559118aea948e067b71542e49be8020a6e8dffa5a40e84f1c9725d23abc2c9012103ca25f9df60856fbe21b02b5dfb24ddb857541ae36eb2dc8a22e960f6a563cdf00247304402205d6b742f60144b8119da59d6884e7d576bbbd7af3d31ae1585ced98aee02f95802203d9998f80e9b08a4cbb8cc2b70676fb2a85ccf39991504e274ec37e607c4e68a012102bb1e0e3c64bfade3b38ac4e840b996b868e358f96c0d39fbfc51b0ceba92f24d024730440220243a648ac452273da2538db29aa42e67ea759e541c39188fc6909db9ef9a3e9202205dc0260e9e6e9037c264b26e4246c0fefa35c9a23b2e3837f4ddd9b1f81d7a010121020e183d80df21a7912b96eedc1eca76dbb6fe8892de2d58381eaf0234de203c060247304402202889b0987639f24d7578625f5f6f0285bae96878d682e7cc6083afef83425b8902206beb7fffa15258bc1e31c97419adae659216ec0d99547aa603db05cd43f238ec012103f6ccb3f8b6c456bc30ae36d305e6dbcde778c94d30f4119ee7494c983511742a024730440220360e9898995ee75d907d99bf566041dc38251e5fc55f7febfce09f7a4dfbd9a002205c0ac1eaecc0e608f961954f1c2faa1799582cfcf01bfe0618bee4cb637ebccd01210314ccf2cfe7dcdf96eaa82c8f5e670f569b51e55e3496219f04164d95ca9c9ad10247304402204184e777af586b5d8648f38cce84b61b371232a39ada7c00a2ef07e68b9552620220544531b2dcb2237d7625817c4110d1d7811a979c0351cf16f72a082b6b0332ce01210345e4af5fde3cd5584a82b56972863e17f5715bb1cb66f9eb24e1510627d7c9510247304402205c58225a7a6a150f8755a6822025bed742c309ce5abaac3272985edcd232837302200fbb05c563d27e8b51f2eea446e5790d145e97f4058616fbb486b03fccec92d8012103ee26e1d57eb2876f429a8292a642f8dc22e0bec632134e8b5d1070b72974a8829c080000', 'hash_hex': '0a3dfa609e0120b2b25d853f4d0fbd64613a35a552b1d39759fd33aafd769223', 'output_idx': 0, 'output_sats': 200000000}, 'quorum_m': 2, 'path_list': [('b8ce6f0f', "m/45'/1'/1'/6/0/0"), ('c22fe2ac', "m/45'/1'/0'/16/0/0"), ('deadd916', "m/45'/1'/2'/6/0/0")]}, {'prev_tx_dict': {'hex': '02000000000105ad51f0e6b8fa00dea857818846449986046c9bb45d615ff45ad29ec80e532cb500000000171600142cdd14024b9490a5a9161c296bff402172d8d7e7feffffff5358dcfb86662636452af9fb85eea457a4b9ff561f13e758b23607142fcd10ea0000000017160014ffbe73f96869c2f970354d7ffe448f53ac435f1efeffffff45145f5dadac3d891b5508947bd75635e5f328427629f19db8dd14e87329f4b800000000171600147a26b913ddf4de28b309119fd5889ced4e14d267feffffff433a783525338c7a613d0d729c420fabfaf76bd50505aef1fbce6660bbabff8500000000171600140f498a82544c3e185f3290f6b503535a0314831dfeffffff0b73b4e48bf2e0a959eab1999386d085a67ae826fc9ca4f95a442997c3e652c900000000171600144c2843e40f72a6943ba129b0875eabffdd00354efeffffff02f21a10000000000017a9145d06690836805bc562141e84106a055f8c1bbf338780f0fa020000000017a914c6162b3fcb88b82981ab6d7867f064981adaf90f87024730440220186eb4ec2744ad20be0ffff18edcc90aa32820fb1cffd0aa64609ef173ebf41a02200ad99996a1a404eec24aced4310a924b0e5219f45777f19f1b6317463e18475c01210279dd68e6864716a858d4969e64210d7b216c7623a4f2c11415066caa028f08be0247304402200c32ff94c50329a8b49b2010a3edfdefa6ca2f7486d9be004805ba124f44ab2a022067da71b118b5e2510552e523ca5f80c6bce4a5bb8b14322306b72bc7cc4591b501210205dd3585ca48f6b97ee13a238a484b921180593317506f58d28d21a9b3ed89160247304402201c7f8352667309d67383b568542206519fd30143d5633880f5d11d81adf7a9b7022021f6bf731d8c4f1613af4d0efc02f9a2152cc8444d76760c8afd4ce55bcc120d0121038cc9458ebf264a31803492c86ce0b1aaf729ad53a710e8a14341f6d358c11a720247304402207b7b444a8267d2972d2cb20fee632cd3b439a1e0bd363af84f16e395d816e2e2022036b56b473199971145fd93ba25c89a96036f4756b4d895456341c90e6ef6b5ef0121029801635f5b14994be58e0bb37eaec9c8ffadacdf513189a806e9e1a207af1ace02473044022031bd0fa64f04225c9764ebac3bdefcaa23f5df918e74598b7b180adc2a233262022051f0af2c760d17f144d60708dfb7bef716489ecb129366c6baadfc67e61e11980121035d03fc0b2ef8ad2f6821f7f561a7936c6772fdd53f1ea58d54e5a8673d4da5019b080000', 'hash_hex': '998eaf2f1d0e95261128c23652d311d9cba27dc505a73d058c7177e258c92a2a', 'output_idx': 1, 'output_sats': 50000000}, 'quorum_m': 2, 'path_list': [('b8ce6f0f', "m/45'/1'/1'/6/0/0"), ('c22fe2ac', "m/45'/1'/0'/16/0/0"), ('deadd916', "m/45'/1'/2'/6/0/0")]}]

outputs = [{'sats': 10000000, 'address': 'tb1q9hj5j7mh9f7t6cwdvz34nj6pyzva5ftj2ecarcdqph5wc3n49hyqchh3cg'}, {'sats': 239997984, 'address': '2MuCSCsAa8QD7AQ2VcMPoumsCrFkTgK8feh', 'quorum_m': 2, 'path_dict': {'b8ce6f0f': "m/45'/1'/1'/6/0/1", 'deadd916': "m/45'/1'/2'/6/0/1", 'c22fe2ac': "m/45'/1'/0'/16/0/1"}}]

fee_satoshis = 2016

[mflaxman]

Thanks @humanumbrella! Confirmed that is a bug on the main branch and that this PR fixes it.

Digging into it now.

[mflaxman]

I just tried generating a PSBT using tb1q9hj5j7mh9f7t6cwdvz34nj6pyzva5ftj2ecarcdqph5wc3n49hyqchh3cg and it works fine:

Doh, I just realized I was on this PR branch when I ran my code above. My code snippet above (as well as yours) does indeed throw an error on the main branch.

Good news is that makes this an easy fix! Thanks @humanumbrella and @howech for catching/fixing this. Submitting a PR to this PR to add test coverage now, then we can merge and I'll cut a new release for pypi.

[mflaxman]

I added test coverage and bumped the version number, but I couldn't find a way in GitHub's workflow to submit a PR to this PR (I think GitHub doesn't allow that because this PR is coming from @howech's branch and not a buidl-python branch).

So, I included @howech's commit along with one from me to add test coverage, and merged it in here: https://github.com/buidl-bitcoin/buidl-python/pull/131

Thank you @humanumbrella and @howech! I cut a new release on pypi: https://pypi.org/project/buidl/0.2.36/