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

Initiated channels do not show total fees for closing txs in getTransactions #2936

Open
dannypaz opened this issue Apr 11, 2019 · 11 comments
Open
Labels
backend Related to the node backend software/interface (e.g. btcd, bitcoin-core) bug Unintended code behaviour help wanted P3 might get fixed, nice to have rpc Related to the RPC interface wallet The wallet (lnwallet) which LND uses

Comments

@dannypaz
Copy link
Contributor

dannypaz commented Apr 11, 2019

Background

Channels that I create (initiate) show opening tx fees, but do not show closing tx fees in either getTransactions or lncli listchaintxns

Your environment

  • v0.5.1-beta
  • Darwin (Mac OS Majave)
  • bitcoind 0.17

Steps to reproduce

When a user requests to open a channel with another node, a call to getTransactions returns the opening transaction with the correct total_fees for the opening transaction. Once you close the channel, the transaction shows up under getTransactions however total_fees remains 0 (possibly due to the way we would calculate the fee based on the multisig from the channel)

NOTE This sample data is on regtest but was also performed on mainnet, please reach out directly if you'd like tx information/logs for mainnet

{ amount: '-0.16786002',
  transaction: '8254ecc786633551d2ca5ae32f75629fd6d7940e9c284229b33edd264997910b',
  blockNumber: 6097,
  timestamp: '1555022291',
  fees: '0.00008787' },
{ amount: '0.16768165',
  transaction: '4cbef435eba3370451c53738e3acb35c4bf575877ad05286150649814d548a4c',
  blockNumber: 6127,
  timestamp: '1555022591',
  fees: '0' } ]

Expected behaviour

total_fees contains the amount of fees for the channel close transaction in getTransactions

Actual behaviour

total_fees is 0 in the closing transaction of the channel from the intiator

@halseth halseth added backend Related to the node backend software/interface (e.g. btcd, bitcoin-core) bug Unintended code behaviour help wanted P3 might get fixed, nice to have rpc Related to the RPC interface wallet The wallet (lnwallet) which LND uses labels Apr 12, 2019
@vctt94
Copy link
Contributor

vctt94 commented Apr 14, 2019

can I help with this one?

@vctt94
Copy link
Contributor

vctt94 commented Apr 14, 2019

I found out what the problem is.

Here on btcwallet https://github.com/btcsuite/btcwallet/blob/master/wallet/notifications.go#L110 the details.Debits is an empty array, therefore, not calculating the fee.

I could think on 2 fixes for that. Calculate it properly on btcwallet, what makes most sense for me.
Or we can calculate the fee at lnd's wallet at https://github.com/lightningnetwork/lnd/blob/master/lnwallet/btcwallet/btcwallet.go#L576

What do you guys suggest?

@halseth
Copy link
Contributor

halseth commented Apr 15, 2019

@vctt94 Sounds like fixing it in btcwallet is the most correct fix. What do you think? :)

@dannypaz
Copy link
Contributor Author

@vctt94 thanks a ton for grabbing this.

@halseth et.al I wanted to make sure that this was actually a bug. Technically, the transaction that comes back from LND in getTransactions is correct, because the funds are taken out of the inputs from the commitment transactions (multi-sig). At that point, your wallet is just receiving a balance cause the multi-sig paid the fees. Do you think this change would be confusing to users?

@dannypaz
Copy link
Contributor Author

IMO there needs to be a way that a user can see what fees were paid from channel opening, for accounting purposes, however I had a change of heart RE what getTransactions should be returning

@alexbosworth
Copy link
Contributor

Related issue: #2594

@carlaKC
Copy link
Collaborator

carlaKC commented Mar 27, 2020

IMO there needs to be a way that a user can see what fees were paid from channel opening, for accounting purposes

#3961 now displays channel initiator in the close summary, which allows you to determine which party paid the opening fees. Opening fees aren't specifically listed in CloseSummary, but I think it's reasonable to get this value from the channel while it's open/ examining the opening tx to determine fees?

At that point, your wallet is just receiving a balance cause the multi-sig paid the fees. Do you think this change would be confusing to users?

I'm not sure whether it makes sense for us to extract fees on lnd level, since listchaintxns is more of an on-chain api, and should remain ignorant of lightning level use cases. My vote would be for making the change in BtcWallet as @matheusd suggests.

@matheusd
Copy link
Contributor

It wasn't me but @vctt94 that suggested the fix :P

I haven't looked very closely at this but IIRC the underlying issue for fixing this on btcwallet is that the wallet doesn't consider the input of the closing transaction (i.e. the multisig output) as a wallet-owned address.

For the wallet to do that (and count the input as a debit) you'd have to import the multisig script into the wallet. Which comes with its own set of issues (the wallet would account for the input as wallet owned, it would scan for it, you'd need to be able to remove the script to reduce the watched set, etc).

There's also the larger issue of who is actually paying the closing tx's fee (was it the local wallet or the remote wallet or was it split among the two?). Unconditionally importing the script into the wallet would lead both wallets to consider as if they paid the fee.

Seems to me all accounting for ln-related operations should only happen in lnd (and getTransactions shouldn't be expected to produce correct fee information for non-completely-wallet-created transactions).

@cryptosharks131
Copy link
Contributor

I wanted to see if this issue has made any progress as it seems to be getting on the older side.

The only accounting hole in LND is the closure data, currently it seems LND is providing no fee data on any closures, even the ones I have initiated.

As a developer building on LND and using accounting to drive metrics, missing this data is not ideal. Needing to integrate with an external service or all the possible backend chain wallets to fill such a small gap seems like the wrong approach.

@Roasbeef
Copy link
Member

@cryptosharks131 have you seen https://github.com/lightninglabs/faraday? It has accounting features that use a backend node to fill in the missing fee closure information.

@cryptosharks131
Copy link
Contributor

Thanks @Roasbeef I am aware of this tool, however, few users have this installed and adding this as a dependency to run LNDg seems like a poor approach to the solution, especially for those using Umbrel type setups. I still believe it would be best if LND was aware of all of its accounting numbers. The closures dataset all seems to populate after the closure has fully confirmed and sweep txs are listed, a poll to the backend at this time should fill in this dataset? Alternatively, a gRPC available to poll txs manually from which ever backend is configured in LND already could also serve the purpose for developers to access without needing to deploy outside solutions.

In the mean time, I have started to add API integration to LNDg in v1.3.0 to attempt to get closure fees from a mempool type block explorer. Although it is not the best solution as it still can fail in some cases, it seems to be the best solution currently to get around this missing data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to the node backend software/interface (e.g. btcd, bitcoin-core) bug Unintended code behaviour help wanted P3 might get fixed, nice to have rpc Related to the RPC interface wallet The wallet (lnwallet) which LND uses
Projects
None yet
Development

No branches or pull requests

9 participants