Add example channel interceptor plugin #4

remyers · 2023-01-04T14:14:55Z

This PR creates an example of a plugin that intercepts channel open messages sent to a node and continues the process of accepting a valid request, or returns a failure message to the sender.

This example plugin reads a custom configuration file with parameters min-active-channels and min-total-capacity and rejects OpenChannel requests from remote peers if they have less than the minimum public active channels or public total capacity.

Depends on Eclair PR# 2552.

- Use inherited unsealed `InterceptOpenChannelCommand` trait from eclair instead of sealed plugin specific `Command`

- this decouples it better from ChannelData.LocalParams and makes it clear what parameters can be altered

t-bast

It's going to be a bit painful, but I'd like to rename that plugin...it is currently a sample plugin that shouldn't be used, but I don't think this is very useful to have. We know we'll want a channel interceptor plugin for dual-funding, that decides how much funds (if any) the non-initiator should put into the channel. Since eclair only allows one plugin that intercepts channel requests, that plugin should implement optional additional restrictions (what you currently do in this PR) and liquidity allocation logic (what I'll add later when doing liquidity ads).

So it would make sense to name it to something that reflects that right now, this way I can simply extend it later when working on liquidity ads. Node operators would likely fork that plugin to tweak the liquidity allocation logic (and add new restrictions if needed).

I think that the funding aspect will be more important to users than the additional restrictions (especially since we moved most of the restrictions directly inside eclair), so I believe naming the plugin channel-funding makes more sense? WDYT?

t-bast · 2023-02-17T08:52:01Z

channel-interceptor/README.md

+
+Disclaimer: this plugin is for demonstration purposes only.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


Why the license here? The whole repository already has an Apache-2 license?

Not sure why I did that; fixed in 2ac483e.

t-bast · 2023-02-17T08:58:46Z

channel-interceptor/README.md

+## Commands
+
+There are no cli commands for this plugin.


This is unnecessary, if there are no commands, we don't even need to mention it?

Fixed in 2ac483e

t-bast · 2023-02-17T09:02:52Z

...src/main/scala/fr/acinq/eclair/plugins/channelinterceptor/OpenChannelInterceptorPlugin.scala

+    // load or generate seed
+    config = loadConfiguration(setup.datadir)


nit: wrong copy-paste for the comment. Also, why do you load the config early since you only use it once inside onKit? If you loaded it inside onKit you wouldn't need to have a var config: Config = _.

This is necessary because of the two-step initialization of plugins. We can only access setup.datadir in OnSetup which we need to load config. We can either have a var config or load the parameters from config in OnSetup and create a var to hold them until OnKit is called.

Fixed the nit in 2ac483e

t-bast · 2023-02-17T09:05:30Z

...test/scala/fr/acinq/eclair/plugins/channelinterceptor/OpenChannelInterceptorPluginSpec.scala

+    assert(minActiveChannels >= 0)
+    assert(minTotalCapacity >= Satoshi(0))


You should assert on exact values here, it would make more sense.

Fixed in ee57ca2

t-bast · 2023-02-17T09:11:28Z

...eptor/src/main/scala/fr/acinq/eclair/plugins/channelinterceptor/OpenChannelInterceptor.scala

+  private sealed trait ErrorResponse
+  private object LessThanMinActiveChannelsError extends ErrorResponse {
+    override def toString: String = s"rejected, less than $minActiveChannels active channels"
+  }
+  private object LessThanMinTotalCapacityError extends ErrorResponse {
+    override def toString: String = s"rejected, less than $minTotalCapacity total capacity"
+  }
+  private object NoPublicChannelsError extends ErrorResponse {
+    override def toString: String = s"rejected, no public channels"
+  }


There errors are used only once and only for their string value, we can remove them and inline the error message?

Fixed in fc2f404

t-bast · 2023-02-17T09:13:22Z

...eptor/src/main/scala/fr/acinq/eclair/plugins/channelinterceptor/OpenChannelInterceptor.scala

+      case WrappedGetNodeResponse(o, PublicNode(_, _, totalCapacity)) if totalCapacity < minTotalCapacity =>
+        rejectOpenChannel(o, LessThanMinTotalCapacityError)
+        Behaviors.same
+      case WrappedGetNodeResponse(o, UnknownNode(_)) =>


This should be configurable: otherwise when using this plugin we'd have no way of accepting channels from private nodes. There should be a boolean in the config, something like allow-private-nodes = true

Fixed in 798a849

t-bast · 2023-02-17T09:15:38Z

...r/src/test/scala/fr/acinq/eclair/plugins/channelinterceptor/OpenChannelInterceptorSpec.scala

+  val publicKey: Crypto.PublicKey = PrivateKey(ByteVector32.One).publicKey
+  val fundingAmount: Satoshi = 1 sat
+  val remoteNodeId: Crypto.PublicKey = PrivateKey(ByteVector32.One).publicKey
+  val temporaryChannelId: ByteVector32 = ByteVector32.Zeroes
+  val openChannel: OpenChannel = OpenChannel(ByteVector32.Zeroes, temporaryChannelId, fundingAmount, 0 msat, 1 sat, UInt64(1), 1 sat, 1 msat, FeeratePerKw(1 sat), CltvExpiryDelta(1), 1, publicKey, publicKey, publicKey, publicKey, publicKey, publicKey, ChannelFlags.Private)
+  val openDualChannel: OpenDualFundedChannel = OpenDualFundedChannel(ByteVector32.Zeroes, ByteVector32.One, FeeratePerKw(5000 sat), FeeratePerKw(4000 sat), 250_000 sat, 500 sat, UInt64(50_000), 15 msat, CltvExpiryDelta(144), 483, 650_000, publicKey(1), publicKey(2), publicKey(3), publicKey(4), publicKey(5), publicKey(6), publicKey(7), ChannelFlags(true))
+  val defaultParams: DefaultParams = DefaultParams(100 sat, 100000 msat, 100 msat, CltvExpiryDelta(288), 10)
+  val channels: Map[Peer.FinalChannelId, actor.ActorRef] = Map(Peer.FinalChannelId(randomBytes32()) -> system.classicSystem.deadLetters)
+  val connectedData: Peer.ConnectedData = Peer.ConnectedData(NodeAddress.fromParts("1.2.3.4", 42000).get, system.classicSystem.deadLetters, Init(TestConstants.Alice.nodeParams.features.initFeatures()), Init(TestConstants.Bob.nodeParams.features.initFeatures()), channels.map { case (k: ChannelId, v) => (k, v) })
+  val openChannelNonInitiator: OpenChannelNonInitiator = OpenChannelNonInitiator(remoteNodeId, Left(openChannel), connectedData.localFeatures, connectedData.remoteFeatures, connectedData.peerConnection.toTyped)
+  val bobAnnouncement: NodeAnnouncement = announcement(TestConstants.Bob.nodeParams.nodeId)
+
+  def publicKey(fill: Byte): PublicKey = PrivateKey(ByteVector.fill(32)(fill)).publicKey


Like what we did in the eclair PR, this can be greatly simplified, some of these values are unused or used only once, it deserves a clean-up to be less confusing and focus only on the test itself.

Indeed, cleaned up in dc34b5a.

remyers · 2023-02-23T09:05:29Z

I think that the funding aspect will be more important to users than the additional restrictions (especially since we moved most of the restrictions directly inside eclair), so I believe naming the plugin channel-funding makes more sense? WDYT?

It makes sense to create a potentially useful plugin rather than a generic example. It's good to also demonstrate other plugin features like using the config file to select which inbound peers though too so people can rift on that for their own plugins.

- also update comments to expand functionality by determining amount of funding to contribute to dual-funding requests

remyers added 2 commits January 4, 2023 13:32

Add channel interceptor plugin

1ffd76b

Update to use a defined API via an actor that monitors for timeout

7da270d

remyers force-pushed the channel-interceptor-plugin branch from 4865b5a to 7da270d Compare January 9, 2023 14:54

remyers added 7 commits January 10, 2023 10:27

Remove unnecessary fundingAmount_opt from AcceptOpenChannel

bca27a8

Update for Eclair change making fundingAmount a property

3e81e2f

Refactor to respond based on routing graph information

552ede2

Simplify command inheritance

f46a55d

- Use inherited unsealed `InterceptOpenChannelCommand` trait from eclair instead of sealed plugin specific `Command`

Add configuration file for channel interceptor parameters

56af843

Update to pass DefaultParams instead of LocalParams

ca881e6

- this decouples it better from ChannelData.LocalParams and makes it clear what parameters can be altered

Refactor to abstract error response strings

4591e98

remyers marked this pull request as ready for review January 27, 2023 12:22

remyers added 2 commits January 27, 2023 13:35

Update README.md with current behavior

077af37

Update after addressing review comments on main Eclair PR# 2552

bdc58d7

remyers requested a review from t-bast February 8, 2023 14:51

remyers mentioned this pull request Feb 8, 2023

Add support for plugins that intercept open channel messages ACINQ/eclair#2552

Merged

t-bast reviewed Feb 17, 2023

View reviewed changes

remyers added 6 commits February 23, 2023 10:43

Rename plugin to channel-funding

900bf8f

- also update comments to expand functionality by determining amount of funding to contribute to dual-funding requests

Remove redundant disclaimer and unnecessary cli section

2ac483e

Fix assert to test default values

ee57ca2

Inline error messages used only once

fc2f404

Add configuration option allow-private-nodes

798a849

Cleanup to simplify test by removing unused values

dc34b5a

remyers mentioned this pull request May 3, 2023

Pass remote address to InterceptOpenChannelPlugin ACINQ/eclair#2641

Merged

fixup! Rename plugin to channel-funding

d5e74cc

DerEwige mentioned this pull request Aug 4, 2023

Release not refering to non existing plugin ACINQ/eclair#2713

Closed

t-bast mentioned this pull request Feb 20, 2024

Add channel funding plugin #11

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add example channel interceptor plugin #4

Add example channel interceptor plugin #4

remyers commented Jan 4, 2023 •

edited

Loading

t-bast left a comment

t-bast Feb 17, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

t-bast Feb 17, 2023

remyers Feb 23, 2023

remyers commented Feb 23, 2023


		Disclaimer: this plugin is for demonstration purposes only.

		THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

		// load or generate seed
		config = loadConfiguration(setup.datadir)

		assert(minActiveChannels >= 0)
		assert(minTotalCapacity >= Satoshi(0))

Add example channel interceptor plugin #4

Are you sure you want to change the base?

Add example channel interceptor plugin #4

Conversation

remyers commented Jan 4, 2023 • edited Loading

t-bast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyers commented Feb 23, 2023

remyers commented Jan 4, 2023 •

edited

Loading