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

Incorrect satoshi rounding #939

Closed
pokazef opened this issue Mar 24, 2018 · 3 comments · Fixed by #943
Closed

Incorrect satoshi rounding #939

pokazef opened this issue Mar 24, 2018 · 3 comments · Fixed by #943
Labels
code health Related to code commenting, refactoring, and other non-behaviour improvements

Comments

@pokazef
Copy link

pokazef commented Mar 24, 2018

Some code is based on the assumption that satoshiAmount = int64(bitcoinAmount * 1e8). This seems to be incorrect. For example, the go code:

a := 3e-8
fmt.Println(int64(a * 1e8))

prints 2, not 3.

This happens in btcwallet.go, where the value of some UTXOs is off by one satoshi which makes the wallet balance report wrong. Here, using the btcutil.NewAmount function instead (which does correct rounding) would solve the problem.

But the issue seems to be present in other places (e.g. blockchain.go) which do not depend on btcutil and thus might need to be explicitly rounded.

@zavan
Copy link

zavan commented Mar 25, 2018

I see an error that may be a result of this:

{
	"payment_error": "unable to route payment to destination: TemporaryChannelFailure: insufficient capacity in available outgoing links: need 175120982 mSAT, max available is 174696018 mSAT",
	"payment_preimage": "",
	"payment_route": null
}

Looks like it's picking up a route based on a rounded down value.

@meshcollider meshcollider added the code health Related to code commenting, refactoring, and other non-behaviour improvements label Mar 25, 2018
@Roasbeef
Copy link
Member

@zavan I don't think that's related at all, this issue is isolated to the way we convert BTC from the rpc calls into satoshis that we'll use for manipulation internally.

@zavan
Copy link

zavan commented Mar 26, 2018

@Roasbeef oh ok. That’s another problem I was getting earlier, it happened multiple times then later evolved into the ones I told you about on Twitter and #940, I can’t reproduce it anymore.

Roasbeef added a commit to Roasbeef/lnd that referenced this issue Mar 26, 2018
In this commit, we fix an existing rounding related bug in the codebase.
The RPC interface for btcd and bitcoind return values in BTC rather than
in satoshis. So in several places, we're forced to convert ourselves
manually. The existing logic attempted to do this, but didn't properly
account for rounding. As a result, our values can be off due to not
rounding incorrectly.

The fix for this is easy: simply properly use btcutil.NewAmount
everywhere which does rounding properly.

Fixes lightningnetwork#939.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Related to code commenting, refactoring, and other non-behaviour improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants