Get Quote and Pay for Inbound Channel #65

nickfarrow · 2022-11-15T15:59:15Z

Fetch quote from

https://nolooking.chaincase.app/api/getinbound?<nodeid>&<capacity>&<duration>&<refund_address>

(see nolooking/sateater#11)
Display address to pay for inbound channel!

public/index.html

src/http.rs

src/inbound.rs

DanGould · 2022-11-16T15:07:14Z

I think for this to make sense in our app we have to merge that address into the PayJoin PSBT. I didn't realize we were returning an address to the UI

DanGould · 2022-11-16T16:07:59Z

Taking a double take on this, I imagine a slightly different flow.

We could add a line to our channels ui with a checkbox like

<label>Inbound channel</label>  <input disabled value=1_000_000 \>

                                    I want a quote for an inbound channel (~30k sats)    [x]

on /schedule, find the quote, and return the ScheduledPayJoin with the bip21 that pays for it. We can show if that quote succeeded or failed, as well as the quote there, all ready to be paid. This design intends to minimize interaction and points of failure.

DanGould · 2022-11-16T19:37:33Z

Changed so /schedule takes a request for inbound and if so adds the output to the proposal psbt

UI, error handling, integration test upgrade to include inbound payment, and response status update TODO

src/scheduler.rs

DanGould · 2022-11-16T17:40:41Z

src/main.rs

@@ -1,5 +1,6 @@
 pub mod args;
 mod http;
+pub mod inbound;


I don't think this interface should be public. Inbound can be offered by the scheduler, but not on its own.

src/inbound.rs

DanGould · 2022-11-17T02:08:20Z

DanGould · 2022-11-17T04:27:48Z

"Allow channels of length 0" commit should

allow opening 0 outbound channels
require inbound to be selected in order to form a valid request with 0 outbound
be better named to reflect that change

DanGould · 2022-11-17T16:57:31Z

newly exposed rust api should be reigned in
UI should use template literals to display on POST /schedule response
errors still need to be handled
integration test upgrade could include inbound payment to a dummy endpoint
I would still like the endpoint to be POST instead of GET too

DanGould · 2022-11-17T22:55:46Z

src/scheduler.rs

+    fn fees(&self) -> bitcoin::Amount {
+        let channel_count = self.channels.len() as u64;
+        let has_reserve_deposit = self.reserve_deposit != bitcoin::Amount::ZERO;
+
+        let mut additional_vsize = if has_reserve_deposit {
+            // <8 invariant bytes = 4 version + 4 locktime>
+            //  + 2 variant bytes for input.len + output.len such that each len < 252
+            //  + OP_0 OP_PUSHBYTES_32 <32 byte script>
+            channel_count * (8 + 1 + 1 + 34)
+        } else {
+            // substitute 1 p2wsh channel (34 bytes) open for 1 p2wpkh reserve output (22 bytes)
+            // that's + 12 bytes
+            (channel_count - 1) * (8 + 1 + 1 + 34) + 12
+        };
+
+        if self.quote.is_some() {
+            additional_vsize = additional_vsize + (8 + 1 + 1 + 22); // P2WPKH (OP_0 OP_PUSHBYTES_20 <20 byte script)
+        }
+
+        bitcoin::Amount::from_sat(self.fee_rate * additional_vsize)


I think this might slightly overestimate the fees. I think that's acceptable to me for an alpha. This fee calculation belongs in the payjoin crate and should be made perfect for any script type

I reckon we should try avoid doing diy fees, is there a reason we're having to calculate yourself as opposed to rust-bitcoin or something?

Yes, we should avoid DIY fee calc. It can lead to many problems if done poorly.

I don't believe rust-bitcoin and rust-miniscript have any fee calculation. BDK is in its infancy for fee calculation.

State of the art fee calculation candidate for rust-bitcoin is actually in the payjoin crate but only exposed to the sender module. You can follow the proposal to upstream the relevant code in rust-bitcoin here and the (good but more testing would be better) fee calculation in the payjoin crate here

see rust-bitcoin/rust-bitcoin@c34d5f8 recently in master but not yet in a release

The important thing to remember is the sender always signs off on the fee calculation. This just sets a minimum.

DanGould · 2022-11-18T04:42:36Z

I think we should proceed with this PR now without altering the integration test. The nolooking lsp is now tested and working on regtest.

The lsp module has no config for the lsp, so it always hits mainnet. I think that's fine for the next tournament release since we have done manual tests, but it makes writing the integration test harder. The lsp code needs to change to inject the lsp url. that should be the next issue.

This is working as is and imo should be merged assuming CI passes

src/http.rs

DanGould · 2022-11-20T00:57:33Z

src/lsp.rs

+    // get address
+    // suggest capacity
+    let base_uri = format!(
+        "https://nolooking.chaincase.app/api/request-inbound?nodeid={}&capacity={}&duration={}&refund_address={}",


should really be a config string but it's viable for this PR

DanGould

Thank you for the rebase! You saved me a lot of time recovering from a broken git state.

I think the last commit belongs in its own PR. The design does not yet allow for integration testing and that's the point of the configuration

DanGould · 2022-11-20T14:16:58Z

src/lsp.rs

@@ -19,14 +19,15 @@ pub struct Quote {
 pub async fn request_quote(
    p2p_address: &P2PAddress,
    refund_address: &Address,
+    lsp_endpoint: &String,


The point of this commit IMO is to make the LSP stubbable or testable. That means initializing it from a config dependency that can be changed on test. I don't think the endpoint should be a parameter of this function unless it's attached to self

DanGould · 2022-11-20T14:20:25Z

src/scheduler.rs

+    pub fn new(lnd: LndClient, endpoint: Url, lsp_endpoint: String) -> Self {
+        Self { lnd, endpoint, lsp_endpoint, pjs: Default::default() }
    }

    pub async fn from_config(config: &crate::config::Config) -> Result<Self, SchedulerError> {
-        Ok(Scheduler::new(LndClient::from_config(&config).await?, config.endpoint.parse().expect("Malformed secure endpoint from config file. Expecting a https or .onion URI to proxy payjoin requests")))
+        Ok(Scheduler::new(LndClient::from_config(&config).await?, config.endpoint.parse().expect("Malformed secure endpoint from config file. Expecting a https or .onion URI to proxy payjoin requests"), config.endpoint.parse().expect("Malformed secure lsp endpoint in config file. Expecting a https url")))


I also don't think Scheduler should know about lsp_endpoint. Instead it could know about LspClient and its public interface including request_quote. Initializing LspClient `from_config would be a better encapsulated design.

It should also be a Url type, like endpoint.

When a users requests a quote for inbound channel capacity, the lightning service provider can return an address to include in their PayJoin PSBT with a price. By paying this address, the user can have a channel opened to them paying from the same transaction they open outbound channels.

DanGould · 2022-11-20T14:31:41Z

src/http.rs

+pub struct ScheduleResponse {
+    bip21: String,
+    address: String,
+    quote: Option<Quote>,
+}


This should really be strongly typed with bitcoin::Amount, bitcoin::Amount, std::time::Duration (I have NO clue this u32 duration is. ms? seconds? minutes? months? you get my point) and bitcoin::Address but this PR works and we need to ship. It's not a regression to our codebase but I know we do cleaner work than this.

nickfarrow marked this pull request as draft November 15, 2022 15:59

nickfarrow force-pushed the inbound branch from cbcc040 to ca25fc2 Compare November 15, 2022 16:09

DanGould reviewed Nov 15, 2022

View reviewed changes

public/index.html Outdated Show resolved Hide resolved

src/http.rs Outdated Show resolved Hide resolved

src/inbound.rs Outdated Show resolved Hide resolved

nickfarrow marked this pull request as ready for review November 16, 2022 11:59

DanGould force-pushed the inbound branch from 6850a81 to e9c30f8 Compare November 16, 2022 19:36

DanGould reviewed Nov 16, 2022

View reviewed changes

DanGould force-pushed the inbound branch from e9c30f8 to cd6f052 Compare November 17, 2022 00:23

DanGould force-pushed the inbound branch from cd6f052 to bf38db8 Compare November 17, 2022 04:01

nickfarrow force-pushed the inbound branch from 776e8f4 to d8e764e Compare November 17, 2022 10:59

DanGould force-pushed the inbound branch from d8e764e to 5e10f60 Compare November 17, 2022 16:52

DanGould force-pushed the inbound branch 3 times, most recently from a86041f to cd854d9 Compare November 17, 2022 22:54

DanGould reviewed Nov 17, 2022

View reviewed changes

DanGould force-pushed the inbound branch from cd854d9 to 3d32717 Compare November 17, 2022 22:58

nickfarrow force-pushed the inbound branch from 9e2a522 to 3d32717 Compare November 18, 2022 01:29

DanGould force-pushed the inbound branch 2 times, most recently from b278d28 to e11cf41 Compare November 18, 2022 04:42

DanGould force-pushed the inbound branch 3 times, most recently from 43525f4 to b23e800 Compare November 18, 2022 15:06

DanGould requested changes Nov 20, 2022

View reviewed changes

DanGould force-pushed the inbound branch from b23e800 to fd331d6 Compare November 20, 2022 01:03

nickfarrow force-pushed the inbound branch from fd331d6 to 986dcb9 Compare November 20, 2022 11:41

DanGould reviewed Nov 20, 2022

View reviewed changes

nickfarrow and others added 8 commits November 20, 2022 09:27

Move calculate_fees() to fees(&self)

63cbd48

Rename wallet_amount to reserve_deposit

ae9d62a

Calculate PayJoin fees including inbound quote

5781fd5

Add inbound payment output to ScheduledPayJoin

e5deba6

Create Inbound channel request interface

1bd8c2b

Respond to POST /schedule with json

648534b

Show quote information if requested

2953eed

DanGould reviewed Nov 20, 2022

View reviewed changes

DanGould force-pushed the inbound branch from 986dcb9 to 2953eed Compare November 20, 2022 14:45

DanGould merged commit 8976750 into payjoin:master Nov 20, 2022

DanGould mentioned this pull request Nov 21, 2022

Inbound Channels #40

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get Quote and Pay for Inbound Channel #65

Get Quote and Pay for Inbound Channel #65

nickfarrow commented Nov 15, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould Nov 16, 2022

DanGould commented Nov 17, 2022

DanGould commented Nov 17, 2022 •

edited

Loading

DanGould commented Nov 17, 2022 •

edited

Loading

DanGould Nov 17, 2022 •

edited

Loading

nickfarrow Nov 18, 2022

DanGould Nov 18, 2022

DanGould Nov 18, 2022

DanGould Nov 20, 2022

DanGould commented Nov 18, 2022

DanGould Nov 20, 2022

DanGould left a comment •

edited

Loading

DanGould Nov 20, 2022 •

edited

Loading

DanGould Nov 20, 2022

DanGould Nov 20, 2022

DanGould Nov 20, 2022

Get Quote and Pay for Inbound Channel #65

Get Quote and Pay for Inbound Channel #65

Conversation

nickfarrow commented Nov 15, 2022 • edited Loading

DanGould commented Nov 16, 2022 • edited Loading

DanGould commented Nov 16, 2022 • edited Loading

DanGould commented Nov 16, 2022 • edited Loading

Choose a reason for hiding this comment

DanGould commented Nov 17, 2022

DanGould commented Nov 17, 2022 • edited Loading

DanGould commented Nov 17, 2022 • edited Loading

DanGould Nov 17, 2022 • edited Loading

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

DanGould commented Nov 18, 2022

Choose a reason for hiding this comment

DanGould left a comment • edited Loading

Choose a reason for hiding this comment

DanGould Nov 20, 2022 • edited Loading

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

nickfarrow commented Nov 15, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould commented Nov 16, 2022 •

edited

Loading

DanGould commented Nov 17, 2022 •

edited

Loading

DanGould commented Nov 17, 2022 •

edited

Loading

DanGould Nov 17, 2022 •

edited

Loading

DanGould left a comment •

edited

Loading

DanGould Nov 20, 2022 •

edited

Loading