Started using files from the SDK (1st step) #418

iartemov-ledger · 2026-01-21T17:06:00Z

duplicated with SDK/lib_standard_app` modules removed
the ones with added values, but colliding name renamed to _ext
MAX_BIP32_PATH_STEPS (was 8) set to MAX_BIP32_PATH == 10 to aligned with the SDK. In any case that was not clean with the swap MAX_BIP32_PATH value that was already 10 = > it may led to an additional stack consumptionm .data size is not impacted
header files inclusion is reorganized with Claude Haiku 4.5 as follows:

- C system headers using <>
- Empty line
- Unit header (for .c files only)
- Empty line + /* SDK headers */ section using ""
- Empty line + /* Local headers */ section using ""
- All headers alphabetically sorted within each category
- No folder paths - only header file names
- No leading empty lines

5758b0f

some unit tests removed as duplicated with the SDK, others updated

…ard and test_get_extended_pubkey_nonstandard_nodisplay tests (for derivation)

…e stack consumption

codecov-commenter · 2026-01-23T09:02:45Z

Codecov Report

❌ Patch coverage is 85.71429% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.19%. Comparing base (d906a66) to head (236c751).

Files with missing lines	Patch %	Lines
src/common/buffer_ext.c	88.33%	7 Missing ⚠️
src/common/parser_ext.c	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #418      +/-   ##
===========================================
- Coverage    84.58%   84.19%   -0.39%     
===========================================
  Files           17       10       -7     
  Lines         2231     1829     -402     
===========================================
- Hits          1887     1540     -347     
+ Misses         344      289      -55

Flag	Coverage Δ
unittests	`84.19% <85.71%> (-0.39%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… SDK

bigspider · 2026-01-23T13:38:40Z

src/common/bip32_ext.h

+
+
+#define MAX_BIP44_ACCOUNT_RECOMMENDED       100
+#define MAX_BIP44_ADDRESS_INDEX_RECOMMENDED 50000


Since we have a constants.h file, that seems like a good place for these, so this file can be deleted altogether.

bigspider · 2026-01-23T13:40:40Z

Makefile

    $(warning Using semihosted PRINTF. Only run with speculos!)
    DEFINES   += HAVE_PRINTF HAVE_SEMIHOSTED_PRINTF PRINTF=semihosted_printf
 endif



Typo usigng in commit message

bigspider · 2026-01-23T13:48:53Z

src/common/bip32_ext.h

 * Note: BIP32 allows up to 256 derivation steps - but only 5 or 6 are used in most cases.
 */
-#define MAX_BIP32_PATH_STEPS 8
+#define MAX_BIP32_PATH_STEPS MAX_BIP32_PATH


Not sure about this change.

I think lib_standard_app/bip32 shouldn't have an opinion about the number of derivation steps - that's a decision of apps. IMHO MAX_BIP32_PATH should actually be deleted from lib_standard_app.

There is a reason there are (or were) two constants MAX_BIP32_PATH_STEPS and MAX_BIP32_PATH: one indicates how many derivation steps are OK for a path used in a wallet policy. But the actual keys (for example as they appear in the PSBT) will have 2 more derivation steps (because of the additional /change/addr_index part. So there is the need of distinguishing these two quantities.

The name of the constants was indeed not great, and the bigger one should be computer as "smaller one + 2", and moved in the same place.

Copilot

Pull request overview

This PR migrates the Bitcoin app to use standard SDK modules from lib_standard_app instead of maintaining duplicated implementations, representing the first step in a broader SDK integration effort. The changes remove redundant code, reorganize header file structure following a standardized pattern, and align the codebase with SDK conventions.

Changes:

Removed local implementations of utilities (bip32, base58, buffer, format, read, write, varint, apdu_parser) now provided by SDK
Created _ext variants (buffer_ext, bip32_ext, parser_ext) for app-specific extensions beyond SDK functionality
Reorganized all header includes with standard pattern: C system headers, unit header, SDK headers, local headers (all alphabetically sorted)
Updated MAX_BIP32_PATH_STEPS from 8 to 10 to align with SDK's MAX_BIP32_PATH
Removed duplicate unit tests now covered by SDK, added new tests for TR musig scenarios

Reviewed changes

Copilot reviewed 118 out of 118 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/common/buffer.c/h	Removed - now using SDK's buffer implementation
src/common/read.c/h	Removed - now using SDK's read utilities
src/common/write.c/h	Removed - now using SDK's write utilities
src/common/varint.c/h	Removed - now using SDK's varint implementation
src/common/format.c/h	Removed - now using SDK's format utilities
src/common/bip32.c/h	Removed - now using SDK's BIP32 implementation
src/common/base58.c/h	Removed - now using SDK's base58 encoding
src/common/buffer_ext.c/h	New file providing app-specific buffer extensions with const-cast workarounds
src/common/bip32_ext.h	New file providing app-specific BIP32 constants (MAX_BIP32_PATH_STEPS, etc)
src/common/parser_ext.c/h	Renamed from parser.c/h, provides app-specific parser extensions
src/boilerplate/apdu_parser.c/h	Removed - functionality moved to parser.h from SDK
src/boilerplate/constants.h	Removed - constants moved to src/constants.h
src/boilerplate/offsets.h	Removed - no longer needed
src/swap/swap_lib_calls.h	Removed - now using SDK's version
unit-tests/test_*.c	Removed duplicate tests (write, format, bip32, base58, apdu_parser) covered by SDK
unit-tests/test_wallet.c	Added tests for TR musig scriptpath and keypath scenarios
unit-tests/CMakeLists.txt	Updated to use SDK paths and removed duplicate test executables
Makefile	Added SDK source files to build configuration
src/*/.c/h	Comprehensive header reorganization following new standard pattern with SDK vs local header separation

Comments suppressed due to low confidence (1)

src/common/parser_ext.c:121

These casts to (uint8_t *) are needed because the SDK's buffer_t.ptr changed to const uint8_t *. However, these lines use memmove to write to the buffer, which violates the const correctness. This could lead to undefined behavior if the buffer is actually const. Verify that buffers[0]->ptr is mutable in this context, or consider using a different approach that respects const-ness.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T14:58:00Z

src/swap/handle_check_address.h

@@ -1,5 +1,7 @@
 #pragma once

+


Extra blank line between pragma once and the Local headers comment. According to the header organization guidelines stated in the PR description, there should be no leading empty lines.

Copilot · 2026-01-23T14:58:01Z

src/musig/musig_sessions.h

+
+

Extra blank line between #include <stdbool.h> and the Local headers comment. According to the header organization guidelines stated in the PR description, there should be no leading empty lines.

Copilot · 2026-01-23T14:58:01Z

src/debug-helpers/debug.c

+
+
+/* Local headers */


Extra blank lines between #include directives and the Local headers comment. According to the header organization guidelines stated in the PR description, there should be no leading empty lines.

Copilot · 2026-01-23T14:58:01Z

src/handler/sign_psbt/txhashes.c


    size_t data_len = data->size - data->offset;
-    uint8_t *data_start_ptr = data->ptr + data->offset;
+    uint8_t *data_start_ptr = (uint8_t *) data->ptr + data->offset;


This cast to (uint8_t *) is added because buffer_t.ptr changed from uint8_t * to const uint8_t * in the SDK. However, data_start_ptr is used later to write to this memory (in crypto_hash_update). If buffer_t.ptr is truly const in the SDK, this cast-away-const could lead to undefined behavior. Verify that the SDK's buffer_t.ptr is not actually const, or that this usage pattern is safe.

Copilot · 2026-01-23T14:58:02Z

src/common/buffer_ext.c

+        return false;
+    }
+
+    memmove((uint8_t *) (buffer->ptr + buffer->offset), data, n);


This cast away const is problematic. The buffer is being written to via the cast pointer. If the SDK's buffer_t.ptr is truly const, this violates const correctness and could lead to undefined behavior. Consider verifying that buffer->ptr is mutable in write contexts, or redesign the buffer API to handle const-ness properly.

Copilot · 2026-01-23T14:58:02Z

src/swap/handle_swap_sign_transaction.h

@@ -1,5 +1,7 @@
 #pragma once

+


Extra blank line between pragma once and the Local headers comment. According to the header organization guidelines stated in the PR description, there should be no leading empty lines.

Copilot · 2026-01-23T14:58:02Z

src/swap/handle_get_printable_amount.h

@@ -1,5 +1,7 @@
 #pragma once

+


Extra blank line between pragma once and the Local headers comment. According to the header organization guidelines stated in the PR description, there should be no leading empty lines.

Copilot · 2026-01-23T14:58:02Z

src/common/buffer_ext.c

+    ((uint8_t *) buffer->ptr)[buffer->offset] = value;
+    buffer_seek_cur(buffer, 1);
+
+    return true;
+}
+
+bool buffer_write_u16(buffer_t *buffer, uint16_t value, endianness_t endianness) {
+    if (!buffer_can_read(buffer, 2)) {
+        return false;
+    }
+
+    if (endianness == BE) {
+        write_u16_be((uint8_t *) buffer->ptr, buffer->offset, value);


Cast away const on buffer->ptr. This function writes to the buffer via the cast pointer. If the SDK's buffer_t.ptr is const uint8_t *, this violates const correctness. Verify that buffer->ptr is mutable in write contexts.

Copilot · 2026-01-23T14:58:03Z

src/common/buffer_ext.c

+        write_u32_be((uint8_t *) buffer->ptr, buffer->offset, value);
+    } else {
+        write_u32_le((uint8_t *) buffer->ptr, buffer->offset, value);
+    }
+    buffer_seek_cur(buffer, 4);
+
+    return true;
+}
+
+bool buffer_write_u64(buffer_t *buffer, uint64_t value, endianness_t endianness) {
+    if (!buffer_can_read(buffer, 8)) {
+        return false;
+    }
+
+    if (endianness == BE) {
+        write_u64_be((uint8_t *) buffer->ptr, buffer->offset, value);


Cast away const on buffer->ptr. This function writes to the buffer via the cast pointer. If the SDK's buffer_t.ptr is const uint8_t *, this violates const correctness. Verify that buffer->ptr is mutable in write contexts.

Copilot · 2026-01-23T14:58:03Z

src/ui/menu_nbgl.c

+/* SDK headers */
+/* SDK headers */


Duplicate "/* SDK headers */" comment on consecutive lines. Remove one of them.

Started usigng files from the SDK (1st step)

94f23dd

iartemov-ledger marked this pull request as draft January 21, 2026 17:06

iartemov-ledger added 12 commits January 22, 2026 10:39

Externalizing bip32 module

95f991f

Externalizing format module

c15767f

Externalizing read module

6862cc8

Externalizing varint module

18d5c2f

Externalizing write module

1468ba6

Removing offset.h as we can use the one from the SDK

0cd4a69

Externalizing parser module

8248465

clang-format fixes

32dde3f

Fixing bip32_path_len=0 issues for test_get_extended_pubkey_non_stand…

af53f4f

…ard and test_get_extended_pubkey_nonstandard_nodisplay tests (for derivation)

Alligning to MAX_BIP32_PATH from the SDK, that is 10. It will increas…

3de63a2

…e stack consumption

Adaptation to unit tests

2a14100

Unit tests fixes

b743317

iartemov-ledger added 5 commits January 23, 2026 10:32

Renaming local io module to io_ext to distinguish from the one in the…

678f665

… SDK

Removing duplicated with the SDK headers

7c310c7

Removing duplicated with the SDK headers (2)

ea12ef5

Re-arranging header files inclusion (AI)

5758b0f

Re-arranging header files inclusion (AI) - adapting UT

236c751

iartemov-ledger requested a review from bigspider January 23, 2026 13:28

bigspider reviewed Jan 23, 2026

View reviewed changes

iartemov-ledger requested a review from Copilot January 23, 2026 14:50

Copilot started reviewing on behalf of iartemov-ledger January 23, 2026 14:51 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes



		#define MAX_BIP44_ACCOUNT_RECOMMENDED 100
		#define MAX_BIP44_ADDRESS_INDEX_RECOMMENDED 50000

Started using files from the SDK (1st step) #418

Are you sure you want to change the base?

Started using files from the SDK (1st step) #418

Uh oh!

Conversation

iartemov-ledger commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bigspider Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

bigspider Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

bigspider Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iartemov-ledger commented Jan 21, 2026 •

edited

Loading

codecov-commenter commented Jan 23, 2026 •

edited

Loading