lightninglabs / pool

Lightning Pool: a non-custodial batched uniform clearing-price auction for Lightning Channel Leases (LCL). An LCL packages up inbound channel liquidity (ability to receive funds) as a fixed income asset with a maturity date expressed in blocks.
MIT License
255 stars 47 forks source link

Expose historical account modification fees via RPC #386

Closed jamaljsr closed 1 year ago

jamaljsr commented 2 years ago

Is your feature request related to a problem? Please describe. i am attempting to calculate historical fees spent while managing my pool account. For each action (init, deposit, withdraw, renew, close), I can see an entry in the response of lnrpc.Lightning.GetTransactions with a label prefixed with "poold --". Unfortunately, the total_fees field of the txns are set to 0 for all except init.

When I query a block explorer using one of the txids, I can see the fee that was paid. It looks like the fees are just being deducted from the account utxo instead of coming from the LND wallet, which makes sense. But this makes it challenging to capture the fees.

Describe the solution you'd like I'd like to be able to obtain the historical fees via RPC for all account modifications. It would be great if there was just a single RPC that returned the timestamped txid w/ fee for all account modifications on my node, similar to how poolrpc.Trader.Leases returns the premium/execution/chain fees for each channel opened in a batch.

Describe alternatives you've considered It would be possible to use a third-party web API or bitcoin node to query the transactions by id in order to obtain the fee from the chain. Ideally, it would be better to have this come directly from poold which already manages the account UTXOs and transactions.

Additional context I spoke to @Roasbeef on chat and he threw out some ideas:

the reason the wallet shows zero fees here is that: it doesn't know about those inputs that don't belong to it, so it can't compute the total fee -- we can fix this at the wallet level by doing another query, but only really for a full node backend since it has that index ideally we start to explicitly store this fee in the pool client db I think we can expose this info tho, given we store all batches we've ever been a part of, then can use the label we use for account modifications to get all our account txns ideally we do a sort of migration to scan everything, compute+store, then we can just return the output later

ffranr commented 2 years ago

The following JSON segment is an example account modification transaction where the total_fees field is set to zero. The transaction deposits 100000 sats into the account.

I don't think pool stores the deposit amount anywhere locally. If it did we could calculate the fee using the deposit amount and the amount field value (which I'm pretty sure includes the fee).

Therefore I think poold will need to calculate the fee by looking up the previous onchain outpoint values.

And alternative strategy might be to work out the fee for the below transaction by counting forward from the account creation transaction (the tx for the latter does include a total_fees value). I think this is the way to go. Does that sound right @guggero / @positiveblue ?

    {
      "tx_hash": "7a631596c39e4a4265d15cc148f6129a8a5458ee6f275a9d1b8ce886d125f808",
      "amount": "-102210",
      "num_confirmations": 0,
      "block_hash": "",
      "block_height": 0,
      "time_stamp": "1665584311",
      "total_fees": "0",
      "dest_addresses": [
        "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
        "bcrt1q6az29wpyc6mnfhykzxhq3lu4kt4wyc0266f7an"
      ],
      "output_details": [
        {
          "output_type": "SCRIPT_TYPE_PUBKEY_HASH",
          "address": "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
          "pk_script": "51205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059",
          "output_index": "0",
          "amount": "200000",
          "is_our_address": false
        },
        {
          "output_type": "SCRIPT_TYPE_WITNESS_V0_PUBKEY_HASH",
          "address": "bcrt1q6az29wpyc6mnfhykzxhq3lu4kt4wyc0266f7an",
          "pk_script": "0014d744a2b824c6b734dc9611ae08ff95b2eae261ea",
          "output_index": "1",
          "amount": "469780688",
          "is_our_address": true
        }
      ],
      "raw_tx_hex": "020000000001024c6496d8fc89e1fd0a4ec060d037d0555c2935dd80a81d753cc6fef27da5e7530000000000000000004c6496d8fc89e1fd0a4ec060d037d0555c2935dd80a81d753cc6fef27da5e75301000000000000000002400d0300000000002251205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059d048001c00000000160014d744a2b824c6b734dc9611ae08ff95b2eae261ea0140b2f1f73f47b63ca963ace29d9caf704845d744dcd945addc582eb163e68d6de63203eeff6791536d1dca8f8d1d501f141316f89a54d54c1eac496c8a3900fe79024730440220201fe6e3a19864ad46b407c4eceffafb1cbbbc7d2d9d9b8895988ebc046ac1c6022077dab219f8acc7b7e7fba2a17e7a0f30c455f14ee94b90d9d3c894301d31cad30121030717b8f30d889d2d09f35a124df9f01f495172b7e57e23d88f51483f62cdeb2500000000",
      "label": " poold -- AccountModification(acct_key=0226333ad7d53a862ea6f6daae81997b62e4075c5d2b2db6b2676f32c827c736f2, expiry=false, deposit=true, is_close=false)",
      "previous_outpoints": [
        {
          "outpoint": "53e7a57df2fec63c751da880dd35295c55d037d060c04e0afde189fcd896644c:0",
          "is_our_output": false
        },
        {
          "outpoint": "53e7a57df2fec63c751da880dd35295c55d037d060c04e0afde189fcd896644c:1",
          "is_our_output": true
        }
      ]
    }
guggero commented 2 years ago

I took a quick look at how we store/publish the transactions in Pool. Unfortunately I think we can only solve this in a very hacky way without first starting to store each account modification.

calculate the fee by looking up the previous onchain outpoint values.

The thing is, we cannot easily look up any transaction on chain, as that requires the transaction index being enabled in bitcoind or btcd (and does not work at all with Neutrino). We can only look up transactions the wallet thinks belong to it (account creation, deposit, withdrawal). If the account is involved in a batch, lnd doesn't think the batch transaction itself belongs to the wallet (as there are no non-script outputs in there). BUT we have a list of all batches we were involved in, including the full batch TX.

So I think we can attempt to calculate the fee like this:

In addition to doing it that way, I think we should also start storing an account diff for each modification to make this easier in the future.

ffranr commented 2 years ago

Thank you for your input @guggero !

I've started work on this bit:

For each modification TX, look up its previous outpoints, first try wallet transactions

And then I'll add on to that the batches lookup after sharing my work.

In addition to doing it that way, I think we should also start storing an account diff for each modification to make this easier in the future.

That ^ makes sense.

ffranr commented 2 years ago

Where batches are concerned, I need to look at the output of this command: https://github.com/lightninglabs/pool/blob/da88f020c51e57a56051b39e82c82f1aac60c2fa/cmd/pool/auction.go#L221

ffranr commented 2 years ago

The output of the pool client auction leases command looks like this:

user@lightninglabs:~/pool$ pool_alice auction leases
{
        "leases": [
                {
                        "channel_point": "f5d38a1df450eb419fafe803b4b3c2f43b29cada46fd4beee0754ec493f249f5:0",
                        "channel_amt_sat": 100000,
                        "channel_duration_blocks": 2016,
                        "channel_lease_expiry": 0,
                        "channel_node_key": "03c9d4c7d4f67f76e41379d2da893505827148b6fa5b483149872fe0a2bc4117fc",
                        "channel_node_tier": "TIER_0",
                        "premium_sat": 499,
                        "clearing_rate_price": 2480,
                        "order_fixed_rate": 496,
                        "execution_fee_sat": 101,
                        "chain_fee_sat": 8162,
                        "order_nonce": "817eec11fb4d0887bfe9c435fc0f75e93d9d3db89c512559e82dabb836a9ef0d",
                        "matched_order_nonce": "1dceec3c0e3b0ea7b4ca1e456bb0b851bfe3e413b392605a34e2a2399fcdd8ba",
                        "purchased": false,
                        "self_chan_balance": 0,
                        "sidecar_channel": false
                }
        ],
        "total_amt_earned_sat": 499,
        "total_amt_paid_sat": 8263
}

I notice that there are two fee fields chain_fee_sat and execution_fee_sat. Are both of these account modification fees?

guggero commented 2 years ago

No, the execution fees are what the market participant pays to the auctioneer (service fees). So only the chain fees should be counted IMO.

ffranr commented 2 years ago

Below you'll find an example lnd wallet transaction for a pool account withdraw action. In this example I attempted to withdraw 50000 sats. The preceding balance was 400k sats.

Every output in the transaction is flagged as "is_our_address": false. I think I need to calculate the sum of all "is_our_address": false outputs in the current and preceding transaction before I can calculate the fee.

{
            "tx_hash": "d558756ccb7c79f5694bf8018c95273865ed806accaad5e33b40a26ee8d6203a",
            "amount": "0",
            "num_confirmations": 0,
            "block_hash": "",
            "block_height": 0,
            "time_stamp": "1666275668",
            "total_fees": "0",
            "dest_addresses": [
                "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
                "bcrt1ptkstdfj04m89sl8u7a95pwffhkzqm0x8t4cnfqlxl3ygud0rzdvsan9nqs"
            ],
            "output_details": [
                {
                    "output_type": "SCRIPT_TYPE_PUBKEY_HASH",
                    "address": "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
                    "pk_script": "51205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059",
                    "output_index": "0",
                    "amount": "50000",
                    "is_our_address": false
                },
                {
                    "output_type": "SCRIPT_TYPE_PUBKEY_HASH",
                    "address": "bcrt1ptkstdfj04m89sl8u7a95pwffhkzqm0x8t4cnfqlxl3ygud0rzdvsan9nqs",
                    "pk_script": "51205da0b6a64faece587cfcf74b40b929bd840dbcc75d713483e6fc488e35e31359",
                    "output_index": "1",
                    "amount": "342505",
                    "is_our_address": false
                }
            ],
            "raw_tx_hex": "02000000000101f549f293c44e75e0ee4bfd46daca293bf4c2b3b403e8af9f41eb50f41d8ad3f50200000000000000000250c30000000000002251205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059e9390500000000002251205da0b6a64faece587cfcf74b40b929bd840dbcc75d713483e6fc488e35e3135901401fde2b3f75bb54befb8440e33d0fa87ac66080fb94d3ce141bff693f9db681a576f270dbb3100fe1bd3ac2169b7abc3fcb0581a0531ba929f6d384dd67b851cf00000000",
            "label": " poold -- AccountModification(acct_key=024d5df438109bd3133ee388dc4f302b1035d5dfd4e9cafcf9cdc983b651e1b515, expiry=false, deposit=false, is_close=false)",
            "previous_outpoints": [
                {
                    "outpoint": "f5d38a1df450eb419fafe803b4b3c2f43b29cada46fd4beee0754ec493f249f5:2",
                    "is_our_output": false
                }
            ]
        },
dstadulis commented 2 years ago

Can we keep track of the output we are tracking in order to observe the fee? If we can't track this, then the delta wouldn't be able to be calculate.

the transaction should have a label is there a label for renewal? checking the logs to see if everything is properly labeled

ffranr commented 2 years ago

I'm still stuck with a strange withdraw transaction. I would expect at least one is_our_address to be true. And I thought at least one output_type should be taproot.

Command:

pool_alice accounts withdraw --trader_key $TRADER_KEY --amt 50000 --sat_per_vbyte 5 --addr bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8

addr is set to a random address.

{
            "tx_hash": "755b00c399d3bf8eec0fe68156af9b90d443436bbb9f282078aeae855a2a0205",
            "amount": "0",
            "num_confirmations": 0,
            "block_hash": "",
            "block_height": 0,
            "time_stamp": "1666710175",
            "total_fees": "0",
            "dest_addresses": [
                "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
                "bcrt1pr9tnxkd5txungm4tl4vjcl0mad3mu8cskyw96axnzla4j0qrwevsq5t4sg"
            ],
            "output_details": [
                {
                    "output_type": "SCRIPT_TYPE_PUBKEY_HASH",
                    "address": "bcrt1p2agvu57xt3p6wut5xql59qpazhx2j02dswlprm2tv0ts2xp4xpvsed8wg8",
                    "pk_script": "51205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059",
                    "output_index": "0",
                    "amount": "50000",
                    "is_our_address": false
                },
                {
                    "output_type": "SCRIPT_TYPE_PUBKEY_HASH",
                    "address": "bcrt1pr9tnxkd5txungm4tl4vjcl0mad3mu8cskyw96axnzla4j0qrwevsq5t4sg",
                    "pk_script": "512019573359b459b9346eabfd592c7dfbeb63be1f10b11c5d74d317fb593c037659",
                    "output_index": "1",
                    "amount": "142300",
                    "is_our_address": false
                }
            ],
            "raw_tx_hex": "0200000000010158b13fd365bee3ce6673efaae79bafdd59f7dba31df42b1353503c44a1b672ec0000000000000000000250c30000000000002251205750ce53c65c43a77174303f42803d15cca93d4d83be11ed4b63d70518353059dc2b02000000000022512019573359b459b9346eabfd592c7dfbeb63be1f10b11c5d74d317fb593c037659014047a40ba6132b9ab6da159aa7fd7bb814ee9f58751d5e50db07d92f8b3a7929f853c49f8488d0518a8404bd62ec164b7f724e554fa9c030839e1d8a4e7570da0100000000",
            "label": " poold -- AccountModification(acct_key=038709293544c27d37c632f15bc5a2a2d0ab6d0fc46e6fbc28ec899015d26c3f9c, expiry=false, deposit=false, is_close=false)",
            "previous_outpoints": [
                {
                    "outpoint": "ec72b6a1443c5053132bf41da3dbf759ddaf9be7aaef7366cee3be65d33fb158:0",
                    "is_our_output": false
                }
            ]
        },

lnd/lncli versions seems to be correct:

user@lightninglabs:~/pool$ reg_alice version
{
    "lncli": {
        "commit": "v0.15.3-beta",
        "commit_hash": "b4e7131bdb47531ad2f00ce345ddcdb58935bba5",
        "version": "0.15.3-beta",
        "app_major": 0,
        "app_minor": 15,
        "app_patch": 3,
        "app_pre_release": "beta",
        "build_tags": [
            "autopilotrpc",
            "signrpc",
            "walletrpc",
            "chainrpc",
            "invoicesrpc",
            "watchtowerrpc",
            "neutrinorpc",
            "monitoring",
            "peersrpc",
            "kvdb_postgres",
            "kvdb_etcd"
        ],
        "go_version": "go1.18.2"
    },
    "lnd": {
        "commit": "v0.15.3-beta",
        "commit_hash": "b4e7131bdb47531ad2f00ce345ddcdb58935bba5",
        "version": "0.15.3-beta",
        "app_major": 0,
        "app_minor": 15,
        "app_patch": 3,
        "app_pre_release": "beta",
        "build_tags": [
            "autopilotrpc",
            "signrpc",
            "walletrpc",
            "chainrpc",
            "invoicesrpc",
            "watchtowerrpc",
            "neutrinorpc",
            "monitoring",
            "peersrpc",
            "kvdb_postgres",
            "kvdb_etcd"
        ],
        "go_version": "go1.18.2"
    }
}

And I'm using the latest commit (+ my changes) for pool and subasta.

Oli answered with the following via chat:

it's kind of expected that the wallet doesn't recognize the withdrawn account. and it also doesn't recognize the remaining account balance because the account output is a script output unknown to the wallet so basically is_our_address is always false for a Pool account output.

it's a script created outside of lnd (only Pool knows how to spend it) so I guess we can only recognize withdrawals if they actually go back to the wallet, then is_our_address will be true for the withdrawn amount at least

I think this basically means that if a user withdraws to an address that lnd does not recognize then all fees for subsequent account actions will be incalculable. Because if at any point we end up with both outputs where is_our_address is false, then we won't know the account value for the next action. Unless I can figure out which output corresponds to the account by some other means.

jamaljsr commented 2 years ago

i was thinking about this issue and had an idea but feels like a completely different approach. Instead of retroactively trying to parse account actions from the transactions, what if we just added a new history field to the accounts which is an array of account modifications going forward. It could store the action (create, withdraw, deposit, renew, close), txid, timestamp, chain_fee, and balance. Obviously this wouldn't provide historical data, but I've already found a workaround for that for the time being. Just figured I'd throw this out there to see if it help at all with implementing a better solution.

ffranr commented 1 year ago

Added RPC to solve this issue: https://github.com/lightninglabs/pool/pull/395