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

openchannel with pushsats isn't accounted for #112

Open
junderw opened this issue Apr 5, 2021 · 3 comments
Open

openchannel with pushsats isn't accounted for #112

junderw opened this issue Apr 5, 2021 · 3 comments
Labels
accounting bug Something isn't working

Comments

@junderw
Copy link

junderw commented Apr 5, 2021

When you use openchannel with the push_sats argument (ie. open channel worth 10 BTC and push 5 BTC to the other side from the beginning state)

I doubt anyone will use push_sats on mainnet in prod... but in our test environment it was very obvious since we use push_sats all the time, and everything is wonky because of it.

Also, should we be considering LOCAL_CHANNEL_OPEN capacity as debited funds?

Currently it shows amount: 10 BTC, credit: false for the channel open when I still control half the funds in the channel.

Perhaps LOCAL_CHANNEL_OPEN should only "debit":

  1. Any amount pushed in first state via push_sats
  2. Any amount that is not usable on our end (set aside for closing fees etc.) (These "set aside" things can be credited during close if we don't use them all)
@guggero guggero added accounting bug Something isn't working labels Apr 6, 2021
@carlaKC
Copy link
Contributor

carlaKC commented Apr 14, 2021

Thanks for the issue! Unfortunately push amounts weren't included because they aren't tracked after the channel has been closed (lightningnetwork/lnd#4490), and it seemed inconsistent to include them when we run the report for the open channel, but not when we run it when the channel is closed. I've created #114 to at least mention this in the docs while we wait on the blocking lnd issue.

I doubt anyone will use push_sats on mainnet in prod.

You'd be surprised 😬

Also, should we be considering LOCAL_CHANNEL_OPEN capacity as debited funds?

The rationale behind this was that the balance moves out of your on-chain wallet balance (as reflected by lnd) and then is repaid to your on-chain balance when you close out. Although this has been brought up by other users anecdotally, so perhaps there's a better way to deal with this movement of funds.

@junderw
Copy link
Author

junderw commented Apr 14, 2021

The rationale behind this was that the balance moves out of your on-chain wallet balance (as reflected by lnd) and then is repaid to your on-chain balance when you close out.

That would require an off-chain credit to go along with it.

I understand viewing on-chain and off-chain as separate wallets, but if I open a channel with 10 BTC, I have (close to) 10 BTC off-chain now... but there is no credit event in the audit so it just looks like that 10 BTC disappeared into a blackhole, then randomly payments that are debiting via off-chain payments appear, but you've never "credited" your off-chain wallet to begin with.

So to summarize, if I open a channel of 10 BTC and push_sats 5 BTC

  • debit on-chain wallet for 10 BTC
  • credit off-chain wallet for 10 BTC - anything set aside for fees
  • debit off-chain wallet for 5 BTC (for the push_sats)

If someone push_sats 5 BTC to us

  • credit off-chain wallet for 5 BTC

It seems like currently faraday only starts tracking from the point when it starts watching lnd and doesn't try to parse all past payments... so the ability to see push_sats after closing seems like it's not a big deal.

@carlaKC
Copy link
Contributor

carlaKC commented Apr 14, 2021

That would require an off-chain credit to go along with it

I think that's a good idea, thanks for the suggestion! As is some additional logic which knows this balance went off chain to use the report. Seems sensible to have a matching entry. A remote push could also show up as this same type of credit.

It seems like currently faraday only starts tracking from the point when it starts watching lnd and doesn't try to parse all past payments

This isn't the case for the audit report, it can be run for any period of time that lnd has records for (with the caveat that there may be some issues with very old payments, because lnd 0.11 was updated to help with accounting).

so the ability to see push_sats after closing seems like it's not a big deal

The problem with this is that the report needs to be consistent, you could run into all sorts of issues in this kind of scenario:

  • Run report for month of March while a channel is still open, push amount is reported
  • Run report for month of March after the channel has been closed, push amount won't be reported

We'd have different reports for the same period of time, which is very messy for auditing, which is why we need the lnd change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accounting bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants