selfcustody / krux

Open-source signing device firmware for Bitcoin
https://selfcustody.github.io/krux/
Other
175 stars 34 forks source link

Mnemonic Editor, East Asian Translations and WonderMV #446

Closed odudex closed 2 weeks ago

odudex commented 1 month ago

Description

This PR addresses the feature requests from issues #401 and #380 with the new mnemonic edition page. It introduces Simplified Chinese and the long-awaited Korean translation and implements the necessary infrastructure to support different form factors for glyphs, such as those used in East Asian languages. Additionally, this PR adds support for a new device, the WonderMV, from a new manufacturer, Hiwonder.

What is the purpose of this pull request?

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 96.43917% with 12 lines in your changes missing coverage. Please review.

Project coverage is 94.82%. Comparing base (6ceb7e8) to head (fa635d3). Report is 68 commits behind head on develop.

Files with missing lines Patch % Lines
src/krux/display.py 76.47% 4 Missing :warning:
src/krux/pages/login.py 87.50% 2 Missing :warning:
src/krux/pages/mnemonic_editor.py 98.97% 2 Missing :warning:
src/krux/input.py 0.00% 1 Missing :warning:
src/krux/light.py 87.50% 1 Missing :warning:
src/krux/pages/__init__.py 97.77% 1 Missing :warning:
src/krux/pages/qr_view.py 93.33% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #446 +/- ## =========================================== + Coverage 94.21% 94.82% +0.60% =========================================== Files 59 60 +1 Lines 7259 7434 +175 =========================================== + Hits 6839 7049 +210 + Misses 420 385 -35 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tadeubas commented 3 weeks ago

RAM at the menu screen, sequence executed from left to right in one boot only – Yahboom (locale pt-BR previous saved)

Login (after boot) Tools Tools > generate QR code - str: a Settings Settings > HW > Printer > CNC Load mnemonic Load mnemonic > Manual > Tiny Seed Load mnemonic > Manual > Tiny Seed > 12w > Load Wallet Home (right after load) Backup Backup > Other formats > Number
develop 1429728 1425088 1421056 1417664 1409920 1420640 1413248 1375168 1378400 1375520 1371840
wonder_mv 1419872 1415232 1411200 1407808 1400064 1410784 1403520 1363424 1367136 1364352 1359552
diff 9856 9856 9856 9856 9856 9856 9728 11744 11264 11168 12288
odudex commented 3 weeks ago

This PR introduces two new languages and would greatly benefit from the implementation of #451.

tadeubas commented 3 weeks ago

Sure! I was just running the same tests because you were concerned about RAM usage... did you see that the 11KB reduction in free memory in HOME increases with an additional 1KB by going to the Backup > Other Formats > Number menu? You said in #449 that this was a direction you weren't willing to follow... Have you tested #451 in this PR to see if there is a significant reduction in the RAM issue you're concerned about?

odudex commented 3 weeks ago

Sure! I was just running the same tests because you were concerned about RAM usage... did you see that the 11KB reduction in free memory in HOME increases with an additional 1KB by going to the Backup > Other Formats > Number menu? You said in #449 that this was a direction you weren't willing to follow... Have you tested #451 in this PR to see if there is a significant reduction in the RAM issue you're concerned about?

I haven't noticed the memory gain after going to Backup->Numbers. Do you have a clue on what could be happening? I'm sure there are many things to learn and opportunities to improve RAM usage!

I have tested #451 on this, and it rocks! It goes from +10K to -29K in comparison with develop branch

tadeubas commented 3 weeks ago

No, I don't 😞 ... I'm sure there are a lot of things to optimize, but for what purpose? Why do we need to look at KB of memory when we still have MB free?

odudex commented 3 weeks ago

To get the signing capacity from 10 to 100+ inputs, it was a byte-by-byte effort. It's a scarce asset we will always want available to implement new features. Even if were running on a computer, RAM efficiency is essential and goes hand in hand with simplicity, speed, and security.

tadeubas commented 3 weeks ago

So we shouldn't aim to test RAM usage for every little piece of code, as that takes a lot of time and effort, but rather to unimport things, especially when we'll need the most RAM, i.e. when the user signs a PSBT. That's where we should focus on looking at the available RAM. This is easily achieved by removing the signing process from the Home Page and creating some logic at boot to handle the exit of the Page to do the signing process, then return to the Home Page at the respective menu when the signing finishes.