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

All amounts should be in msatoshi #1818

Closed
rustyrussell opened this issue Aug 10, 2018 · 7 comments
Closed

All amounts should be in msatoshi #1818

rustyrussell opened this issue Aug 10, 2018 · 7 comments
Labels

Comments

@rustyrussell
Copy link
Contributor

For example, reserve is in satoshi, leading to confusion about whether we're over it or not. Easier to be msatoshi everywhere....

@jwaugh
Copy link

jwaugh commented Aug 10, 2018

Thank you, @rustyrussell for posting this issue for me. Glad we were able to get my issue sorted out. Wouldn't have done that if it had all been mSats instead. 👍

@jb55
Copy link
Collaborator

jb55 commented Aug 10, 2018

Even fundchannel?

@cdecker
Copy link
Member

cdecker commented Aug 10, 2018

My logic so far was that everything on-chain should be satoshis, while off-chain is all msatoshi. There is one exception to this, which is listfunds where I thought it might be very weird showing msatoshi and satoshi amounts next to each other (outputs vs. channels). Where specifically was this mixed?

@jwaugh
Copy link

jwaugh commented Aug 11, 2018

I made the mistake of using sats instead of msats when trying to create an invoice so there wasn't enough on the other side of the channel to send back because the reserve hadn't been met. I just figured, when talking to @rustyrussell on the IRC channel that it made sense for everything Lightning-related to use mSats, since that's the base measurement.

In hindsight, it was a silly mix up on my part, and there are probably a lot more important issues to work on than something this nitpicky.

I think Rusty was just trying to make sure the UX was as clean as possible for newbies. But maybe it shouldn't be expected to be that simple yet seeing as how new it is and is being actively created as we type here. I'm learning... slowly, but surely. :) Just actually funded my second channel using c-lightning; waiting on confirmations then I can play some more.

Also, I do have a question... I didn't have my nodes up for roughly 24 hours, maybe a little more, and came back to my channel being closed. Is there something I can do about this? Maybe I should file a separate issue. I'll do that, too. Reply there, if you'd like.

@cdecker
Copy link
Member

cdecker commented Aug 22, 2018

Would it make sense to have amounts allow an optional unit suffix? So instead of saying fundchannel [peer-id] 1000 you'd say fundchannel [peer-id] 1000sat or fundchannel [peer-id] 10000000msat, or is that even worse?

@jb55
Copy link
Collaborator

jb55 commented Aug 22, 2018

@cdecker I thought the same. Right now I do fundchannel nodeid $(bcalc -n 1000 bits to sats). It would be neat if units could be explicit...

@ZmnSCPxj
Copy link
Collaborator

The struct amount_msat and parse_amount_msat wraps this for us now, and also provides msat/sat/btc units.

darosior added a commit to darosior/lightning that referenced this issue Dec 1, 2019
cheroot release(/"changes"?) notes:
    ElementsProject#218 via PR ElementsProject#219: Fix HTTP parser to return 400 on invalid major-only HTTP version in Request-Line.
    ElementsProject#198 via 9f7affe: Fix race condition when toggling stats counting in the middle of request processing.
    Improve post Python 3.9 compatibility checks.
    Fix support of abstract namespace sockets.
    ElementsProject#222 via 621f4ee: Fix socket.SO_PEERCRED constant fallback value under PowerPC
    Revisit PR ElementsProject#85 under PR ElementsProject#221. Now backports.functools_lru_cache is only required on Python 3.2 and earlier.
    CherryPy ElementsProject#1206 via PR ElementsProject#204: Fix race condition in threadpool shrink code.
    PR ElementsProject#224: Refactored “open URL” behavior in webtest to rely on retry_call. Callers can no longer pass raise_subcls or ssl_context positionally, but must pass them as keyword arguments.
    ElementsProject#231 via PR ElementsProject#232: Remove custom setup.cfg parser handling, allowing the project (including sdist) to build/run on setuptools 41.4. Now building cheroot requires setuptools 30.3 or later (for declarative config support) and preferably 34.4 or later (as indicated in pyproject.toml).
    Workers are now request-based, addressing the long-standing issue with keep-alive connections (ElementsProject#91 via PR ElementsProject#199).
    Deprecated use of negative timeouts as alias for infinite timeouts in ThreadPool.stop.
    CherryPy ElementsProject#1662 via PR ElementsProject#74: For OPTION requests, bypass URI as path if it does not appear absolute.
    CherryPy ElementsProject#1818: Restore support for None default argument to WebCase.getPage().

https://github.com/cherrypy/cheroot/blob/master/CHANGES.rst

flaky changes:
    Bugfixes - Reraise KeyboardInterrupt when running tests under pytest.

https://github.com/box/flaky/blob/v3.6.1/HISTORY.rst#361-2019-08-06

python-bitcoinlib:
    New RPC `generatetoaddress(self,numblocks,addr)`.
    Fixed Python 2.7 incompatibility.
    Various OpenSSL fixes, including a memory leak.

https://github.com/petertodd/python-bitcoinlib/blob/python-bitcoinlib-v0.10.2/release-notes.md#v0102

pytest release notes:
    A lot of misc fixes, see https://docs.pytest.org/en/latest/changelog.html.
cdecker pushed a commit that referenced this issue Dec 2, 2019
cheroot release(/"changes"?) notes:
    #218 via PR #219: Fix HTTP parser to return 400 on invalid major-only HTTP version in Request-Line.
    #198 via 9f7affe: Fix race condition when toggling stats counting in the middle of request processing.
    Improve post Python 3.9 compatibility checks.
    Fix support of abstract namespace sockets.
    #222 via 621f4ee: Fix socket.SO_PEERCRED constant fallback value under PowerPC
    Revisit PR #85 under PR #221. Now backports.functools_lru_cache is only required on Python 3.2 and earlier.
    CherryPy #1206 via PR #204: Fix race condition in threadpool shrink code.
    PR #224: Refactored “open URL” behavior in webtest to rely on retry_call. Callers can no longer pass raise_subcls or ssl_context positionally, but must pass them as keyword arguments.
    #231 via PR #232: Remove custom setup.cfg parser handling, allowing the project (including sdist) to build/run on setuptools 41.4. Now building cheroot requires setuptools 30.3 or later (for declarative config support) and preferably 34.4 or later (as indicated in pyproject.toml).
    Workers are now request-based, addressing the long-standing issue with keep-alive connections (#91 via PR #199).
    Deprecated use of negative timeouts as alias for infinite timeouts in ThreadPool.stop.
    CherryPy #1662 via PR #74: For OPTION requests, bypass URI as path if it does not appear absolute.
    CherryPy #1818: Restore support for None default argument to WebCase.getPage().

https://github.com/cherrypy/cheroot/blob/master/CHANGES.rst

flaky changes:
    Bugfixes - Reraise KeyboardInterrupt when running tests under pytest.

https://github.com/box/flaky/blob/v3.6.1/HISTORY.rst#361-2019-08-06

python-bitcoinlib:
    New RPC `generatetoaddress(self,numblocks,addr)`.
    Fixed Python 2.7 incompatibility.
    Various OpenSSL fixes, including a memory leak.

https://github.com/petertodd/python-bitcoinlib/blob/python-bitcoinlib-v0.10.2/release-notes.md#v0102

pytest release notes:
    A lot of misc fixes, see https://docs.pytest.org/en/latest/changelog.html.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants