Skip to content

fix: low entropy calc when using Mnemonic XOR#745

Open
qlrd wants to merge 3 commits intoselfcustody:developfrom
qlrd:fix/mnemonic-xor-shannon
Open

fix: low entropy calc when using Mnemonic XOR#745
qlrd wants to merge 3 commits intoselfcustody:developfrom
qlrd:fix/mnemonic-xor-shannon

Conversation

@qlrd
Copy link
Copy Markdown
Member

@qlrd qlrd commented Oct 20, 2025

What is this PR for?

This commit add a fix for shannon entropy calculation of a choosen entroypy bytes before operate the XOR between input and current mnemonic.

Changes made to:

  • Code
  • Tests
  • Docs
  • CHANGELOG

Did you build the code and tested on device?

  • Yes, build and tested on

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Docs update
  • Other

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.40%. Comparing base (573de43) to head (eb52af1).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #745      +/-   ##
===========================================
+ Coverage    97.38%   97.40%   +0.01%     
===========================================
  Files           83       83              
  Lines        10548    10586      +38     
===========================================
+ Hits         10272    10311      +39     
+ Misses         276      275       -1     

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

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from b7fd7c4 to 14e048f Compare October 20, 2025 01:17
@odudex
Copy link
Copy Markdown
Member

odudex commented Oct 20, 2025

Shannon of derivatives (variation), instead of raw data, was used in dice rolls to detect arithmetic progression patterns, but to have Shannon measurement you shouldn't derive the data.
If you're going to measure it byte-wise, the "histogram" or bucket will range from 0 to 255. But in the context of mnemonics, to have a entropy measurement maybe a 11bit words should be used, with 2048 possible values for the bucket.

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch 2 times, most recently from bc3c7fb to f0e2f25 Compare October 21, 2025 14:02
@qlrd qlrd marked this pull request as ready for review October 21, 2025 15:16
@qlrd qlrd marked this pull request as draft October 21, 2025 15:36
@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Oct 21, 2025

Tested with device and the mnemonics shared by @jdlcdl on #727.

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch 2 times, most recently from 37c45b3 to c1fc6cd Compare October 22, 2025 01:40
@qlrd qlrd marked this pull request as ready for review October 22, 2025 01:49
@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch 3 times, most recently from d4c10d9 to a6d7b50 Compare October 22, 2025 12:48
@qlrd qlrd marked this pull request as draft October 22, 2025 12:49
@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from a6d7b50 to 0e8c0e5 Compare October 22, 2025 12:52
@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Oct 22, 2025

There's another mnemonic that have low entropy (2.29 IDK which unit) on tests and not documented AFAIK.

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from 0e8c0e5 to abe2204 Compare October 22, 2025 16:42
@qlrd qlrd marked this pull request as ready for review October 22, 2025 19:13
@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from abe2204 to fd225d3 Compare October 23, 2025 12:14
@odudex
Copy link
Copy Markdown
Member

odudex commented Oct 24, 2025

I liked the mnemonic entropy measurement.
But IMO, if we're going to create a setting with a threshold, we should use the measurement when loading any type of mnemonic and show a warning in case the threshold is not met.

@tadeubas
Copy link
Copy Markdown
Member

IMHO we should not offer a settings for this, especially a default that "value was a random choice". Instead we need to ensure a "secure" value, as we already do with dice rolls and camera.

@odudex
Copy link
Copy Markdown
Member

odudex commented Oct 24, 2025

Yes

IMHO we should not offer a settings for this, especially a default that "value was a random choice". Instead we need to ensure a "secure" value, as we already do with dice rolls and camera.

Yes, this needs to be well discussed. The safest approach is to set hard-coded value that cover obvious problematic entropy, and don't overwhelm user with settings in this case.

@tadeubas
Copy link
Copy Markdown
Member

tadeubas commented Oct 24, 2025

It would be nice to somehow show the distribution, if possible - similar to how we do with dice - or at least display the entropy in bits instead of an error with 0.413817, since it’s not clear what that number represents.

@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Oct 29, 2025

I liked the mnemonic entropy measurement. But IMO, if we're going to create a setting with a threshold, we should use the measurement when loading any type of mnemonic and show a warning in case the threshold is not met.

I thought in a bits/word unit. It's not the mnemonic word, but instead the 11bit word. Maybe this could lead to a misconsception, but write it correctly on firmware could be too big.

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch 2 times, most recently from f1c308e to 474167a Compare October 29, 2025 18:41
@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Oct 30, 2025

Didn't tested on simulator and in device with new changes about warning entropy measure in both simulator and device.

In Simulator, IDK, this is happening:

❯ poetry run poe simulator
Poe => python simulator/simulator.py --device maixpy_amigo
pygame 2.6.1 (SDL 2.32.10, Python 3.14.0)
Hello from the pygame community. https://www.pygame.org/contribute.html
Traceback (most recent call last):
  File "/Users/qlrd/github/krux/simulator/simulator.py", line 49, in <module>
    parser.add_argument(
    ~~~~~~~~~~~~~~~~~~~^
        "--printer",
        ^^^^^^^^^^^^
    ...<3 lines>...
        action=argparse.BooleanOptionalAction,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/opt/homebrew/Cellar/python@3.14/3.14.0_1/Frameworks/Python.framework/Versions/3.14/lib/python3.14/argparse.py", line 1538, in add_argument
    action = action_class(**kwargs)
TypeError: BooleanOptionalAction.__init__() got an unexpected keyword argument 'type'

Didn't re-build in device because wasn't able to simulate.

@qlrd qlrd changed the title fix: fix low entropy calc when using Mnemonic XOR fix: low entropy calc when using Mnemonic XOR Oct 30, 2025
@tadeubas
Copy link
Copy Markdown
Member

Did you merge with develop?

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from 474167a to 836db0b Compare October 30, 2025 18:50
@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Oct 30, 2025

Maybe use py3.14 could be the issue?

@qlrd
Copy link
Copy Markdown
Member Author

qlrd commented Dec 5, 2025

Didn't tested on simulator and in device with new changes about warning entropy measure in both simulator and device.

In Simulator, IDK, this is happening:

❯ poetry run poe simulator
Poe => python simulator/simulator.py --device maixpy_amigo
pygame 2.6.1 (SDL 2.32.10, Python 3.14.0)
Hello from the pygame community. https://www.pygame.org/contribute.html
Traceback (most recent call last):
  File "/Users/qlrd/github/krux/simulator/simulator.py", line 49, in <module>
    parser.add_argument(
    ~~~~~~~~~~~~~~~~~~~^
        "--printer",
        ^^^^^^^^^^^^
    ...<3 lines>...
        action=argparse.BooleanOptionalAction,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/opt/homebrew/Cellar/python@3.14/3.14.0_1/Frameworks/Python.framework/Versions/3.14/lib/python3.14/argparse.py", line 1538, in add_argument
    action = action_class(**kwargs)
TypeError: BooleanOptionalAction.__init__() got an unexpected keyword argument 'type'

Didn't re-build in device because wasn't able to simulate.

Fixed and rebased.

@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from 836db0b to 97d9160 Compare December 5, 2025 16:43
qlrd added 3 commits December 5, 2025 13:55
This commit add a fix for shannon entropy validation. Before this commit
we checked for only two specific cases (zero/all-ff entropy).

We can, instead, use a factor that measures the ammount of information
have this entropy and we can avoid what could be called "dangerous
entropies for the current context": entropies that have less information
than the current mnemonic entropy.

This can be tunned by a constant called `MIN_BITS_PER_WORD11` that is a
float value measuring the amount of entropy of the involved mnemonics.
@qlrd qlrd force-pushed the fix/mnemonic-xor-shannon branch from 97d9160 to eb52af1 Compare December 5, 2025 17:03
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.

3 participants