simplify amount_split by iterating over amount bits

[theStack]

Small optimization: rather than involving a string data type here (which needs to also be reversed and sliced), we can just directly operate on the amount integer variable by iterating over all the bits and add 2^i to the return list if bit i is set.

(A similar change was done a few weeks ago also for cashu-feni: cashubtc/cashu-feni#44).

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (5c820f9) 55.41% compared to head (71c1b72) 55.40%.

❗ Current head 71c1b72 differs from pull request most recent head bafb6d0. Consider uploading reports for the commit bafb6d0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #245      +/-   ##
==========================================
- Coverage   55.41%   55.40%   -0.02%     
==========================================
  Files          42       42              
  Lines        3315     3314       -1     
==========================================
- Hits         1837     1836       -1     
  Misses       1478     1478              

Impacted Files	Coverage Δ
cashu/core/split.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[AngusP]

IMO should also add a check that amount >= 0 and raise an Exception if not, as this behaves incorrectly if the amount is negative:

In [1]: amount = -12

In [2]: rv = []
   ...: for i in range(amount.bit_length()):
   ...:     if amount & (1 << i):  # if bit i is set, add 2**i to list
   ...:         rv.append(1 << i)
   ...: 

In [3]: rv
Out[3]: [4]  # 4 != -12 or 12 ⁉️

In [4]: amount = 12

In [5]: rv = []
   ...: for i in range(amount.bit_length()):
   ...:     if amount & (1 << i):  # if bit i is set, add 2**i to list
   ...:         rv.append(1 << i)
   ...: 

In [6]: rv
Out[6]: [4, 8]  # 8 + 4 == 12 ✅

(Though the old version would also mess up with negative amounts as bin(-12) == "-0b1100" 🤣 so the [:-2] cut is wrong)

[theStack]

Thanks, that makes sense. Added an assert amount >= value at the beginning of the function, i.e. an AssertionError exception is thrown if a negative value was passed.

[theStack]

@AngusP: Thanks for reviewing! Seems like the tests have a code path with negative amount to amount_split which triggers the newly introduced exception and makes the test fail (on master, it would return something wrong instead). Have to investigate a bit how to best fix this.

[AngusP]

@theStack

 msg = 'Mint Error: invalid split amount: -1'

    async def assert_err(f, msg):
        """Compute f() and expect an error message 'msg'."""
        try:
            await f
        except Exception as exc:
>           assert exc.args[0] == msg, Exception(
                f"Expected error: {msg}, got: {exc.args[0]}"
            )
E           AssertionError: Exception("Expected error: Mint Error: invalid split amount: -1, got: can't split negative amount")
E           assert "can't split negative amount" == 'Mint Error: ...it amount: -1'
E             - Mint Error: invalid split amount: -1
E             + can't split negative amount

tests/test_wallet.py:23: AssertionError

The test is failing just because the error message is different:

              assert "can't split negative amount" == 'Mint Error: ...it amount: -1'
---             - Mint Error: invalid split amount: -1
+++             + can't split negative amount

So should be pretty easy to fix, you've changed it to "can't split negative amount" whereas presumably some other bit of code spotted this error later

[TheRealCheebs]

In an effort to close out some of these older pull requests I have opened a new one with the rebased changed #851. If that is merged this one should be closed at the same time.