Skip to content

Add unified Minotari Ledger installer#7864

Open
tzmWW wants to merge 15 commits into
tari-project:developmentfrom
tzmWW:bounty/7795-ledger-installer
Open

Add unified Minotari Ledger installer#7864
tzmWW wants to merge 15 commits into
tari-project:developmentfrom
tzmWW:bounty/7795-ledger-installer

Conversation

@tzmWW

@tzmWW tzmWW commented May 29, 2026

Copy link
Copy Markdown

Fixes #7795

Summary

  • Add one cross-platform Python installer for the Minotari Ledger Wallet app.
  • Detect supported Ledger devices with ledgerwallet target IDs and report the original Nano S as unsupported.
  • Resolve a matching Tari release artifact, require the .zip.sha256 sidecar, verify it, and safely extract the archive.
  • Install current minotari_ledger_wallet.apdu artifacts through ledgerblue.runScript --scp; legacy manifest fallback is intentionally not supported.
  • Keep the existing per-model scripts as thin compatibility wrappers around the unified auto-detecting installer.
  • Stage the unified installer and wrappers as release assets so downloaded wrapper assets remain runnable.

Correctness Note

The current stable Tari release may not include Ledger wallet assets, while pre-releases do. The default lookup scans recent non-draft releases, including pre-releases, and only selects a Ledger artifact when both the model zip and matching checksum sidecar are present.

Testing

  • uv run python applications/minotari_ledger_wallet/wallet/install_scripts/test_install_minotari_ledger.py
  • uv run python applications/minotari_ledger_wallet/wallet/install_scripts/install_minotari_ledger.py --help
  • uv run python -m py_compile applications/minotari_ledger_wallet/wallet/install_scripts/install_minotari_ledger.py applications/minotari_ledger_wallet/wallet/install_scripts/test_install_minotari_ledger.py
  • uvx ruff check applications/minotari_ledger_wallet/wallet/install_scripts/install_minotari_ledger.py applications/minotari_ledger_wallet/wallet/install_scripts/test_install_minotari_ledger.py
  • Repo-layout wrapper --help smoke test
  • Flat release-asset wrapper --help smoke test
  • git diff --check
  • Real Nano S Plus hardware install succeeded after updating device firmware to a compatible Ledger OS version.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a unified Python-based installer (install_minotari_ledger.py) along with unit tests to replace the previous model-specific shell and PowerShell scripts, which are now thin compatibility wrappers. The reviewer provided valuable feedback identifying three key issues in the new installer: a process replacement issue on Windows when using os.execve that causes overlapping shell output, a security vulnerability in parse_sha256_file where incorrect checksums could pass verification under certain conditions, and a potential crash if the Content-Length header contains an invalid integer. Addressing these issues will significantly improve the installer's robustness, security, and cross-platform compatibility.

@tzmWW

tzmWW commented Jun 8, 2026

Copy link
Copy Markdown
Author

Updated the branch:

  • Replaced custom raw APDU replay with ledgerblue.runScript --scp in install_minotari_ledger.py
  • Removed the custom APDU parser/sender and status-word handling code.
  • Kept ledgerwallet target-ID detection for robustness.

Tests:

  • Tested the installation process on a Ledger Nano S Plus. Downloaded and verified v5.4.0-pre.2
  • An outdated firmware makes the installation fail, so made sure the user is correctly prompted.

@tzmWW tzmWW requested a review from a team as a code owner June 8, 2026 22:40
@tzmWW

tzmWW commented Jun 8, 2026

Copy link
Copy Markdown
Author

Update:

  • Simplified workflow structure: removed legacy manifest fallback and old --model flag to avoid unwanted overrides.
  • Fixed asset-selection bug: same-model zips without .sha256 are skipped instead of blocking valid later assets. This is a tiny fix that add robustness for future releases.
  • Additional errors during installation are now wrapped in InstallerErrors for correct error handling and user prompting.
  • Updated installer docs and test suite accordingly.

@tzmWW tzmWW force-pushed the bounty/7795-ledger-installer branch from d95ea8c to cfef2a7 Compare June 12, 2026 13:27
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.

Create better ledger installer

1 participant