pool icon indicating copy to clipboard operation
pool copied to clipboard

Expose historical account modification fees via RPC

Open jamaljsr opened this issue 1 year ago • 4 comments

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

jamaljsr avatar Aug 24 '22 07:08 jamaljsr

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
        }
      ]
    }

ffranr avatar Oct 12 '22 15:10 ffranr

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:

  • Look at each account modification TX in the wallet
  • For each modification TX, look up its previous outpoints, first try wallet transactions, if that fails, try batches. If TX not found, we cannot calculate the fee.

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.

guggero avatar Oct 13 '22 11:10 guggero

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 avatar Oct 13 '22 12:10 ffranr

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 avatar Oct 13 '22 15:10 ffranr

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?

ffranr avatar Oct 20 '22 12:10 ffranr

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

guggero avatar Oct 20 '22 12:10 guggero

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
                }
            ]
        },

ffranr avatar Oct 20 '22 14:10 ffranr

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

dstadulis avatar Oct 20 '22 16:10 dstadulis

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.

ffranr avatar Oct 25 '22 17:10 ffranr

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.

jamaljsr avatar Oct 27 '22 13:10 jamaljsr