[WIP] Clear text representation of wallet policies (attempt using Rust C bindings)#509
Closed
bigspider wants to merge 9 commits into
Closed
[WIP] Clear text representation of wallet policies (attempt using Rust C bindings)#509bigspider wants to merge 9 commits into
bigspider wants to merge 9 commits into
Conversation
Vendor the upstream BIP-388 cleartext crates from LedgerHQ/vanadium (branch cleartext-c-bindings) under rust/. bip388-c exposes a no_std C ABI (bip388_to_cleartext / bip388_confusion_score) over a caller-provided arena and generates include/bip388.h via cbindgen; bip388 is its reference parser/renderer dependency, kept as a sibling so its "../bip388" path resolves. This replaces the hand-maintained C port + gen.py codegen of the earlier attempt: the spec codegen now stays inside the Rust build.rs. The two target/ dirs are git-ignored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cross-compile the vendored bip388-c crate to the device's Rust target (cortex-m3 -> thumbv7m-none-eabi; cortex-m35p -> thumbv8m.main-none-eabi) and link the resulting libbip388_c.a, exposing the cleartext API to the C app. Gated behind ENABLE_CLEARTEXT (default 1), which sets HAVE_CLEARTEXT and adds the crate's include dir, the -lbip388_c link, and a cargo build hooked via APP_CUSTOM_LINK_DEPENDENCIES so the lib is (re)built before the app is linked. Building with ENABLE_CLEARTEXT=0 drops the feature and needs no Rust toolchain. The device build requires cargo and the matching rustup target in the build image; only the host unit-test path is exercised in CI here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
src/common/cleartext.{c,h} expose cleartext_encode(): one call into
bip388_to_cleartext() that renders a descriptor template into NUL-terminated
lines and reports has_cleartext and the confusion score. The whole .c body is
gated on HAVE_CLEARTEXT so the app builds without the Rust lib when the feature
is disabled; the public header carries only mirrored constants (static-asserted
against the binding) so it can be included widely without pulling in bip388.h.
The scratch arena lives in static storage (8 KiB) rather than on the stack to
respect Nano X's stack budget. A rust_eh_personality stub is emitted only under
CLEARTEXT_HOST_BUILD (unit tests), never for the freestanding device build.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ui_display_register_wallet_policy() gains cleartext_lines/n_cleartext_lines
parameters and the review screen renders them as a spending-path block before
the descriptor template ("Spending policy" for a single line, otherwise
"Primary spending path" + "Spending path #N" per tapleaf). A NULL
descriptor_template now hides the raw-template pair.
Also refactor the NBGL flows onto a make_pair_list(nbPairs, wrapping) helper
that sets every nbgl_layoutTagValueList_t field explicitly, so the per-flow
"wrapping" choice (true for register/sign-message) can no longer leak between
flows via the shared static pairList.
No behavior change yet: the only caller passes NULL/0, so no cleartext is shown
until the handler is wired up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handler_register_wallet now calls cleartext_encode() on the descriptor template and shows the resulting spending-path lines when every part has a cleartext form and the confusion score is at or below CLEARTEXT_MAX_CONFUSION_SCORE; otherwise it falls back to the raw template. For plain multisig policies (multi/sortedmulti, optionally under sh/wsh, detected via is_plain_multisig() on the parsed AST) the raw descriptor template is hidden, since the cleartext fully captures the spending condition. All of this is gated on HAVE_CLEARTEXT. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CMake builds the vendored crate's host (no_std) libbip388_c.a via cargo and links it through a small `cleartext` wrapper lib (HAVE_CLEARTEXT + CLEARTEXT_HOST_BUILD). When cargo is absent the cleartext targets are skipped and test_register_wallet falls back to the no-feature path, so the suite still builds without a Rust toolchain. - test_cleartext: smoke-tests cleartext_encode() over single-sig, sortedmulti, a taproot tree, and a parse error (the audited rendering vectors live in the Rust crate's own tests, so they are not re-ported here). - test_register_wallet: the mocked UI captures whether the raw descriptor was shown; cases gain an optional `descriptor_hidden` flag, asserted only under HAVE_CLEARTEXT. The five plain-multisig vectors are marked descriptor_hidden. Also reword a cleartext.h doc comment to drop a literal "/**" that tripped -Wcomment (fatal under the app's -Werror). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #509 +/- ##
===========================================
- Coverage 72.36% 72.36% -0.01%
===========================================
Files 71 71
Lines 10347 10362 +15
Branches 1876 1879 +3
===========================================
+ Hits 7488 7498 +10
- Misses 2113 2116 +3
- Partials 746 748 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The ledger-app-builder CI image ships the Rust toolchain but not every cross target, so `cargo build --target thumbv8m.main-none-eabi` failed with "can't find crate for `core`". The lib recipe now runs `rustup target add $(RUST_TARGET)` first (best-effort: skipped if rustup is absent, letting cargo emit its own clear error). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The vendored crate's #[panic_handler] calls abort(). Linking the app pulled newlib's abort()/raise() from libc.a, whose syscall stubs (_kill, _getpid, _exit) BOLOS does not provide, failing the link with undefined symbols. Define abort() in the app (device build only) as __builtin_trap(); being a strong symbol in an app object, it satisfies the crate's reference so newlib's abort/signal machinery is never pulled in. The host unit-test build keeps libc's abort (guarded by CLEARTEXT_HOST_BUILD, alongside rust_eh_personality). A panic is unreachable for in-range policies (the binding's a-priori arena-fit check), so this is a safety backstop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Code coverage reportPer-file coverage
|
Contributor
|
elf sizes
Stack consumption summary
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrite of #494 to use C bindings directly from the Rust crate.