lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.65k stars 2.07k forks source link

`SendOuputs` fee estimation is incorrect #6873

Closed benthecarman closed 2 years ago

benthecarman commented 2 years ago

Background

I am calling SendOuputs with satPerKw = 1000 and it creates a tx with a weight of 568 but fees of 932.

This seems like it only happens with taproot inputs

Your environment

Steps to reproduce

scala code of my test case:

    val sendAmt = Satoshis(10000)
    val feeRate = SatoshisPerKW.fromLong(1000)

    for {
      oldBalA <- lndA.walletBalance().map(_.balance)
      oldBalB <- lndB.walletBalance().map(_.balance)

      addr <- lndB.getNewAddress
      output = TransactionOutput(sendAmt, addr.scriptPubKey)

      tx <- lndA.sendOutputs(Vector(output), feeRate, spendUnconfirmed = false)
      _ <- lndA.publishTransaction(tx)
      _ <- bitcoind.getNewAddress.flatMap(bitcoind.generateToAddress(6, _))

      detailsOpt <- lndA.getTransaction(tx.txIdBE)
      _ = assert(detailsOpt.isDefined)
      details = detailsOpt.get

      newBalA <- lndA.walletBalance().map(_.balance)
      newBalB <- lndB.walletBalance().map(_.balance)
    } yield ...

Expected behaviour

correctly estimate fee

Actual behaviour

fee is off by a large margin

Roasbeef commented 2 years ago

Looks like this is the culprit: https://github.com/btcsuite/btcwallet/blob/5aafe47898507963ae099739e33ff5be9e5620c3/wallet/txauthor/author.go#L111-L125

We then end up using a default size for p2pkh, which is smaller than p2tr.

benthecarman commented 2 years ago

looks like here as well: https://github.com/btcsuite/btcwallet/blob/6ab9b615576f4a86c31bcaef9eddee060b36da00/wallet/txsizes/size.go#L207

Roasbeef commented 2 years ago

I think fee estimation within the main daemon (so the sweeper, and funding inputs, etc) should be correct however.

guggero commented 2 years ago

I think fee estimation within the main daemon (so the sweeper, and funding inputs, etc) should be correct however.

Yeah, I did a code sweep and searched for all occurrences of fee estimation. But it looks like I missed some code in btcwallet.

benthecarman commented 2 years ago

Any blockers on this? I've had a fix for the underlying issue open for ~2 weeks now

https://github.com/btcsuite/btcwallet/pull/809