fix: ledger builds#7260
Conversation
Fix ledger builds locally
WalkthroughThis update revises documentation for the Minotari Ledger Wallet to clarify installation instructions and standardize path formatting. Several source files reorder import statements for clarity and consistency without altering functionality. In the wallet source code, string construction for certain fields is refactored to ensure correct variable lifetime and safe referencing. Minor code simplifications and whitespace cleanups are also included. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
applications/minotari_ledger_wallet/wallet/README.md (1)
155-155: Add language specifications to fenced code blocks.The static analysis tool correctly identifies that these fenced code blocks are missing language specifications, which improves syntax highlighting and documentation clarity.
Apply this diff to add appropriate language specifications:
-``` +```bash ledgerctl install app_nanosplus.json -``` +``` -``` +```bash ledgerctl install ./target/nanosplus/release/app_nanosplus.json ledgerctl install ./target/stax/release/app_stax.json -``` +```Also applies to: 158-158
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
applications/minotari_ledger_wallet/wallet/README.md(1 hunks)applications/minotari_ledger_wallet/wallet/src/handlers/get_one_sided_metadata_signature.rs(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
applications/minotari_ledger_wallet/wallet/README.md
155-155: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
158-158: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test (mainnet, stagenet)
- GitHub Check: test (nextnet, nextnet)
- GitHub Check: test (testnet, esmeralda)
- GitHub Check: ci
- GitHub Check: cargo check with stable
- GitHub Check: Cucumber tests / Base Layer
- GitHub Check: Cucumber tests / FFI
🔇 Additional comments (4)
applications/minotari_ledger_wallet/wallet/README.md (2)
148-149: LGTM! Installation directory clarification improves usability.The explicit clarification that installation commands must be run from the Tari ledger wallet repository root enhances the documentation's clarity and reduces potential user confusion.
152-152: LGTM! Path standardization improves cross-platform compatibility.The standardization to Unix-style forward slash paths (
./target/nanosplus/release/) improves consistency and works across different operating systems.Also applies to: 159-160
applications/minotari_ledger_wallet/wallet/src/handlers/get_one_sided_metadata_signature.rs (2)
136-136: LGTM! String lifetime management improvement for Amount field.Creating a named
field_valuevariable ensures the formatted string lives long enough to be referenced by theFieldstruct, preventing potential lifetime issues.Also applies to: 139-139
141-145: LGTM! String lifetime management improvement for Receiver field.The same pattern is correctly applied for the receiver address field, ensuring proper string lifetime management while maintaining the same display logic.
Test Results (CI) 3 files 126 suites 57m 22s ⏱️ Results for commit d005d21. ♻️ This comment has been updated with latest results. |
Test Results (Integration tests) 2 files + 2 10 suites +10 1h 0m 13s ⏱️ + 1h 0m 13s For more details on these failures, see this check. Results for commit d005d21. ± Comparison against base commit 86539c8. ♻️ This comment has been updated with latest results. |
applications/minotari_ledger_wallet/wallet/src/handlers/get_one_sided_metadata_signature.rs
Outdated
Show resolved
Hide resolved
leet4tari
left a comment
leet4tari
left a comment
There was a problem hiding this comment.
utACK - LGTM - seen build in fork.
|
How does adding these fields fix the build? |
There were clippy errors |
* development: chore: fix regression bug (tari-project#7276) feat!: expand gRPC readiness status to contain current processed block info (tari-project#7262) fix!: payref migration and indexes, add grpc query via output hash (tari-project#7266) feat: auto zero value coinbase reward calculation (tari-project#7259) feat!: improve grpc token supply (tari-project#7261) chore: new release v4.7.0-pre.0 (tari-project#7268) fix: get_all_completed_transactions limit issues (tari-project#7267) fix: ledger builds (tari-project#7260) feat: offline signing (tari-project#7122) test: verify accumulated difficulty (tari-project#7243)
5 participants
Description
Fixed ledger builds locally using the latest ledger docker image; however, the objective is to fix the ledger wallet auto-builds in GitHub, and this may still need some tweaks in follow-up commits.
Motivation and Context
Ledger auto-builds in GitHub were failing.
How Has This Been Tested?
https://github.com/hansieodendaal/tari/actions/runs/15869532430
What process can a PR reviewer use to test or verify this change?
Code review.
Check GitHub actions output.
Breaking Changes
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
Refactor