add a `funder` plugin #4489

niftynei · 2021-04-22T23:05:41Z

Plugin to democratize a peer's ability to contribute funds to incoming openchannel request.

Current knobs include:

three kinds of policy: match (% of their funding), available (% of your available funds), and fixed (sat value)
minimum and maximum funding from peer
minimum and maximum that you'll put in a channel
fuzzing (defaults to 5%)
fund_probability. only funds X% of incoming requests
reserve-tank. never* spends available funds beneath this amount

*assuming races never happen

Future work: we don't take feerates into account right now we calculating the total available funds to add.

When an RPC originates from a plugin, and there's no associated command, it's useful to be able to signal that you're finished.

rustyrussell · 2021-04-22T23:29:12Z

openingd/dualopend.c

+		/* Set the channel_id back to what they originally sent us,
+		 * in open_channel2, since they won't know the combined
+		 * channel_id yet for errors */
+		state->channel_id = tmp_chan_id;


This seems like we should only be updating the channel_id field when we send the reply?

rustyrussell · 2021-04-22T23:35:12Z

lightningd/dual_open_control.c

@@ -244,6 +244,10 @@ struct openchannel2_payload {
 	/* What's the maximum amount of funding
 	 * this channel can hold */
 	struct amount_sat channel_max;
+	/* Snapshot of our current utxo set, available */


Not sure this is actually that useful: it's more accurate for them to count utxos themselves and take into account the feerate. It's a little more work, but I think the plug-in should listutxos itself...

rustyrussell · 2021-04-23T02:38:54Z

Here's proposed patch for the tmp_channel_id issue: spec should probably be updated too?

dualopend: don't use final channel_id for accepter_start2

The other side doesn't know it until *after* it parses this msg.  We
add a quick hack to still allow old nodes to work (for now!).

This also fixes a bug (spotted by @niftynei) where any errors we sent
before accepter_start2 would have the new (unknowable!) channel_id
rather than the temp one.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/openingd/dualopend.c b/openingd/dualopend.c
index ad8d5420e..d753dff8e 100644
--- a/openingd/dualopend.c
+++ b/openingd/dualopend.c
@@ -1897,7 +1897,6 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
 	struct bitcoin_blkid chain_hash;
 	struct tlv_opening_tlvs *open_tlv;
 	char *err_reason;
-	struct channel_id tmp_chan_id;
 	u8 *msg;
 	struct amount_sat total;
 	enum dualopend_wire msg_type;
@@ -1939,7 +1938,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
 	 * `open_channel2`), a temporary `channel_id` should be found
 	 * by using a zeroed out basepoint for the unknown peer.
 	 */
-	derive_tmp_channel_id(&tmp_chan_id,
+	derive_tmp_channel_id(&state->channel_id,
 			      &state->their_points.revocation);
 	if (!channel_id_eq(&state->channel_id, &tmp_chan_id))
 		negotiation_failed(state, "open_channel2 channel_id incorrect."
@@ -1949,12 +1948,6 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
 				   type_to_string(tmpctx, struct channel_id,
 						  &state->channel_id));
 
-	/* Everything's ok. Let's figure out the actual channel_id now */
-	derive_channel_id_v2(&state->channel_id,
-			     &state->our_points.revocation,
-			     &state->their_points.revocation);
-
-
 	/* Save feerate on the state as well */
 	state->feerate_per_kw_funding = tx_state->feerate_per_kw_funding;
 
@@ -2105,6 +2098,11 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
 				     &state->first_per_commitment_point[LOCAL],
 				     a_tlv);
 
+	/* Everything's ok. Let's figure out the actual channel_id now */
+	derive_channel_id_v2(&state->channel_id,
+			     &state->our_points.revocation,
+			     &state->their_points.revocation);
+
 	sync_crypto_write(state->pps, msg);
 	peer_billboard(false, "channel open: accept sent, waiting for reply");
 
@@ -2479,6 +2477,23 @@ static void opener_start(struct state *state, u8 *msg)
 		open_err_fatal(state,  "Parsing accept_channel2 %s",
 			       tal_hex(msg, msg));
 
+	if (!channel_id_eq(&cid, &state->channel_id)) {
+		struct channel_id future_chan_id;
+		/* FIXME: v0.10.0 actually replied with the complete channel id here,
+		 * so we need to accept it for now */
+		derive_channel_id_v2(&future_chan_id,
+				     &state->our_points.revocation,
+				     &state->their_points.revocation);
+		if (!channel_id_eq(&cid, &future_chan_id)) {
+			peer_failed_err(state->pps, &cid,
+					"accept_channel2 ids don't match: "
+					"expected %s, got %s",
+					type_to_string(msg, struct channel_id,
+						       &state->channel_id),
+					type_to_string(msg, struct channel_id, &cid));
+		}
+	}
+
 	if (a_tlv->option_upfront_shutdown_script) {
 		state->upfront_shutdown_script[REMOTE]
 			= tal_steal(state,
@@ -2492,14 +2507,6 @@ static void opener_start(struct state *state, u8 *msg)
 			     &state->our_points.revocation,
 			     &state->their_points.revocation);
 
-	if (!channel_id_eq(&cid, &state->channel_id))
-		peer_failed_err(state->pps, &cid,
-				"accept_channel2 ids don't match: "
-				"expected %s, got %s",
-				type_to_string(msg, struct channel_id,
-					       &state->channel_id),
-				type_to_string(msg, struct channel_id, &cid));
-
 	/* Check that total funding doesn't overflow */
 	if (!amount_sat_add(&total, tx_state->opener_funding,
 			    tx_state->accepter_funding))

These are now included in all builds

We removed/changed the fields on the openchannel2 hook but never updated this test (which doesn't run with the CI for #reasons)

@niftynei

The other side doesn't know it until *after* it parses this msg. We add a quick hack to still allow old nodes to work (for now!). This also fixes a bug (spotted by @niftynei) where any errors we sent before accepter_start2 would have the new (unknowable!) channel_id rather than the temp one. Authored-by: Rusty Russell <rusty@rustcorp.com.au>

niftynei · 2021-04-26T22:40:20Z

Ok, patches have been applied (and already folded in mea culpa).

rustyrussell

Minor fixes only...

rustyrussell · 2021-04-28T03:04:27Z

plugins/libplugin.h

+/* Signals that we've completed a command. Useful for when
+ * there's no `cmd` present */
+struct command_result *command_done(void);
+


Drop this commit. The one place you end up using it, you do indeed have a command. You just don't use it!

I don't want to actually do anything with the command though. There's no response to send on our side when this completes, it's a 'silent no-op'?

OK, this is because the "cmd" is actually a notification, which doesn't want a response. It does, however, want to be freed!

I'll submit a followup PR which cleans this up everywhere...

rustyrussell · 2021-04-28T03:07:16Z

plugins/funder.c

+				   err, json_tok_full_len(result),
+				   json_tok_full(buf, result));
+
+		/* we skip resrved funds */


Need to buy a vwl?

rustyrussell · 2021-04-28T03:12:26Z

plugins/funder_policy.c

@@ -89,6 +89,32 @@ default_funder_policy(enum funder_opt policy,
 				 100);
 }

+char *funder_check_policy(struct funder_policy *policy)


const struct funder_policy?

rustyrussell · 2021-04-28T03:14:00Z

plugins/funder.c

+	err = funder_check_policy(&current_policy);
+	if (err)
+		return command_done_err(cmd, JSONRPC2_INVALID_PARAMS,
+					err, NULL);


I think you need to modify a copy, then assign it back to current_policy at the end.

Otherwise, setting a bad value causes an error, but sticks around...

rustyrussell · 2021-04-28T03:14:29Z

plugins/funder.c

+	return command_finished(cmd, res);
+}
+
+const struct plugin_command commands[] = { {


static const struct plugin_command commands?

rustyrussell · 2021-04-28T03:16:02Z

doc/PLUGINS.md

@@ -1157,7 +1157,8 @@ requests an RBF for a channel funding transaction.
    "funding_feerate_per_kw": 7500,
    "feerate_our_max": 10000,
    "feerate_our_min": 253,
-    "locktime": 2453,
+    "channel_max": "200000msat",


It will never be this value though, so it's misleading.

rustyrussell · 2021-04-28T03:17:02Z

lightningd/dual_open_control.c

@@ -206,6 +209,8 @@ rbf_channel_hook_serialize(struct rbf_channel_payload *payload,
 		     payload->feerate_our_min);
 	json_add_num(stream, "funding_feerate_per_kw",
 		     payload->funding_feerate_per_kw);
+	json_add_amount_sat_only(stream, "channel_max",
+				 payload->channel_max);


Since this converts to msat, and that fails for UINT64_MAX, this channel_max will actually only appear for non-wumbo channels. Which is probably OK, but should be documented.

Field should also be called "channel_max_msat".

ACK. also updated the channel_max field that exist on openchannel2 hook.

Changelog-Added: Plugins: add a `channel_max_msat` value to the `openchannel2` hook. Tells you the total max funding this channel is allowed to have.

For dual-funding's accepter plugin, we only want to send psbts that need to be signed to `signpsbt`; this lets us quickly check if they're "signable"

You gotta send over an amount if you send a psbt!

This is set by the peer and is non-negotiable. We're not even going to check if you got it right. You were told about it via `openchannel2`. It is what it is.

Oops

For scaling/multiplying sats

Behold! An immaculately concepted plugin for configuring your node to do amazing things* *fund channel open requests Changelog-Added: Plugins: Add `funder` plugin, which allows you to setup a policy for funding v2 channel open requests. Requres --experimental-dual-fund option

@rustyrussell

Compute available_funds locally, instead of getting it from the openchannel2 hook payload. Suggested-By: Rusty Russell @rustyrussell

If you're using the regtest node, turn on dual funding and automatically attempt to dual fund at a 100% match for every channel open that you do.

Error out if we've got the wrong info

Changelog-Added: Plugins: `funder` plugin now has new command `funderupdate` which will show current funding configuration and allow you to modify them

Changelog-Added: Plugins: `rbf_channel` hook has `channel_max_msat` parameter

Fund an RBF same as you would an open. We dont' do anything fancy with our inputs -- we dont' have a copy of the last PSBT that was sent.

Dual-funded channels won't match the old amount check. Good news is we're already returning the outnum so we just use that.

niftynei · 2021-04-29T21:42:10Z

Updated, as per comments.

rustyrussell · 2021-05-03T01:36:05Z

Ack de9102a

libplugin: add no-op command complete function

5065f41

When an RPC originates from a plugin, and there's no associated command, it's useful to be able to signal that you're finished.

niftynei added the funding label Apr 22, 2021

niftynei added this to the v0.10.1 milestone Apr 22, 2021

niftynei requested a review from ZmnSCPxj April 22, 2021 23:05

niftynei requested a review from cdecker as a code owner April 22, 2021 23:05

rustyrussell reviewed Apr 23, 2021

View reviewed changes

niftynei added 2 commits April 23, 2021 12:18

tests: remove EXPERIMENTAL_FEATURE flag from openchannel2 hooks

2001b1f

These are now included in all builds

plugin-tests: cleanup outdated openchannel2 hook fields

536d032

We removed/changed the fields on the openchannel2 hook but never updated this test (which doesn't run with the CI for #reasons)

niftynei force-pushed the nifty/accept-plugin branch 3 times, most recently from 69aca6a to f773987 Compare April 25, 2021 14:51

niftynei force-pushed the nifty/accept-plugin branch from f773987 to fc77f51 Compare April 26, 2021 16:07

rustyrussell reviewed Apr 28, 2021

View reviewed changes

niftynei force-pushed the nifty/accept-plugin branch from fc77f51 to de7bbf0 Compare April 28, 2021 22:26

niftynei added 13 commits April 28, 2021 17:40

openchannel2: add channel_max_msat to openchannel2 hook payload

5d47c67

Changelog-Added: Plugins: add a `channel_max_msat` value to the `openchannel2` hook. Tells you the total max funding this channel is allowed to have.

psbt-open: method to quickly check if has our input

dea5f2d

For dual-funding's accepter plugin, we only want to send psbts that need to be signed to `signpsbt`; this lets us quickly check if they're "signable"

openchannel2/rbf hooks: reject if response malformed

e489b66

You gotta send over an amount if you send a psbt!

dualfund: set the locktime for the user-provided PSBT

c88f307

This is set by the peer and is non-negotiable. We're not even going to check if you got it right. You were told about it via `openchannel2`. It is what it is.

openchannel: add missing string args to format string

890ac99

Oops

amount: amount_sat_scale method

d8534e7

For scaling/multiplying sats

funder: use listfunds to fetch utxo data

e13b310

Compute available_funds locally, instead of getting it from the openchannel2 hook payload. Suggested-By: Rusty Russell @rustyrussell

funder-test: tests for our policy configurations

86eb1e0

contrib: if you're in dev mode, use dual-funding (with matching)

45e87a9

If you're using the regtest node, turn on dual funding and automatically attempt to dual fund at a 100% match for every channel open that you do.

funder: sanitize inputs

ec846cd

Error out if we've got the wrong info

funder: funderupdate command to see/set configs

9795006

Changelog-Added: Plugins: `funder` plugin now has new command `funderupdate` which will show current funding configuration and allow you to modify them

rbf_channel hook: add channel_max_msat parameter

d97ef4d

Changelog-Added: Plugins: `rbf_channel` hook has `channel_max_msat` parameter

niftynei added 3 commits April 29, 2021 11:23

funder: handle RBF callback

1e0f05f

Fund an RBF same as you would an open. We dont' do anything fancy with our inputs -- we dont' have a copy of the last PSBT that was sent.

pyln-testing: use provided outnum instead of trying to find it

884f1d4

Dual-funded channels won't match the old amount check. Good news is we're already returning the outnum so we just use that.

tests: add test for funder options

de9102a

niftynei force-pushed the nifty/accept-plugin branch from de7bbf0 to de9102a Compare April 29, 2021 16:31

rustyrussell merged commit 29155c2 into ElementsProject:master May 3, 2021

andrerfneves mentioned this pull request May 31, 2021

Add solo socratic 14 BitDevsNYC/BitDevsNYC.github.io#69

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a `funder` plugin #4489

add a `funder` plugin #4489

niftynei commented Apr 22, 2021 •

edited

Loading

rustyrussell Apr 22, 2021

rustyrussell Apr 22, 2021

rustyrussell commented Apr 23, 2021

niftynei commented Apr 26, 2021

rustyrussell left a comment

rustyrussell Apr 28, 2021

niftynei Apr 28, 2021

rustyrussell May 3, 2021

rustyrussell Apr 28, 2021

rustyrussell Apr 28, 2021

rustyrussell Apr 28, 2021

rustyrussell Apr 28, 2021

rustyrussell Apr 28, 2021

rustyrussell Apr 28, 2021

niftynei Apr 28, 2021

niftynei commented Apr 29, 2021

rustyrussell commented May 3, 2021

add a funder plugin #4489

add a funder plugin #4489

Conversation

niftynei commented Apr 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rustyrussell commented Apr 23, 2021

niftynei commented Apr 26, 2021

rustyrussell 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

niftynei commented Apr 29, 2021

rustyrussell commented May 3, 2021

add a `funder` plugin #4489

add a `funder` plugin #4489

niftynei commented Apr 22, 2021 •

edited

Loading