Merge branch 'LedgerHQ:develop' into develop

bitcoinerlab · Jul 17, 2023 · 8f441a0 · 8f441a0
2 parents 2383790 + f292967
commit 8f441a0
Show file tree

Hide file tree

Showing 83 changed files with 5,836 additions and 1,151 deletions.
diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml
@@ -264,4 +264,4 @@ jobs:
       - name: Run tests
         run: |
           cd bitcoin_client_rs/
-          cargo test
+          cargo test --no-default-features --features="async"
diff --git a/.github/workflows/guidelines-enforcer.yml b/.github/workflows/guidelines-enforcer.yml
@@ -0,0 +1,22 @@
+name: Ensure compliance with Ledger guidelines
+
+# This workflow is mandatory in all applications
+# It calls a reusable workflow guidelines_enforcer developed by Ledger's internal developer team.
+# The successful completion of the reusable workflow is a mandatory step for an app to be available on the Ledger
+# application store.
+#
+# More information on the guidelines can be found in the repository:
+# LedgerHQ/ledger-app-workflows/
+
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - master
+      - develop
+  pull_request:
+
+jobs:
+  guidelines_enforcer:
+    name: Call Ledger guidelines_enforcer
+    uses: LedgerHQ/ledger-app-workflows/.github/workflows/reusable_guidelines_enforcer.yml@v1
diff --git a/.github/workflows/swap-ci-workflow.yml b/.github/workflows/swap-ci-workflow.yml
@@ -0,0 +1,16 @@
+name: Swap functional tests
+
+on:
+  workflow_dispatch:
+  push:
+    branches:
+    - master
+    - develop
+  pull_request:
+
+jobs:
+  job_functional_tests:
+    uses: LedgerHQ/app-exchange/.github/workflows/reusable_swap_functional_tests.yml@develop
+    with:
+      branch_for_bitcoin: ${{ github.ref }}
+      test_filter: '"btc or bitcoin or Bitcoin"'
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 Dates are in `dd-mm-yyyy` format.
 
+## [2.1.3] - 21-06-2023
+
+### Changed
+
+- Improved UX for self-transfers, that is, transactions where all the outputs are change outputs.
+- Outputs containing a single `OP_RETURN` (without any data push) can now be signed in order to support [BIP-0322](https://github.com/bitcoin/bips/blob/master/bip-0322.mediawiki) implementations.
+
+
+### Fixed
+
+- Wrong address generation for miniscript policies containing an unusual `thresh(1,X)` fragment (that is, with threshold 1, and a single condition). This should not happen in practice, as the policy is redundant for just `X`. Client libraries have been updated to detect and prevent usage of these policies.
+- Resolved a slight regression in signing performance introduced in v2.1.2.
+
 ## [2.1.2] - 03-04-2023
 
 ### Added
@@ -15,6 +28,8 @@ Dates are in `dd-mm-yyyy` format.
 
 ### Fixed
 
+- Miniscript policies containing an `a:` fragment returned an incorrect address in versions `2.1.0` and `2.1.1` of the app. The **upgrade is strongly recommended** for users of miniscript wallets.
+- The app will now reject showing or returning an address for a wallet policy if the `address_index` is larger than or equal to `2147483648`; previous version would return an address for a hardened derivation, which is undesirable.
 - Nested segwit transactions (P2SH-P2WPKH and P2SH-P2WSH) can now be signed (with a warning) if the PSBT contains the witness-utxo but no non-witness-utxo. This aligns their behavior to other types of Segwitv0 transactions since version 2.0.6.
 
 ## [2.1.1] - 23-01-2023

diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 # ****************************************************************************
 #    Ledger App for Bitcoin
-#    (c) 2021 Ledger SAS.
+#    (c) 2023 Ledger SAS.
 #
 #   Licensed under the Apache License, Version 2.0 (the "License");
 #   you may not use this file except in compliance with the License.
@@ -22,32 +22,44 @@ endif
 include $(BOLOS_SDK)/Makefile.defines
 
 # TODO: compile with the right path restrictions
-# APP_LOAD_PARAMS  = --curve secp256k1
-APP_LOAD_PARAMS  = $(COMMON_LOAD_PARAMS)
-APP_PATH = ""
 
+# Application allowed derivation curves.
+CURVE_APP_LOAD_PARAMS = secp256k1
+
+# Application allowed derivation paths.
+PATH_APP_LOAD_PARAMS = ""
+APP_LOAD_PARAMS += --path_slip21 "LEDGER-Wallet policy"
+
+# Application version
 APPVERSION_M = 2
 APPVERSION_N = 1
-APPVERSION_P = 2
+APPVERSION_P = 3
 APPVERSION   = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
-
 APP_STACK_SIZE = 3072
 
+# Setting to allow building variant applications
+VARIANT_PARAM = COIN
+VARIANT_VALUES = bitcoin_testnet bitcoin
+
 # simplify for tests
 ifndef COIN
 COIN=bitcoin_testnet
 endif
 
-# Flags: BOLOS_SETTINGS, GLOBAL_PIN, DERIVE_MASTER
-APP_LOAD_FLAGS=--appFlags 0xa50
+########################################
+#     Application custom permissions   #
+########################################
+HAVE_APPLICATION_FLAG_DERIVE_MASTER = 1
+HAVE_APPLICATION_FLAG_GLOBAL_PIN = 1
+HAVE_APPLICATION_FLAG_BOLOS_SETTINGS = 1
+HAVE_APPLICATION_FLAG_LIBRARY = 1
 
 ifeq ($(COIN),bitcoin_testnet)
 
 # Bitcoin testnet, no legacy support
 DEFINES   += BIP32_PUBKEY_VERSION=0x043587CF
 DEFINES   += BIP44_COIN_TYPE=1
-DEFINES   += BIP44_COIN_TYPE_2=1
 DEFINES   += COIN_P2PKH_VERSION=111
 DEFINES   += COIN_P2SH_VERSION=196
 DEFINES   += COIN_NATIVE_SEGWIT_PREFIX=\"tb\"
@@ -60,7 +72,6 @@ else ifeq ($(COIN),bitcoin)
 # Bitcoin mainnet, no legacy support
 DEFINES   += BIP32_PUBKEY_VERSION=0x0488B21E
 DEFINES   += BIP44_COIN_TYPE=0
-DEFINES   += BIP44_COIN_TYPE_2=0
 DEFINES   += COIN_P2PKH_VERSION=0
 DEFINES   += COIN_P2SH_VERSION=5
 DEFINES   += COIN_NATIVE_SEGWIT_PREFIX=\"bc\"
@@ -74,33 +85,39 @@ $(error Unsupported COIN - use bitcoin_testnet, bitcoin)
 endif
 endif
 
-APP_LOAD_PARAMS += $(APP_LOAD_FLAGS)
+# Application icons following guidelines:
+# https://developers.ledger.com/docs/embedded-app/design-requirements/#device-icon
+ICON_NANOS = icons/nanos_app_bitcoin.gif
+ICON_NANOX = icons/nanox_app_bitcoin.gif
+ICON_NANOSP = icons/nanox_app_bitcoin.gif
+ICON_STAX = icons/stax_app_bitcoin.gif
 
-ifeq ($(TARGET_NAME),TARGET_NANOS)
-ICONNAME=icons/nanos_app_bitcoin.gif
-else ifeq ($(TARGET_NAME),TARGET_STAX)
-ICONNAME=icons/stax_app_bitcoin.gif
-else
-ICONNAME=icons/nanox_app_bitcoin.gif
-endif
+########################################
+# Application communication interfaces #
+########################################
+ENABLE_BLUETOOTH = 1
 
-all: default
+########################################
+#         NBGL custom features         #
+########################################
+ENABLE_NBGL_QRCODE = 1
 
-# TODO: double check if all those flags are still relevant/needed (was copied from legacy app-bitcoin)
+########################################
+#          Features disablers          #
+########################################
+# Don't use standard app file to avoid conflicts for now
+DISABLE_STANDARD_APP_FILES = 1
 
-DEFINES   += APPNAME=\"$(APPNAME)\"
-DEFINES   += APPVERSION=\"$(APPVERSION)\"
-DEFINES   += MAJOR_VERSION=$(APPVERSION_M) MINOR_VERSION=$(APPVERSION_N) PATCH_VERSION=$(APPVERSION_P)
-DEFINES   += OS_IO_SEPROXYHAL
-DEFINES   += HAVE_SPRINTF HAVE_SNPRINTF_FORMAT_U
-DEFINES   += HAVE_IO_USB HAVE_L4_USBLIB IO_USB_MAX_ENDPOINTS=4 IO_HID_EP_LENGTH=64 HAVE_USB_APDU
-DEFINES   += LEDGER_MAJOR_VERSION=$(APPVERSION_M) LEDGER_MINOR_VERSION=$(APPVERSION_N) LEDGER_PATCH_VERSION=$(APPVERSION_P) TCS_LOADER_PATCH_VERSION=0
+# Don't use default IO_SEPROXY_BUFFER_SIZE to use another
+# value for NANOS for an unknown reason.
+DISABLE_DEFAULT_IO_SEPROXY_BUFFER_SIZE = 1
 
-DEFINES   += HAVE_WEBUSB WEBUSB_URL_SIZE_B=0 WEBUSB_URL=""
+# Don't use STANDARD_USB as we want IO_USB_MAX_ENDPOINTS=4
+# and the default is 6
+DISABLE_STANDARD_USB = 1
 
+DEFINES   += HAVE_IO_USB HAVE_L4_USBLIB IO_USB_MAX_ENDPOINTS=4 IO_HID_EP_LENGTH=64 HAVE_USB_APDU
 DEFINES   += UNUSED\(x\)=\(void\)x
-DEFINES   += APPVERSION=\"$(APPVERSION)\"
-
 DEFINES   += HAVE_BOLOS_APP_STACK_CANARY
 
 
@@ -111,24 +128,6 @@ else
 DEFINES       += IO_SEPROXYHAL_BUFFER_SIZE_B=300
 endif
 
-ifeq ($(TARGET_NAME),TARGET_STAX)
-    DEFINES    += NBGL_QRCODE
-else
-    DEFINES    += HAVE_BAGL HAVE_UX_FLOW
-    ifneq ($(TARGET_NAME),TARGET_NANOS)
-        DEFINES       += HAVE_BAGL BAGL_WIDTH=128 BAGL_HEIGHT=64
-        DEFINES       += HAVE_BAGL_ELLIPSIS # long label truncation feature
-        DEFINES       += HAVE_BAGL_FONT_OPEN_SANS_REGULAR_11PX
-        DEFINES       += HAVE_BAGL_FONT_OPEN_SANS_EXTRABOLD_11PX
-        DEFINES       += HAVE_BAGL_FONT_OPEN_SANS_LIGHT_16PX
-    endif
-endif
-
-ifeq ($(TARGET_NAME),$(filter $(TARGET_NAME),TARGET_NANOX TARGET_STAX))
-DEFINES       += HAVE_BLE BLE_COMMAND_TIMEOUT_MS=2000
-DEFINES       += HAVE_BLE_APDU # basic ledger apdu transport over BLE
-endif
-
 ifeq ($(TARGET_NAME),TARGET_NANOS)
     # enables optimizations using the shared 1K CXRAM region
     DEFINES   += USE_CXRAM_SECTION
@@ -139,81 +138,23 @@ CFLAGS    += -include debug-helpers/debug.h
 
 # DEFINES   += HAVE_PRINT_STACK_POINTER
 
-ifndef DEBUG
-        DEBUG = 0
+ifeq ($(DEBUG),10)
+    $(warning Using semihosted PRINTF. Only run with speculos!)
+    DEFINES   += HAVE_PRINTF HAVE_SEMIHOSTED_PRINTF PRINTF=semihosted_printf
 endif
 
-ifeq ($(DEBUG),0)
-        DEFINES   += PRINTF\(...\)=
-else
-        ifeq ($(DEBUG),10)
-                $(warning Using semihosted PRINTF. Only run with speculos!)
-                DEFINES   += HAVE_PRINTF HAVE_SEMIHOSTED_PRINTF PRINTF=semihosted_printf
-        else
-                ifeq ($(TARGET_NAME),TARGET_NANOS)
-                        DEFINES   += HAVE_PRINTF PRINTF=screen_printf
-                else
-                        DEFINES   += HAVE_PRINTF PRINTF=mcu_usb_printf
-                endif
-        endif
-endif
-
-
 # Needed to be able to include the definition of G_cx
 INCLUDES_PATH += $(BOLOS_SDK)/lib_cxng/src
 
-
-ifneq ($(BOLOS_ENV),)
-$(info BOLOS_ENV=$(BOLOS_ENV))
-CLANGPATH := $(BOLOS_ENV)/clang-arm-fropi/bin/
-GCCPATH   := $(BOLOS_ENV)/gcc-arm-none-eabi-5_3-2016q1/bin/
-else
-$(info BOLOS_ENV is not set: falling back to CLANGPATH and GCCPATH)
-endif
-ifeq ($(CLANGPATH),)
-$(info CLANGPATH is not set: clang will be used from PATH)
-endif
-ifeq ($(GCCPATH),)
-$(info GCCPATH is not set: arm-none-eabi-* will be used from PATH)
-endif
-
-CC      := $(CLANGPATH)clang
-CFLAGS  += -Oz
-AS      := $(GCCPATH)arm-none-eabi-gcc
-LD      := $(GCCPATH)arm-none-eabi-gcc
-LDFLAGS += -O3 -Os
-LDLIBS  += -lm -lgcc -lc
-
-include $(BOLOS_SDK)/Makefile.glyphs
-
+# Application source files
 APP_SOURCE_PATH += src
 SDK_SOURCE_PATH += lib_stusb lib_stusb_impl
 
-ifneq ($(TARGET_NAME),TARGET_STAX)
-SDK_SOURCE_PATH += lib_ux
-endif
-
-ifeq ($(TARGET_NAME),$(filter $(TARGET_NAME),TARGET_NANOX TARGET_STAX))
-    SDK_SOURCE_PATH += lib_blewbxx lib_blewbxx_impl
-endif
-
-load: all
-	python3 -m ledgerblue.loadApp $(APP_LOAD_PARAMS)
-
-load-offline: all
-	python3 -m ledgerblue.loadApp $(APP_LOAD_PARAMS) --offline
-
-delete:
-	python3 -m ledgerblue.deleteApp $(COMMON_DELETE_PARAMS)
-
-include $(BOLOS_SDK)/Makefile.rules
-
-dep/%.d: %.c Makefile
-
-
-listvariants:
-	@echo VARIANTS COIN bitcoin_testnet bitcoin
+# Allow usage of function from lib_standard_app/crypto_helpers.c
+INCLUDES_PATH  += ${BOLOS_SDK}
+APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/crypto_helpers.c
 
+include $(BOLOS_SDK)/Makefile.standard_app
 
 # Makes a detailed report of code and data size in debug/size-report.txt
 # More useful for production builds with DEBUG=0

diff --git a/bitcoin_client/ledger_bitcoin/__init__.py b/bitcoin_client/ledger_bitcoin/__init__.py
@@ -7,7 +7,7 @@
 
 from .wallet import AddressType, WalletPolicy, MultisigWallet, WalletType
 
-__version__ = '0.2.1'
+__version__ = '0.2.2'
 
 __all__ = [
     "Client",

diff --git a/bitcoin_client/ledger_bitcoin/bip380/README b/bitcoin_client/ledger_bitcoin/bip380/README
@@ -0,0 +1,4 @@
+This folder is based on https://github.com/Eunovo/python-bip380/tree/4226b7f2b70211d696155f6fd39edc611761ed0b, in turn built on https://github.com/darosior/python-bip380/commit/d2f5d8f5b41cba189bd793c1081e9d61d2d160c1.
+
+The library is "not ready for any real world use", however we _only_ use it in order to generate addresses for descriptors containing miniscript, and compare the result with the address computed by the device.
+This is a generic mitigation for any bug related to address generation on the device, like [this](https://donjon.ledger.com/lsb/019/).
diff --git a/bitcoin_client/ledger_bitcoin/bip380/__init__.py b/bitcoin_client/ledger_bitcoin/bip380/__init__.py
@@ -0,0 +1 @@
+__version__ = "0.0.3"