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

Liquidity Ads #2550

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Liquidity Ads #2550

wants to merge 1 commit into from

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Jan 4, 2023

The initiator of open_channel2, tx_init_rbf and splice_init can request funding from the remote node. The non-initiator node will:

  • let the open-channel-interceptor plugin decide whether to lease liquidity for new channels or not, and how much
  • always honor liquidity requests on existing channels (RBF and splice)

We currently don't modify commitment transactions to enforce the lease. This is different from lightning/bolts#878 and instead matches lightning/bolts#1145.

We currently use the temporary tlv tag 1337 while we're waiting for feedback on our spec proposal.

Liquidity ads are included in the node_announcement message, which lets buyers compare sellers and connect to sellers that provide rates they are comfortable with.

We store every liquidity purchase (whether we're buyer or seller) in the audit DB. This is important information when choosing which peers are worth keeping channels with.

@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2023

Codecov Report

Attention: Patch coverage is 93.85475% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 86.07%. Comparing base (c8184b3) to head (74f2c0d).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2550      +/-   ##
==========================================
+ Coverage   85.96%   86.07%   +0.11%     
==========================================
  Files         219      220       +1     
  Lines       18441    18709     +268     
  Branches      762      809      +47     
==========================================
+ Hits        15853    16104     +251     
- Misses       2588     2605      +17     
Files Coverage Δ
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 56.61% <100.00%> (+0.18%) ⬆️
.../src/main/scala/fr/acinq/eclair/PluginParams.scala 66.66% <ø> (ø)
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 100.00% <ø> (ø)
.../scala/fr/acinq/eclair/channel/ChannelEvents.scala 100.00% <100.00%> (ø)
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 94.56% <100.00%> (+0.02%) ⬆️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 85.47% <100.00%> (+0.27%) ⬆️
...inq/eclair/channel/fund/InteractiveTxBuilder.scala 91.80% <100.00%> (+0.42%) ⬆️
...cinq/eclair/channel/fund/InteractiveTxFunder.scala 92.80% <100.00%> (+0.32%) ⬆️
...c/main/scala/fr/acinq/eclair/db/pg/PgAuditDb.scala 99.73% <100.00%> (+0.04%) ⬆️
...cala/fr/acinq/eclair/db/sqlite/SqliteAuditDb.scala 99.72% <100.00%> (+0.04%) ⬆️
... and 16 more

... and 1 file with indirect coverage changes

@t-bast t-bast force-pushed the liquidity-ads branch 3 times, most recently from 709ce50 to b077e3e Compare November 13, 2023 15:02
@t-bast t-bast force-pushed the liquidity-ads branch 4 times, most recently from 7344ecd to 2ab7242 Compare November 28, 2023 14:05
@t-bast t-bast force-pushed the liquidity-ads branch 2 times, most recently from 0030428 to 5cf3b6d Compare December 6, 2023 10:14
@pm47
Copy link
Member

pm47 commented Dec 12, 2023

FYI, here is a patch on channel/Monitoring.scala to track liquidity splices:

diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Monitoring.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Monitoring.scala
index fa15594cc0..4900a46467 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Monitoring.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Monitoring.scala
@@ -57,10 +57,18 @@ object Monitoring {
      */
     def recordSplice(fundingParams: InteractiveTxParams, sharedTx: SharedTransaction): Unit = {
       if (fundingParams.localContribution > 0.sat) {
-        Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Local).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceIn).record(fundingParams.localContribution.toLong)
+        if (fundingParams.isInitiator) {
+          Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Local).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceIn).record(fundingParams.localContribution.toLong)
+        } else {
+          Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Local).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceLiquidity).record(fundingParams.localContribution.toLong)
+        }
       }
       if (fundingParams.remoteContribution > 0.sat) {
-        Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Remote).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceIn).record(fundingParams.remoteContribution.toLong)
+        if (fundingParams.isInitiator) {
+          Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Remote).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceLiquidity).record(fundingParams.remoteContribution.toLong)
+        } else {
+          Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Remote).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceIn).record(fundingParams.remoteContribution.toLong)
+        }
       }
       if (sharedTx.localOnlyNonChangeOutputs.nonEmpty) {
         Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Local).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceOut).record(sharedTx.localOnlyNonChangeOutputs.map(_.amount).sum.toLong)
@@ -68,10 +76,10 @@ object Monitoring {
       if (sharedTx.remoteOutputs.nonEmpty) {
         Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Remote).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceOut).record(sharedTx.remoteOutputs.map(_.amount).sum.toLong)
       }
-      if (fundingParams.localContribution < 0.sat && sharedTx.localOutputs.isEmpty) {
+      if (fundingParams.localContribution < 0.sat && sharedTx.localOutputs.isEmpty && fundingParams.remoteContribution == 0.sat) {
         Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Local).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceCpfp).record(Math.abs(fundingParams.localContribution.toLong))
       }
-      if (fundingParams.remoteContribution < 0.sat && sharedTx.remoteOutputs.isEmpty) {
+      if (fundingParams.remoteContribution < 0.sat && sharedTx.remoteOutputs.isEmpty && fundingParams.localContribution == 0.sat) {
         Metrics.Splices.withTag(Tags.Origin, Tags.Origins.Remote).withTag(Tags.SpliceType, Tags.SpliceTypes.SpliceCpfp).record(Math.abs(fundingParams.remoteContribution.toLong))
       }
     }
@@ -114,6 +122,7 @@ object Monitoring {
       val SpliceIn = "splice-in"
       val SpliceOut = "splice-out"
       val SpliceCpfp = "splice-cpfp"
+      val SpliceLiquidity = "splice-liquidity"
     }
   }

A "purer" way would have been to add a isInitiator tag, instead of a SpliceLiquidity type, but it's also less user friendly. Here we are arbitrarily categorizing splices anyway. Will see how it goes on feature branches.

pm47 pushed a commit to ACINQ/lightning-kmp that referenced this pull request Dec 13, 2023
Implement a prototype for liquidity ads, compatible with ACINQ/eclair#2550
Note that we only implement the buyer side, which limits testing.
The specification is available here: lightning/bolts#878

We currently don't add CLTV locks to the commitment transactions, for simplicity's sake.
@pm47
Copy link
Member

pm47 commented Mar 28, 2024

Please rebase 🙏

The initiator of `open_channel2`, `tx_init_rbf` and `splice_init` can
request funding from the remote node. The non-initiator node will:

- let the open-channel-interceptor plugin decide whether to lease
  liquidity for new channels or not, and how much
- always honor liquidity requests on existing channels (RBF and splice)

We currently don't modify commitment transactions to enforce the lease.
This is different from lightning/bolts#878 and
instead matches lightning/bolts#1145.

We currently use the temporary tlv tag 1337 while we're waiting for
feedback on our spec proposal.

Liquidity ads are included in the `node_announcement` message, which
lets buyers compare sellers and connect to sellers that provide rates
they are comfortable with.

We store every liquidity purchase (whether we're buyer or seller) in the
audit DB. This is important information when choosing which peers are
worth keeping channels with.
@t-bast
Copy link
Member Author

t-bast commented Mar 29, 2024

Rebased and squashed! I'll work on porting the annoying-to-rebase parts to master to simplify future rebases.

@@ -114,6 +115,11 @@ class PgAuditDb(implicit ds: DataSource) extends AuditDb with Logging {
statement.executeUpdate("CREATE INDEX transactions_published_channel_id_idx ON audit.transactions_published(channel_id)")
}

def migration1213(statement: Statement): Unit = {
statement.executeUpdate("CREATE TABLE IF NOT EXISTS audit.liquidity_purchases (tx_id TEXT NOT NULL, channel_id TEXT NOT NULL, node_id TEXT NOT NULL, is_buyer BOOLEAN NOT NULL, amount_sat BIGINT NOT NULL, mining_fee_sat BIGINT NOT NULL, service_fee_sat BIGINT NOT NULL, funding_tx_index BIGINT NOT NULL, capacity_sat BIGINT NOT NULL, local_contribution_sat BIGINT NOT NULL, remote_contribution_sat BIGINT NOT NULL, local_balance_msat BIGINT NOT NULL, remote_balance_msat BIGINT NOT NULL, outgoing_htlc_count BIGINT NOT NULL, incoming_htlc_count BIGINT NOT NULL, seller_sig TEXT NOT NULL, witness TEXT NOT NULL, created_at TIMESTAMP WITH TIME ZONE NOT NULL, confirmed_at TIMESTAMP WITH TIME ZONE)")
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self: we should remove the IF NOT EXISTS added in a few places, they're just an artifact from private branches.

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.

None yet

3 participants