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

Restore payment amount fuzzing #3212

Merged
merged 5 commits into from
Nov 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions common/json.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,21 @@ bool json_to_number(const char *buffer, const jsmntok_t *tok,
return true;
}

bool json_to_u16(const char *buffer, const jsmntok_t *tok,
short unsigned int *num)
{
uint64_t u64;

if (!json_to_u64(buffer, tok, &u64))
return false;
*num = u64;

/* Just in case it doesn't fit. */
if (*num != u64)
return false;
return true;
}

bool json_to_int(const char *buffer, const jsmntok_t *tok, int *num)
{
char *end;
Expand Down
4 changes: 4 additions & 0 deletions common/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ bool json_to_number(const char *buffer, const jsmntok_t *tok,
bool json_to_u64(const char *buffer, const jsmntok_t *tok,
uint64_t *num);

/* Extract number from this (may be a string, or a number literal) */
bool json_to_u16(const char *buffer, const jsmntok_t *tok,
uint16_t *num);

/* Extract double from this (must be a number literal) */
bool json_to_double(const char *buffer, const jsmntok_t *tok, double *num);

Expand Down
30 changes: 29 additions & 1 deletion contrib/pylightning/lightning/lightning.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,28 @@ def dev_memleak(self):
"""
return self.call("dev-memleak")

def dev_pay(self, bolt11, msatoshi=None, label=None, riskfactor=None,
description=None, maxfeepercent=None, retry_for=None,
maxdelay=None, exemptfee=None, use_shadow=True):
"""
A developer version of `pay`, with the possibility to deactivate
shadow routing (used for testing).
"""
payload = {
"bolt11": bolt11,
"msatoshi": msatoshi,
"label": label,
"riskfactor": riskfactor,
"maxfeepercent": maxfeepercent,
"retry_for": retry_for,
"maxdelay": maxdelay,
"exemptfee": exemptfee,
"use_shadow": use_shadow,
# Deprecated.
"description": description,
}
return self.call("pay", payload)

def dev_reenable_commit(self, peer_id):
"""
Re-enable the commit timer on peer {id}
Expand Down Expand Up @@ -761,7 +783,9 @@ def newaddr(self, addresstype=None):
"""
return self.call("newaddr", {"addresstype": addresstype})

def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, description=None):
def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None,
description=None, maxfeepercent=None, retry_for=None,
maxdelay=None, exemptfee=None):
"""
Send payment specified by {bolt11} with {msatoshi}
(ignored if {bolt11} has an amount), optional {label}
Expand All @@ -772,6 +796,10 @@ def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, description=No
"msatoshi": msatoshi,
"label": label,
"riskfactor": riskfactor,
"maxfeepercent": maxfeepercent,
"retry_for": retry_for,
"maxdelay": maxdelay,
"exemptfee": exemptfee,
# Deprecated.
"description": description,
}
Expand Down
22 changes: 11 additions & 11 deletions doc/lightning-pay.7
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ randomization\.
1: Route Randomization


2: Shadow Route


Route randomization means the payment algorithm does not always use the
lowest-fee or shortest route\. This prevents some highly-connected node
from learning all of the user payments by reducing their fees below the
network average\.


Shadow route means the payment algorithm will virtually extend the time
delays along the route, making it appear to intermediate nodes that the
route is longer than it actually is\. This prevents intermediate nodes
from reliably guessing their distance from the payee\.
2: Shadow Route


Shadow route means the payment algorithm will virtually extend the route
by adding delays and fees along it, making it appear to intermediate nodes
that the route is longer than it actually is\. This prevents intermediate
nodes from reliably guessing their distance from the payee\.


Route randomization will never exceed \fImaxfeepercent\fR of the payment\.
Expand All @@ -84,6 +84,7 @@ You can monitor the progress and retries of a payment using the

The following error codes may occur:

.RS
.IP \[bu]
-1: Catchall nonspecific error\.
.IP \[bu]
Expand All @@ -109,13 +110,15 @@ invoice expiration) as UNIX epoch time in seconds\.
.IP \[bu]
210: Payment timed out without a payment in progress\.

.RE

Error codes 202 and 204 will only get reported at \fBsendpay\fR; in
\fBpay\fR we will keep retrying if we would have gotten those errors\.


A routing failure object has the fields below:

.RS
.IP \[bu]
\fIerring_index\fR: The index of the node along the route that reported
the error\. 0 for the local node, 1 for the first hop, and so on\.
Expand All @@ -132,6 +135,7 @@ error, or \fI0:0:0\fR if the destination node raised the error\.
received from the remote node\. Only present if error is from the
remote node and the \fIfailcode\fR has the UPDATE bit set, as per BOLT #4\.

.RE

The \fIdata\fR field of errors will include statistics \fIgetroute_tries\fR and
\fIsendpay_tries\fR\. It will also contain a \fIfailures\fR field with detailed
Expand All @@ -150,7 +154,3 @@ Rusty Russell \fI<[email protected]\fR> is mainly responsible\.

Main web site: \fIhttps://github.com/ElementsProject/lightning\fR

.HL

Last updated 2019-08-01 14:59:36 CEST

12 changes: 6 additions & 6 deletions doc/lightning-pay.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ randomization.

1: Route Randomization

2: Shadow Route

Route randomization means the payment algorithm does not always use the
lowest-fee or shortest route. This prevents some highly-connected node
from learning all of the user payments by reducing their fees below the
network average.

Shadow route means the payment algorithm will virtually extend the time
delays along the route, making it appear to intermediate nodes that the
route is longer than it actually is. This prevents intermediate nodes
from reliably guessing their distance from the payee.
2: Shadow Route

Shadow route means the payment algorithm will virtually extend the route
by adding delays and fees along it, making it appear to intermediate nodes
that the route is longer than it actually is. This prevents intermediate
nodes from reliably guessing their distance from the payee.

Route randomization will never exceed *maxfeepercent* of the payment.
Route randomization and shadow routing will not take routes that would
Expand Down
61 changes: 45 additions & 16 deletions plugins/pay.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ struct pay_command {
/* Any remaining routehints to try. */
struct route_info **routehints;

#if DEVELOPER
/* Disable the use of shadow route ? */
double use_shadow;
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this dev-only please? I don't want normal people to disable shadow routing, it's a terrible idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do

/* Current node during shadow route calculation. */
const char *shadow_dest;
};
Expand Down Expand Up @@ -820,25 +825,30 @@ static struct command_result *add_shadow_route(struct command *cmd,
const jsmntok_t *chan, *best = NULL;
size_t i;
u64 sample = 0;
u32 cltv, best_cltv;
struct route_info *route = tal_arr(NULL, struct route_info, 1);

json_for_each_arr(i, chan, channels) {
struct amount_sat sat;
u64 v;
u64 v = pseudorand(UINT64_MAX);

json_to_sat(buf, json_get_member(buf, chan, "satoshis"), &sat);
if (amount_msat_greater_sat(pc->msat, sat))
continue;
if (!best || v > sample) {
struct amount_sat sat;

/* Don't use if total would exceed 1/4 of our time allowance. */
json_to_number(buf, json_get_member(buf, chan, "delay"), &cltv);
if ((pc->final_cltv + cltv) * 4 > pc->maxdelay)
continue;
json_to_sat(buf, json_get_member(buf, chan, "satoshis"), &sat);
if (amount_msat_greater_sat(pc->msat, sat))
continue;

/* Don't use if total would exceed 1/4 of our time allowance. */
json_to_u16(buf, json_get_member(buf, chan, "delay"),
&route[0].cltv_expiry_delta);
if ((pc->final_cltv + route[0].cltv_expiry_delta) * 4 > pc->maxdelay)
continue;

json_to_number(buf, json_get_member(buf, chan, "base_fee_millisatoshi"),
&route[0].fee_base_msat);
json_to_number(buf, json_get_member(buf, chan, "fee_per_millionth"),
&route[0].fee_proportional_millionths);

v = pseudorand(UINT64_MAX);
if (!best || v > sample) {
best = chan;
best_cltv = cltv;
sample = v;
}
}
Expand All @@ -850,18 +860,28 @@ static struct command_result *add_shadow_route(struct command *cmd,
return start_pay_attempt(cmd, pc, "Initial attempt");
}

pc->final_cltv += best_cltv;
pc->final_cltv += route[0].cltv_expiry_delta;
pc->shadow_dest = json_strdup(pc, buf,
json_get_member(buf, best, "destination"));
route_msatoshi(&pc->msat, pc->msat, route, 1);
tal_append_fmt(&pc->ps->shadow,
"Added %u cltv delay for shadow to %s. ",
best_cltv, pc->shadow_dest);
"Added %u cltv delay, %u base fee, and %u ppm fee "
"for shadow to %s.",
route[0].cltv_expiry_delta, route[0].fee_base_msat,
route[0].fee_proportional_millionths,
pc->shadow_dest);
tal_free(route);

return shadow_route(cmd, pc);
}

static struct command_result *shadow_route(struct command *cmd,
struct pay_command *pc)
{
#if DEVELOPER
if (!pc->use_shadow)
return start_pay_attempt(cmd, pc, "Initial attempt");
#endif
if (pseudorand(2) == 0)
return start_pay_attempt(cmd, pc, "Initial attempt");

Expand Down Expand Up @@ -1023,6 +1043,9 @@ static struct command_result *json_pay(struct command *cmd,
double *maxfeepercent;
unsigned int *maxdelay;
struct amount_msat *exemptfee;
#if DEVELOPER
bool *use_shadow;
#endif

if (!param(cmd, buf, params,
p_req("bolt11", param_string, &b11str),
Expand All @@ -1034,6 +1057,9 @@ static struct command_result *json_pay(struct command *cmd,
p_opt_def("maxdelay", param_number, &maxdelay,
maxdelay_default),
p_opt_def("exemptfee", param_msat, &exemptfee, AMOUNT_MSAT(5000)),
#if DEVELOPER
p_opt_def("use_shadow", param_bool, &use_shadow, true),
#endif
NULL))
return command_param_failed();

Expand Down Expand Up @@ -1085,6 +1111,9 @@ static struct command_result *json_pay(struct command *cmd,
pc->current_routehint = NULL;
pc->routehints = filter_routehints(pc, b11->routes);
pc->expensive_route = NULL;
#if DEVELOPER
pc->use_shadow = *use_shadow;
#endif

/* Get capacities of local channels (no parameters) */
return send_outreq(cmd, "listpeers", listpeers_done, forward_error, pc,
Expand Down
8 changes: 4 additions & 4 deletions tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ def test_htlc_sig_persistence(node_factory, bitcoind, executor):
assert len(l1.rpc.listfunds()['outputs']) == 3


@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
@unittest.skipIf(not DEVELOPER, "needs to deactivate shadow routing")
def test_htlc_out_timeout(node_factory, bitcoind, executor):
"""Test that we drop onchain if the peer doesn't time out HTLC"""

Expand All @@ -328,7 +328,7 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor):
inv = l2.rpc.invoice(amt, 'test_htlc_out_timeout', 'desc')['bolt11']
assert only_one(l2.rpc.listinvoices('test_htlc_out_timeout')['invoices'])['status'] == 'unpaid'

executor.submit(l1.rpc.pay, inv)
executor.submit(l1.rpc.dev_pay, inv, use_shadow=False)

# l1 will disconnect, and not reconnect.
l1.daemon.wait_for_log('dev_disconnect: @WIRE_REVOKE_AND_ACK')
Expand Down Expand Up @@ -373,7 +373,7 @@ def test_htlc_out_timeout(node_factory, bitcoind, executor):
l2.daemon.wait_for_log('onchaind complete, forgetting peer')


@unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
@unittest.skipIf(not DEVELOPER, "needs to deactivate shadow routing")
def test_htlc_in_timeout(node_factory, bitcoind, executor):
"""Test that we drop onchain if the peer doesn't accept fulfilled HTLC"""

Expand All @@ -395,7 +395,7 @@ def test_htlc_in_timeout(node_factory, bitcoind, executor):
inv = l2.rpc.invoice(amt, 'test_htlc_in_timeout', 'desc')['bolt11']
assert only_one(l2.rpc.listinvoices('test_htlc_in_timeout')['invoices'])['status'] == 'unpaid'

executor.submit(l1.rpc.pay, inv)
executor.submit(l1.rpc.dev_pay, inv, use_shadow=False)

# l1 will disconnect and not reconnect.
l1.daemon.wait_for_log('dev_disconnect: -WIRE_REVOKE_AND_ACK')
Expand Down
Loading