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 #4639

Merged
merged 56 commits into from
Jul 20, 2021
Merged

liquidity ads #4639

merged 56 commits into from
Jul 20, 2021

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Jul 9, 2021

Implements lightning/bolts#878

@niftynei niftynei requested a review from cdecker as a code owner July 9, 2021 18:01
@niftynei niftynei added this to the v0.10.1 milestone Jul 9, 2021
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack e364d06

@@ -60,6 +61,9 @@ struct daemon {

/* Features lightningd told us to set. */
struct feature_set *our_features;

/* The channel lease rates we're advertising */
struct lease_rates *rates;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably could be const?

fromwire_lease_rates(pptr, max, entry->rates);
} else
entry->rates = NULL;

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, these functions are no longer used and should have been totally deleted. Will do after.

@@ -125,26 +126,57 @@ static void get_nannounce_parts(const u8 *node_announcement,
sizes[0] = 2 + fromwire_u16(&flen, &len);
assert(flen != NULL && len >= 4);

/* BOLT-0fe3485a5320efaa2be8cfa0e570ad4d0259cec3 #7:
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I've stared using BOLT= instead, rather than commit message. Makes it a bit harder to check, but good fi spec is in flux.

const u8 *orig;

/* Get last one we have. */
orig = gossip_store_get(tmpctx, gs, node->bcast.index);
get_nannounce_parts(orig, oparts, osizes);
get_nannounce_parts(nannounce, nparts, nsizes);

*only_missing_tlv = memeq(oparts[0], osizes[0], nparts[0], nsizes[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

Let this be NULL? One caller wants that...

}
}

sign_and_send_nannounce(daemon, nannounce, timestamp);
Copy link
Contributor

Choose a reason for hiding this comment

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

THis looks like a lot of cut and paste in what could be a flag? Can be fixed after though.

bitcoin/psbt.c Outdated
*
* The minimum witness weight for an input is 110.
*/
weight += 110;
Copy link
Contributor

Choose a reason for hiding this comment

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

+= ?? This seems wrong?

Also, psbt_input_weight is a very generic name for something containing this: suggest minweight as a parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whatever code that I was using for this no longer exists, so I'm deleting this commit.

"funding_feerate": funding_feerate,
}
return self.call("queryrates", payload)

Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid adding it to lightning.py: it converts unknown calls automatically.

msgdata,dualopend_validate_lease,their_pubkey,pubkey,

msgtype,dualopend_validate_lease_reply,7127
msgdata,dualopend_validate_lease_reply,err_msg,?wirestring,
Copy link
Contributor

Choose a reason for hiding this comment

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

I probably would have left dualopend to validate the signature, and hand it up when it says channel is open, but this works too (and can be simplified later if we want).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it seems made the (incorrect) assumption that only lightningd could verify these. Ooof. Will leave as is for now.

bitcoin/script.c Outdated
* OP_ENDIF
* OP_CHECKSIG
*/
u8 *bitcoin_wscript_to_local(const tal_t *ctx, u16 to_self_delay, u32 csv,
Copy link
Contributor

Choose a reason for hiding this comment

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

csv is a terrible name for this parameter! lease_remaining?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated!


The **parsefeerate** command returns the current feerate for any valid
*feerate_str*. This is useful for finding the current feerate that a
**fundpsbt** or **utxopsbt** command might use.
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole commit seems unnecessary? feerates does this already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not exactly, feerates returns the named current feerates; this takes any arbitrary feerate string and returns its value in perkw. I mean, technically, I could halfway parse the feerate string, figure out if it's a plain number or a named fee rate and if it's the latter match it up with the returned fields from feerates. Except that's a bit more work than just piping it through to lightningd to do it for me instead?

@rustyrussell
Copy link
Contributor

Added some neatening commits on the end, and trivial rebase now first commit is merged.

u32 *lease_basis, *channel_fee_max_ppt, *funding_weight;

if (!param(cmd, buffer, params,
p_req("lease_fee_base_msat", param_sat, &lease_base_sat),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really struggled with what to name this. The field itself is satoshi only, but given that we print our RPCs out uniformly as msats, I went with the msat designation... (even though only sats are valid.)

Comment on lines 11 to 16
/* FIXME: why can't i do memeqzero? */
return rates->funding_weight == 0
&& rates->channel_fee_max_base_msat == 0
&& rates->channel_fee_max_proportional_thousandths == 0
&& rates->lease_fee_base_sat == 0
&& rates->lease_fee_basis == 0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

acking your later fix :)

Comment on lines -982 to +1039
assert funder_opts['fuzz_percent'] == 5
assert funder_opts['fuzz_percent'] == 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Broken out to separate commit.

|| chan_fee_msats)
policy->rates = default_lease_rates(policy);
else
policy->rates = NULL;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The true default is NULL; we only use the defaults iff any one of the parameters is set.

bitcoin/psbt.c Outdated
*
* The minimum witness weight for an input is 110.
*/
weight += 110;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whatever code that I was using for this no longer exists, so I'm deleting this commit.

msgdata,dualopend_validate_lease,their_pubkey,pubkey,

msgtype,dualopend_validate_lease_reply,7127
msgdata,dualopend_validate_lease_reply,err_msg,?wirestring,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it seems made the (incorrect) assumption that only lightningd could verify these. Ooof. Will leave as is for now.

bitcoin/script.c Outdated
* OP_ENDIF
* OP_CHECKSIG
*/
u8 *bitcoin_wscript_to_local(const tal_t *ctx, u16 to_self_delay, u32 csv,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated!


The **parsefeerate** command returns the current feerate for any valid
*feerate_str*. This is useful for finding the current feerate that a
**fundpsbt** or **utxopsbt** command might use.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not exactly, feerates returns the named current feerates; this takes any arbitrary feerate string and returns its value in perkw. I mean, technically, I could halfway parse the feerate string, figure out if it's a plain number or a named fee rate and if it's the latter match it up with the returned fields from feerates. Except that's a bit more work than just piping it through to lightningd to do it for me instead?

const struct json_out *params TAKES,
const char *guide,
...)
static const char *rpc_scan_list(const tal_t *ctx,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't actually end up using this commit, I'm throwing it away.

@niftynei niftynei force-pushed the nifty/liquid-ad-open branch 4 times, most recently from ed51715 to ac8e879 Compare July 19, 2021 19:06
Import the wires from spec. Here we go!
When we get a node announcement out, we can now pull out its offer
characteristic
Correctly mark whether or not the TLV's are the same/different, given two
node announcements
If there's a rate-card for liquidity, we don't know about it until
after startup (the plugin *should* call us at init to tell us what their
current rates are)
Changelog-Experimental: JSON-RPC: new RPC `setleaserates`, for passing in the rates to advertise for a channel lease (option_will_fund)
Note that we use the names from the spec.

Changelog-Experimental: JSON-RPC: `listnodes` now includes the `lease_rates`, if available
A couple of the fields for liquidity_ads are u16
If received lease rates are empty (all zeroes), turn them off
As we move to advertising liquidity, set the default to not have any
fuzz. This means we won't randomly reject an offer to buy liquidity.
Implement support for liquidity ads in `funder` plugin. We set the
command line options for the leases, as well as sending the updated ads
to lightningd (who then passes them through to gossipd)
Make sure everything updates/flows through as expected.
convenience, mostly
When we accept a bid to create a channel lease, we send back a signature
committing to our max channel lease amounts.
When a request comes through, we forward it over to the funder who
uses the currently set policy to figure out how to handle it.

Includes small update to the policy engine which decides whether or not
to fund a request.

Changelog-Experimental: Plugins: `openchannel2` hook now includes optional fields for a channel lease request
Changelog-Experimental: EXPERIMENTAL-DUAL-FUND: JSON-RPC: openchannel_init now takes a `requested_amt`, which is an amount to request from peer
Changelog-Added: JSON-RPC: `fundchannel` now takes optional `request_amt` parameter
niftynei and others added 26 commits July 19, 2021 20:05
Adds new tables to database, backfills, basically copies the fee_rates
state machine for channeld.
Useful for parsing a passed in feerate before calling lightningd with
it, e.g. when you need to know what the feerate is for a fundpsbt before
calling fundpsbt

Changelog-Added: JSON-RPC: new command `parsefeerate` which takes a feerate string and returns the calculated perkw/perkb
If the channel is under lease, the lessor's (accepter, really)
funds have a CSV lock that reflects the length of the remainder of the
lease.
Channel leases modify the CSV height that an output is eligible for
being spent at,  persist this to the database
If an output's CSV lock hasn't been surpassed yet, don't try to
include it in a transaction
Typically we forget a channel if 2016 blocks have passed and
the funding transaction hasn't been mined yet, however we
SHOULD NOT forget these channels if we've got funds in them!
If the channel is leased and our peer is too far behind, fail the
channel.
By default, we won't close a channel that we leased to a peer.
You can override this with the `force_lease_closed` flag.

Changelog-Added: JSON-RPC: close now has parameter to force close a leased channel (option_will_fund)
We need to update the peer wrt our blockheight on reconnect
We need to know what the lease we're expecting is. To do this
we pass around the hex encoded portion of the wire format.

We can use this passed in expected lease rates to confirm that the peer
is, in fact, using the same rates as what we have currently.

Changelog-Added: JSON-RPC: fundchannel, multifundchannel, and openchannel_init now accept a 'compact_lease' for any requested funds
We need to parse the feerate string, so we can figure out what our
weight fee will be for a leased channel, so we go get the feerate
and use that to calculate what our expected lease fee will be for
the requested amount.
If there's no plugin currently in place, we simply won't return any
funding at all, in which case we'd expect them to handle however
they want. (our implementation would fail the open, as we only accept
opens that have at least as much as we've requested provided)
Make it easier to figure out what the inputs were to check_balances
on two failure cases
We need the 'actual' accepter's funding for the reserve calculations,
which includes the lease fee that the opener is paying them, so we
calculate it before doing all that jazz.

However, we MUST send the actual "on paper" (e.g. without lease fee)
amount to the peer in accept_channel2, so we stash the original amount
and send it.
with channel leases, we use the open_tlv data after a round of talking
to the peer (which clears out the tmpctx). It'll get cleaned up
when this peer opens/fails.
Check that channel leases work as expected for unilateral closes
and cheats!
Since we now use 'compact_lease' to gate an open (if the rates have
changed, we fail), we no longer need to rely on query rates for figuring
things out, so we make it dev-only.

Changelog-Changed: JSON-API: queryrates is now developer only
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't actually use the NULL-is-equal in the one caller, so ignore that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And fix up lease_rates_fromhex to be the obvious calling convention.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@niftynei
Copy link
Collaborator Author

ACK 7be8e71

@niftynei niftynei merged commit 35bec51 into ElementsProject:master Jul 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants