Skip to content

Retrieve appflags from app.elf and use it for derivation#618

Merged
nroggeman-ledger merged 2 commits intomasterfrom
support-appflags-from-elf
Dec 4, 2025
Merged

Retrieve appflags from app.elf and use it for derivation#618
nroggeman-ledger merged 2 commits intomasterfrom
support-appflags-from-elf

Conversation

@nroggeman-ledger
Copy link
Copy Markdown
Contributor

@nroggeman-ledger nroggeman-ledger commented Dec 2, 2025

The goal of this PR is to be able to support retrieving appflags from app.elf and use it for derivation.

It is linked to https://ledgerhq.atlassian.net/browse/B2CA-2435

Related PRs:

@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch 2 times, most recently from 79242f5 to 155e3c7 Compare December 2, 2025 15:43
Copy link
Copy Markdown
Contributor

@iartemov-ledger iartemov-ledger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1st round, as it is late evening.

@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from 155e3c7 to 4ee5dc3 Compare December 3, 2025 08:39
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 11.40351% with 101 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.71%. Comparing base (9042e33) to head (ff54461).

Files with missing lines Patch % Lines
src/bolos/os_bip32.c 17.54% 45 Missing and 2 partials ⚠️
speculos/main.py 0.00% 35 Missing ⚠️
src/launcher.c 0.00% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #618      +/-   ##
==========================================
- Coverage   40.94%   40.71%   -0.23%     
==========================================
  Files         132      132              
  Lines       14106    14215     +109     
  Branches     1256     1275      +19     
==========================================
+ Hits         5775     5787      +12     
- Misses       7951     8046      +95     
- Partials      380      382       +2     

☔ 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.

@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from 4ee5dc3 to cf1f5a9 Compare December 3, 2025 09:28
@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch 2 times, most recently from a0c9ea1 to 17658fa Compare December 3, 2025 13:35
@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from 17658fa to f575980 Compare December 3, 2025 14:47
@nroggeman-ledger nroggeman-ledger changed the title Retrieve appflags from elf in install_parameters and use it for derivation Retrieve appflags from app.elf and use it for derivation Dec 3, 2025
@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from f575980 to 9401842 Compare December 3, 2025 15:00
@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from 9401842 to 4d03434 Compare December 3, 2025 15:22
Copy link
Copy Markdown
Contributor

@iartemov-ledger iartemov-ledger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for BTC !

I'd real like to have the real exception value for the wrong path checking, 0x4212 in this case. Why to wait a new PR. We're not obliged to change everything, but can correct values gradually.

iartemov-ledger
iartemov-ledger previously approved these changes Dec 4, 2025
Copy link
Copy Markdown
Contributor

@iartemov-ledger iartemov-ledger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested:

  • attempt to derive at root-level without HAVE_APPLICATION_FLAG_DERIVE_MASTER (future test_get_extended_pubkey_non_standard BTC test)
  • attempt to BIP32 derivation with a path different from PATH_APP_LOAD_PARAMS (future test_get_extended_pubkey_non_standard BTC test)
  • attempt to SLIP21 derivation with a path different from PATH_SLIP21_APP_LOAD_PARAMS (an app modification is required)
  • attempt to use a curve different from CURVE_APP_LOAD_PARAMS (an app modification is required)

iartemov-ledger
iartemov-ledger previously approved these changes Dec 4, 2025
@nroggeman-ledger nroggeman-ledger force-pushed the support-appflags-from-elf branch from 5e565d2 to ff54461 Compare December 4, 2025 13:57
@nroggeman-ledger nroggeman-ledger merged commit 8c1741f into master Dec 4, 2025
55 checks passed
@nroggeman-ledger nroggeman-ledger deleted the support-appflags-from-elf branch December 4, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants