-
Notifications
You must be signed in to change notification settings - Fork 223
psbt: add MuSig2 fields #784
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
base: master
Are you sure you want to change the base?
Conversation
fc57dfb
to
46ceff9
Compare
I ended copying @bigspider's code. However there's no test coverage. I also can't manually test it on a device without passing the BIP-388 policy policy and (no BIP?) HMAC, which is a separate issue. |
Fixed a few bugs. I wanted to test this using #792 and taking some code from MooSig, but the latter changed the |
I think I get the gist of the |
It was moved by bitcoin/bitcoin#31161
``` error: "/home/runner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/Cargo.lock" does not exist, unable to build with the standard library, try: rustup component add rust-src --toolchain nightly-x86_64-unknown-linux-gnu ``` https://github.com/bitcoin-core/HWI/actions/runs/16440824447/job/46462763374?pr=795
We can now use the main esp-idf toochain to install the appropriate version of the qemu emulator, rather than building it ourselves from source. Mirrors change recently made to Jade repo.
See e.g. https://github.com/bitcoin-core/HWI/actions/runs/16440824447/job/46463441252 ``` work/bitcoin/build/bin/bitcoind: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.38' not found (required by work/bitcoin/build/bin/bitcoind) ```` Job lingered for 45 minutes.
Seems to be no longer needed: Coldcard/firmware#537 Related CI failure: https://github.com/bitcoin-core/HWI/actions/runs/16464204169/job/46537622260?pr=797
Manually re-applied the patch after the original code seems to have moved around a bit.
``` ERROR: coldcard: test_signtx (test_device.TestSignTx.test_signtx) (addrtypes=['legacy'], multisig_types=['legacy'], external=True, op_return=False) ---------------------------------------------------------------------- Traceback (most recent call last): File "/__w/HWI/HWI/test/test_device.py", line 588, in test_signtx self._test_signtx(addrtypes, multisig_types, external, op_return) File "/__w/HWI/HWI/test/test_device.py", line 576, in _test_signtx self._generate_and_finalize(True, psbt) File "/__w/HWI/HWI/test/test_device.py", line 403, in _generate_and_finalize self.assertTrue(first_sign_res["signed"]) ~~~~~~~~~~~~~~^^^^^^^^^^ KeyError: 'signed' ```
This reverts commit edab2af. The always() option is too powerfull. The next commit implements an alternative solution to the original issue.
This ensures the failure to build a simulator for one device doesn't abort running jobs for the others. They're still grouped by manufacturer. Alternative to bitcoin-core#743.
Build failure on ubuntu-latest: ``` ../py/stackctrl.c: In function ‘mp_stack_ctrl_init’: ../py/stackctrl.c:32:32: error: storing the address of local variable ‘stack_dummy’ in ‘mp_state_ctx.thread.stack_top’ [-Werror=dangling-pointer=] 32 | MP_STATE_THREAD(stack_top) = (char *)&stack_dummy; ../py/stackctrl.c:31:18: note: ‘stack_dummy’ declared here 31 | volatile int stack_dummy; | ^~~~~~~~~~~ In file included from ../py/runtime.h:29, from ../py/stackctrl.c:27: ../py/mpstate.h:282:23: note: ‘mp_state_ctx’ declared here 282 | extern mp_state_ctx_t mp_state_ctx; | ^~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [../py/mkrules.mk:77: build/py/stackctrl.o] Error 1 ``` Test failure (after downgrading build sim): ``` File "/github/home/.cache/pypoetry/virtualenvs/hwi-crEDFiR--py3.9/lib/python3.9/site-packages/sdl2/dll.py", line 362, in <module> dll = DLL("SDL2", ["SDL2", "SDL2-2.0", "SDL2-2.0.0"], os.getenv("PYSDL2_DLL_PATH")) File "/github/home/.cache/pypoetry/virtualenvs/hwi-crEDFiR--py3.9/lib/python3.9/site-packages/sdl2/dll.py", line 253, in __init__ raise RuntimeError("could not find any library for %s (%s)" % RuntimeError: could not find any library for SDL2 (PYSDL2_DLL_PATH: unset) ``` https://github.com/bitcoin-core/HWI/actions/runs/16466809973/job/46548656293?pr=795
The build on ubuntu-latest succeeds, but the resulting binary uses a too recent version of glibc for the test runners to handle. This only seems to impact Trezor 1, but just downgrade for Trezor T as well.
NanoS support has been dropped: LedgerHQ/app-bitcoin-new#262 NanoX also makes it possible to test MuSig2 in the future. Keep NanoS for legacy.
I've been testing this with various wallets and I'm pretty sure it works now. Rebasing on #795 to make sure CI passes. |
Co-Authored-By: Salvatore Ingala <6681844+bigspider@users.noreply.github.com>
Fixed linter Looks like I accidentally dropped I also added the new test vectors, copied from Bitcoin Core. |
Fixes #783
Based on #795