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.

Sign up for GitHub

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

askrene-getroutes: fix the value of the amount and delay #8066

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

askrene-getroutes: fix the value of the amount and delay #8066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions contrib/msggen/msggen/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16190,7 +16190,7 @@
"delay": {
"type": "u32",
"description": [
"The total CLTV expected by the node at the start of this hop."
"The total CLTV on this hop's HTLC."
]
}
}
Expand Down Expand Up @@ -16241,14 +16241,14 @@
{
"short_channel_id_dir": "109x1x1/1",
"next_node_id": "nodeid020202020202020202020202020202020202020202020202020202020202",
"amount_msat": 1250026,
"delay": 12
"amount_msat": 1250013,
"delay": 6
},
{
"short_channel_id_dir": "123x1x1/0",
"next_node_id": "nodeid030303030303030303030303030303030303030303030303030303030303",
"amount_msat": 1250013,
"delay": 6
"amount_msat": 1250000,
"delay": 0
}
]
}
Expand Down
10 changes: 5 additions & 5 deletions doc/schemas/lightning-getroutes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
"delay": {
"type": "u32",
"description": [
"The total CLTV expected by the node at the start of this hop."
"The total CLTV on this hop's HTLC."
]
}
}
Expand Down Expand Up @@ -206,14 +206,14 @@
{
"short_channel_id_dir": "109x1x1/1",
"next_node_id": "nodeid020202020202020202020202020202020202020202020202020202020202",
"amount_msat": 1250026,
"delay": 12
"amount_msat": 1250013,
"delay": 6
},
{
"short_channel_id_dir": "123x1x1/0",
"next_node_id": "nodeid030303030303030303030303030303030303030303030303030303030303",
"amount_msat": 1250013,
"delay": 6
"amount_msat": 1250000,
"delay": 0
}
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If we were to make this change, what is the fee for this route? What is the total delay? You can no longer tell? Information has been lost.

We can't change the API anyway, but we could do:

  1. Add "node_id" with the prev node id (to make that explicit)
  2. Add "amount_msat_for_next" and "delay_for_next" which would be these fields.
  3. Make the documentation describe these fields more clearly.

Copy link
Collaborator Author

@Lagrang3 Lagrang3 Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Agree, we could add those fields instead of changing the API.
Or we could add for each route (not each hop) a total_delay and total_amount_msat, values.

Or maybe leave getroutes as it is and create a new rpc, for instance askrene-routes, just any different name,
that gives us precisely the quantities that we need to feed to the sphinx_path API.

For example:

deliver 1000 to C, final_cltv = 10
A -> B -> C
base fee = 1
delay delta = 6

routes: [
    {total_amount: 1002,
     final_amount: 1000,
     total_delay: 22,
     final_delay: 10,
     final_node: C,
     path: [ {node: A, channel: AB, amount: 1001, delay: 16},
                  {node: B, channel: BC, amount: 1000, delay: 10}] }
]

To build the onions:

for each route:
    sp = new sphinx_path()
    for each hop in route['path']:
        payload = onion_nonfinal_hop(hop['channel'], hop['amount'], hop['delay']+blockheight)
        sp.add_hop(hop['node'], payload)
    sp.add_final_hop(route['final_node'], route['final_amount'], ...)

Expand Down
4 changes: 2 additions & 2 deletions plugins/askrene/askrene.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,8 @@ static const char *get_routes(const tal_t *ctx,
struct gossmap_node *far_end;
const struct half_chan *h = flow_edge(flows[i], j);

rh->amount = msat;
rh->delay = delay;
if (!amount_msat_add_fee(&msat, h->base_fee, h->proportional_fee))
plugin_err(rq->plugin, "Adding fee to amount");
delay += h->delay;
Expand All @@ -534,8 +536,6 @@ static const char *get_routes(const tal_t *ctx,
rh->direction = flows[i]->dirs[j];
far_end = gossmap_nth_node(rq->gossmap, flows[i]->path[j], !flows[i]->dirs[j]);
gossmap_node_get_id(rq->gossmap, far_end, &rh->node_id);
rh->amount = msat;
rh->delay = delay;
}
(*amounts)[i] = flows[i]->delivers;
rq_log(tmpctx, rq, LOG_INFORM, "Flow %zu/%zu: %s",
Expand Down
18 changes: 5 additions & 13 deletions plugins/xpay/xpay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,8 @@ struct hop {
struct pubkey next_node;
/* Via this channel */
struct short_channel_id_dir scidd;
/* This is amount the node needs (including fees) */
struct amount_msat amount_in;
/* ... to send this amount */
struct amount_msat amount_out;
/* This is the delay, including delay across node */
u32 cltv_value_in;
/* This is the delay, out from node. */
u32 cltv_value_out;
};
Expand Down Expand Up @@ -347,14 +343,14 @@ static struct amount_msat initial_sent(const struct attempt *attempt)
{
if (tal_count(attempt->hops) == 0)
return attempt->delivers;
return attempt->hops[0].amount_in;
return attempt->hops[0].amount_out;
}

static u32 initial_cltv_delta(const struct attempt *attempt)
{
if (tal_count(attempt->hops) == 0)
return attempt->payment->final_cltv;
return attempt->hops[0].cltv_value_in;
return attempt->hops[0].cltv_value_out;
}

/* The current attempt is the first to succeed: we assume all the ones
Expand Down Expand Up @@ -776,7 +772,7 @@ static struct amount_msat total_fees_being_sent(const struct payment *payment)
if (tal_count(attempt->hops) == 0)
continue;
if (!amount_msat_sub(&fee,
attempt->hops[0].amount_in,
attempt->hops[0].amount_out,
attempt->delivers))
abort();
if (!amount_msat_accumulate(&sum, fee))
Expand Down Expand Up @@ -1053,16 +1049,12 @@ static struct command_result *getroutes_done(struct command *aux_cmd,
",delay:%}",
JSON_SCAN(json_to_short_channel_id_dir,
&hop->scidd),
JSON_SCAN(json_to_msat, &hop->amount_in),
JSON_SCAN(json_to_msat, &hop->amount_out),
JSON_SCAN(json_to_pubkey, &hop->next_node),
JSON_SCAN(json_to_u32, &hop->cltv_value_in));
JSON_SCAN(json_to_u32, &hop->cltv_value_out));
if (err)
plugin_err(aux_cmd->plugin, "Malformed routes: %s",
err);
if (j > 0) {
hops[j-1].amount_out = hop->amount_in;
hops[j-1].cltv_value_out = hop->cltv_value_in;
}
}
hops[j-1].amount_out = delivers;
hops[j-1].cltv_value_out = payment->final_cltv;
Expand Down
Loading
Loading