lnwallet: use btcutil.NewAmount for BTC -> SAT conversions

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 #939.
This commit is contained in:
Olaoluwa Osuntokun 2018-03-25 19:15:05 -07:00
parent 23b3eaf548
commit af4afdb6f0
No known key found for this signature in database
GPG Key ID: 964EA263DD637C21
4 changed files with 66 additions and 21 deletions

@ -7,6 +7,7 @@ import (
"github.com/roasbeef/btcd/chaincfg/chainhash"
"github.com/roasbeef/btcd/wire"
"github.com/roasbeef/btcutil"
"github.com/lightninglabs/neutrino"
"github.com/lightningnetwork/lnd/lnwallet"
@ -77,10 +78,15 @@ func (b *BtcWallet) GetUtxo(op *wire.OutPoint, heightHint uint32) (*wire.TxOut,
return nil, err
}
// We'll ensure we properly convert the amount given in BTC to
// satoshis.
amt, err := btcutil.NewAmount(txout.Value)
if err != nil {
return nil, err
}
return &wire.TxOut{
// Sadly, gettxout returns the output value in BTC
// instead of satoshis.
Value: int64(txout.Value * 1e8),
Value: int64(amt),
PkScript: pkScript,
}, nil
@ -97,10 +103,15 @@ func (b *BtcWallet) GetUtxo(op *wire.OutPoint, heightHint uint32) (*wire.TxOut,
return nil, err
}
// Sadly, gettxout returns the output value in BTC instead of
// satoshis.
amt, err := btcutil.NewAmount(txout.Value)
if err != nil {
return nil, err
}
return &wire.TxOut{
// Sadly, gettxout returns the output value in BTC
// instead of satoshis.
Value: int64(txout.Value * 1e8),
Value: int64(amt),
PkScript: pkScript,
}, nil

@ -330,9 +330,16 @@ func (b *BtcWallet) ListUnspentWitness(minConfs int32) ([]*lnwallet.Utxo, error)
return nil, err
}
// We'll ensure we properly convert the amount given in
// BTC to satoshis.
amt, err := btcutil.NewAmount(output.Amount)
if err != nil {
return nil, err
}
utxo := &lnwallet.Utxo{
AddressType: addressType,
Value: btcutil.Amount(output.Amount * 1e8),
Value: amt,
PkScript: pkScript,
OutPoint: wire.OutPoint{
Hash: *txid,

@ -163,7 +163,11 @@ func forceStateTransition(chanA, chanB *LightningChannel) error {
func createTestChannels(revocationWindow int) (*LightningChannel,
*LightningChannel, func(), error) {
channelCapacity := btcutil.Amount(10 * 1e8)
channelCapacity, err := btcutil.NewAmount(10)
if err != nil {
return nil, nil, nil, err
}
channelBal := channelCapacity / 2
aliceDustLimit := btcutil.Amount(200)
bobDustLimit := btcutil.Amount(1300)

@ -101,12 +101,14 @@ var (
// assertProperBalance asserts than the total value of the unspent outputs
// within the wallet are *exactly* amount. If unable to retrieve the current
// balance, or the assertion fails, the test will halt with a fatal error.
func assertProperBalance(t *testing.T, lw *lnwallet.LightningWallet, numConfirms int32, amount int64) {
func assertProperBalance(t *testing.T, lw *lnwallet.LightningWallet,
numConfirms int32, amount float64) {
balance, err := lw.ConfirmedBalance(numConfirms)
if err != nil {
t.Fatalf("unable to query for balance: %v", err)
}
if balance != btcutil.Amount(amount*1e8) {
if balance.ToBTC() != amount {
t.Fatalf("wallet credits not properly loaded, should have 40BTC, "+
"instead have %v", balance)
}
@ -149,7 +151,7 @@ func calcStaticFee(numHTLCs int) btcutil.Amount {
}
func loadTestCredits(miner *rpctest.Harness, w *lnwallet.LightningWallet,
numOutputs, btcPerOutput int) error {
numOutputs int, btcPerOutput float64) error {
// For initial neutrino connection, wait a second.
// TODO(aakselrod): Eliminate the need for this.
@ -159,12 +161,15 @@ func loadTestCredits(miner *rpctest.Harness, w *lnwallet.LightningWallet,
}
// Using the mining node, spend from a coinbase output numOutputs to
// give us btcPerOutput with each output.
satoshiPerOutput := int64(btcPerOutput * 1e8)
satoshiPerOutput, err := btcutil.NewAmount(btcPerOutput)
if err != nil {
return fmt.Errorf("unable to create amt: %v", err)
}
expectedBalance, err := w.ConfirmedBalance(1)
if err != nil {
return err
}
expectedBalance += btcutil.Amount(satoshiPerOutput * int64(numOutputs))
expectedBalance += btcutil.Amount(int64(satoshiPerOutput) * int64(numOutputs))
addrs := make([]btcutil.Address, 0, numOutputs)
for i := 0; i < numOutputs; i++ {
// Grab a fresh address from the wallet to house this output.
@ -181,7 +186,7 @@ func loadTestCredits(miner *rpctest.Harness, w *lnwallet.LightningWallet,
addrs = append(addrs, walletAddr)
output := &wire.TxOut{
Value: satoshiPerOutput,
Value: int64(satoshiPerOutput),
PkScript: script,
}
if _, err := miner.SendOutputs([]*wire.TxOut{output}, 10); err != nil {
@ -278,7 +283,10 @@ func createTestWallet(tempTestDir string, miningNode *rpctest.Harness,
func testDualFundingReservationWorkflow(miner *rpctest.Harness,
alice, bob *lnwallet.LightningWallet, t *testing.T) {
const fundingAmount = btcutil.Amount(5 * 1e8)
fundingAmount, err := btcutil.NewAmount(5)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
// In this scenario, we'll test a dual funder reservation, with each
// side putting in 10 BTC.
@ -450,7 +458,10 @@ func testFundingTransactionLockedOutputs(miner *rpctest.Harness,
alice, _ *lnwallet.LightningWallet, t *testing.T) {
// Create a single channel asking for 16 BTC total.
fundingAmount := btcutil.Amount(8 * 1e8)
fundingAmount, err := btcutil.NewAmount(8)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
feeRate, err := alice.Cfg.FeeEstimator.EstimateFeePerVSize(1)
if err != nil {
t.Fatalf("unable to query fee estimator: %v", err)
@ -467,7 +478,10 @@ func testFundingTransactionLockedOutputs(miner *rpctest.Harness,
// Now attempt to reserve funds for another channel, this time
// requesting 900 BTC. We only have around 64BTC worth of outpoints
// that aren't locked, so this should fail.
amt := btcutil.Amount(900 * 1e8)
amt, err := btcutil.NewAmount(900)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
failedReservation, err := alice.InitChannelReservation(amt, amt, 0,
feePerKw, feeRate, bobPub, bobAddr, chainHash,
lnwire.FFAnnounceChannel)
@ -492,7 +506,10 @@ func testFundingCancellationNotEnoughFunds(miner *rpctest.Harness,
feePerKw := feeRate.FeePerKWeight()
// Create a reservation for 44 BTC.
fundingAmount := btcutil.Amount(44 * 1e8)
fundingAmount, err := btcutil.NewAmount(44)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
chanReservation, err := alice.InitChannelReservation(fundingAmount,
fundingAmount, 0, feePerKw, feeRate, bobPub, bobAddr, chainHash,
lnwire.FFAnnounceChannel)
@ -571,10 +588,13 @@ func testReservationInitiatorBalanceBelowDustCancel(miner *rpctest.Harness,
// We'll attempt to create a new reservation with an extremely high fee
// rate. This should push our balance into the negative and result in a
// failure to create the reservation.
fundingAmount := btcutil.Amount(4 * 1e8)
fundingAmount, err := btcutil.NewAmount(4)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
feePerVSize := lnwallet.SatPerVByte(btcutil.SatoshiPerBitcoin * 4 / 100)
feePerKw := feePerVSize.FeePerKWeight()
_, err := alice.InitChannelReservation(
_, err = alice.InitChannelReservation(
fundingAmount, fundingAmount, 0, feePerKw, feePerVSize, bobPub,
bobAddr, chainHash, lnwire.FFAnnounceChannel,
)
@ -638,7 +658,10 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness,
// First, Alice will Initialize a reservation for a channel with 4 BTC
// funded solely by us. We'll also initially push 1 BTC of the channel
// towards Bob's side.
fundingAmt := btcutil.Amount(4 * 1e8)
fundingAmt, err := btcutil.NewAmount(4)
if err != nil {
t.Fatalf("unable to create amt: %v", err)
}
pushAmt := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin)
feeRate, err := alice.Cfg.FeeEstimator.EstimateFeePerVSize(1)
if err != nil {