Skip to content

Add closedchannels RPC#2642

Merged
t-bast merged 10 commits into
ACINQ:masterfrom
rorp:closedchannels_rpc
Jun 2, 2023
Merged

Add closedchannels RPC#2642
t-bast merged 10 commits into
ACINQ:masterfrom
rorp:closedchannels_rpc

Conversation

@rorp
Copy link
Copy Markdown
Contributor

@rorp rorp commented Apr 17, 2023

This PR allows to access the historic channel data without relying on third party services like LN explorers.

@t-bast
Copy link
Copy Markdown
Member

t-bast commented May 2, 2023

Can you explain why you think this is useful?

Please note that the API is strictly for managing your node and we don't want to end up maintaining too many unused APIs. Exotic use-cases / analysis should run directly on the DB (and ideally on the read-only replicated DB to avoid impacting the running node).

@rorp
Copy link
Copy Markdown
Contributor Author

rorp commented May 2, 2023

Can you explain why you think this is useful?

First of all, it's going to make me as famous as Rusty Russell is (well, sorta :)) via Bitcoin Optech Newsletter: https://bitcoinops.org/en/newsletters/2023/04/05/#core-lightning-5967

Second, in addition to providing more control over the node to the users, it helps to retain the users' privacy.

Eg.: every time I receive a "channel closed" notification on my Nostr alarm bot, the closed channel disappears from the list that returns the channels API call, and the only way to find out which peer closed the channel, what was the cause, what was the balance, etc is looking at the channel with a Lightning explorer like 1ML or Amboss, leaking my private data in the process. You cannot see the exact balance though. However, I have all the data I need right here at my fingertips, but I have no way to read them.

Please note that the API is strictly for managing your node and we don't want to end up maintaining too many unused APIs.
Exotic use-cases / analysis should run directly on the DB (and ideally on the read-only replicated DB to avoid impacting the running node).

What is so exotic about knowing what happened with my own money? It's an essential part of managing my node. As I'm a node op, I know that very well.

The code and the data already exist. Just a few lines of code are needed to put them together.

Also, it's not the first time you suggest to access the database directly. There are 2 problems though:

  1. I use Sqlite backend. Eclair locks eclair.sqlite exclusively, other processes cannot access it. No luck here...
  2. The channel database contains raw data blobs. Their structure is undocumented and will change without notice (as it has happened before). And this is the REAL problem.

Although JSON format isn't documented either, one can use their natural (or artificial for some) intelligence to figure out the structure.

I use Python for my small automation scripts. How should I write a script that tells if the channel force closed using this data?

sqlite> select * from local_channels where is_closed=1 limit 1;
K? ????5?R??z?	?1 ?N?] ????||1|1681668108044||||1681668156943

With this PR I can do

channel = json.loads('{ closed channel JSON }')
if not channel['data']['mutualClosePublished']:
    print("force closed")

@t-bast
Copy link
Copy Markdown
Member

t-bast commented May 3, 2023

Fair enough! The list of closed channels is an always-growing list that will eventually be huge, so I think we shouldn't allow listing everything at once, this is too footgunny. What about adding pagination and making the count parameter mandatory?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 3, 2023

Codecov Report

Merging #2642 (f82a2c5) into master (e7b4631) will increase coverage by 0.00%.
The diff coverage is 70.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master    #2642   +/-   ##
=======================================
  Coverage   85.88%   85.88%           
=======================================
  Files         214      214           
  Lines       17591    17613   +22     
  Branches      728      738   +10     
=======================================
+ Hits        15108    15127   +19     
- Misses       2483     2486    +3     
Impacted Files Coverage Δ
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 55.76% <0.00%> (-0.79%) ⬇️
.../main/scala/fr/acinq/eclair/db/DualDatabases.scala 10.94% <0.00%> (-0.17%) ⬇️
...a/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala 97.89% <88.88%> (-0.95%) ⬇️
...ain/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala 99.28% <100.00%> (+0.03%) ⬆️

... and 6 files with indirect coverage changes

Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala Outdated
Copy link
Copy Markdown
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's forget about the performance for nodes with too much historical data, it's probably fine for now. We can optimize that later by moving closed channels to their own table.

Needs a rebase!

Comment thread eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala
Copy link
Copy Markdown
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting close, a few comments on the changes in the DB files, but otherwise looks good to me.

Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteChannelsDb.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/db/pg/PgChannelsDb.scala Outdated
Copy link
Copy Markdown
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your patience 🙏

@t-bast t-bast merged commit 37eb142 into ACINQ:master Jun 2, 2023
@rorp rorp deleted the closedchannels_rpc branch June 2, 2023 19:55
@rorp
Copy link
Copy Markdown
Contributor Author

rorp commented Jun 7, 2023

We did it! https://bitcoinops.org/en/newsletters/2023/06/07/#eclair-2642

@t-bast
Copy link
Copy Markdown
Member

t-bast commented Jun 8, 2023

Congrats 😄

t-bast added a commit that referenced this pull request Jun 16, 2023
This release introduces a few API changes:

- `audit` now accepts `--count` and `--skip` parameters to limit the
  number of retrieved items (#2474, #2487)
- `sendtoroute` removes the `--trampolineNodes` argument and
  implicitly uses a single trampoline hop (#2480)
- `sendtoroute` now accept `--maxFeeMsat` to specify an upper bound
  of fees (#2626)
- `payinvoice` always returns the payment result when used with
  `--blocking`, even when using MPP (#2525)
- `node` returns high-level information about a remote node (#2568)
- `channel-created` is a new websocket event that is published when
  a channel's funding transaction has been broadcast (#2567)
- `channel-opened` websocket event was updated to contain the final
  `channel_id` and be published when a channel is ready to process
  payments (#2567)
- `getsentinfo` can now be used with `--offer` to list payments sent
  to a specific offer
- `listreceivedpayments` lists payments received by your node (#2607)
- `closedchannels` lists closed channels. It accepts `--count` and
  `--skip` parameters to limit the number of retrieved items as well
  (#2642)
- `cpfpbumpfees` can be used to unblock chains of unconfirmed
  transactions by creating a child transaction that pays a high fee
  (#1783)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants