Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pyln: failing test msat from float str #4237

Merged

Conversation

m-schmoock
Copy link
Collaborator

Currently we are not able to create pyln Millisatoshi from floats, e.g.:

  • "0.01btc"
  • "0.1sat"
  • ...

This adds a test that is currently expected to fail because of this bug.

Changelog-None

@rustyrussell
Copy link
Contributor

Hmm, how's this?

diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py
index 4a7ee33f0..c970edaac 100644
--- a/contrib/pyln-client/pyln/client/lightning.py
+++ b/contrib/pyln-client/pyln/client/lightning.py
@@ -49,11 +49,11 @@ class Millisatoshi:
         """
         if isinstance(v, str):
             if v.endswith("msat"):
-                self.millisatoshis = int(v[0:-4])
+                self.millisatoshis = Decimal(v[0:-4])
             elif v.endswith("sat"):
-                self.millisatoshis = int(v[0:-3]) * 1000
+                self.millisatoshis = Decimal(v[0:-3]) * 1000
             elif v.endswith("btc"):
-                self.millisatoshis = int(v[0:-3]) * 1000 * 10**8
+                self.millisatoshis = Decimal(v[0:-3]) * 1000 * 10**8
             else:
                 raise TypeError(
                     "Millisatoshi must be string with msat/sat/btc suffix or"

(untested!)

Changelog-fixed: pyln: parsing msat from a float string
We were not able to create pyln Millisatoshi from floats, e.g.:
 - "0.01btc"
 - "0.1sat"
 - ...

This adds a test that makes sure this won't happen again.
@m-schmoock
Copy link
Collaborator Author

@rustyrussell I took your suggestion except setting the Decimal to an intermediate variable, which was necessary so we do not get flake8 nits: error: Incompatible types in assignment (expression has type "int", variable has type "Decimal")

I also extended the testcase a bit

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack a32a686

Thanks!

@rustyrussell rustyrussell merged commit 7bfb5f1 into ElementsProject:master Dec 2, 2020
@m-schmoock m-schmoock deleted the pyln/fix_millisatoshi_floats branch December 2, 2020 09: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.

2 participants