mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2025-02-20 13:54:36 +01:00
Code Issues Projects Releases Wiki Activity

gossipd: fix routing issue.

Browse source 
I had a routing problem, and wrote a simple unit test which passed.  So
I wrote one which copied the failure case (and importantly, had a non-1
fee factor), which triggerd it.

In that real example, we underflowed which resulted in us not finding
a route.  Simply don't consider routes which are infinite.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2017-08-31 11:38:12 +09:30 • committed by Christian Decker
parent 8a829ba9cb
commit c8aa50a382
5 changed files with 290 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

35
gossipd/routing.c
View file

@ -336,11 +336,21 @@ static void bfg_one_edge(struct node *node, size_t edgenum, double riskfactor)
assert(c->dst == node);
for (h = 0; h < ROUTING_MAX_HOPS; h++) {
/* FIXME: Bias against smaller channels. */
s64 fee = connection_fee(c, node->bfg[h].total);
u64 risk = node->bfg[h].risk + risk_fee(node->bfg[h].total + fee,
c->delay, riskfactor);
s64 fee;
u64 risk;
if (node->bfg[h].total == INFINITE)
continue;
fee = connection_fee(c, node->bfg[h].total);
risk = node->bfg[h].risk + risk_fee(node->bfg[h].total + fee,
c->delay, riskfactor);
if (node->bfg[h].total + (s64)fee + (s64)risk
< c->src->bfg[h+1].total + (s64)c->src->bfg[h+1].risk) {
status_trace("...%s can reach here in hoplen %zu total %"PRIu64,
type_to_string(trc, struct pubkey,
&c->src->id),
h, node->bfg[h].total + fee);
c->src->bfg[h+1].total = node->bfg[h].total + fee;
c->src->bfg[h+1].risk = risk;
c->src->bfg[h+1].prev = c;
@ -393,19 +403,16 @@ find_route(const tal_t *ctx, struct routing_state *rstate,
n = node_map_next(rstate->nodes, &it)) {
size_t num_edges = tal_count(n->in);
for (i = 0; i < num_edges; i++) {
if (!n->in[i]->active)
status_trace("Node %s edge %i/%zu",
type_to_string(trc, struct pubkey,
&n->id),
i, num_edges);
if (!n->in[i]->active) {
status_trace("...inactive");
continue;
}
bfg_one_edge(n, i, riskfactor);
status_trace("We seek %p->%p, this is %p -> %p",
dst, src,
n->in[i]->src, n->in[i]->dst);
status_trace("Checking from %s to %s",
type_to_string(trc,
struct pubkey,
&n->in[i]->src->id),
type_to_string(trc,
struct pubkey,
&n->in[i]->dst->id));
status_trace("...done");
}
}
}

1
gossipd/test/.gitignore vendored Normal file
View file

@ -0,0 +1 @@
run-find_route

25
gossipd/test/Makefile Normal file
View file

@ -0,0 +1,25 @@
check: gossipd-tests
# Note that these actually #include everything they need, except ccan/ and bitcoin/.
# That allows for unit testing of statics, and special effects.
GOSSIPD_TEST_SRC := $(wildcard gossipd/test/run-*.c)
GOSSIPD_TEST_OBJS := $(GOSSIPD_TEST_SRC:.c=.o)
GOSSIPD_TEST_PROGRAMS := $(GOSSIPD_TEST_OBJS:.o=)
GOSSIPD_TEST_COMMON_OBJS := \
common/pseudorand.o \
common/type_to_string.o \
common/utils.o
update-mocks: $(GOSSIPD_TEST_SRC:%=update-mocks/%)
$(GOSSIPD_TEST_PROGRAMS): $(GOSSIPD_TEST_COMMON_OBJS) $(BITCOIN_OBJS)
# Test objects require source to be generated, since they include ..
$(GOSSIPD_TEST_OBJS): $(GOSSIPD_GEN_SRC) $(LIGHTNINGD_GOSSIP_SRC)
ALL_OBJS += $(GOSSIPD_TEST_OBJS)
ALL_TEST_PROGRAMS += $(GOSSIPD_TEST_PROGRAMS)
gossipd-tests: $(GOSSIPD_TEST_PROGRAMS:%=unittest/%)

124
gossipd/test/run-find_route-specific.c Normal file
View file

@ -0,0 +1,124 @@
/* We can't seem to route the following:
*
* Expect route 03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf -> 0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae -> 02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06
*
* getchannels:
* {'channels': [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]}
*/
#include <common/status.h>
#include <stdio.h>
#define status_trace(fmt, ...) \
do { printf((fmt) ,##__VA_ARGS__); printf("\n"); } while(0)
#include "../routing.c"
struct broadcast_state *new_broadcast_state(tal_t *ctx UNNEEDED)
{
return NULL;
}
/* AUTOGENERATED MOCKS START */
/* Generated stub for fromwire_channel_announcement */
bool fromwire_channel_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *node_signature_1 UNNEEDED, secp256k1_ecdsa_signature *node_signature_2 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_1 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_2 UNNEEDED, u8 **features UNNEEDED, struct sha256_double *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *node_id_1 UNNEEDED, struct pubkey *node_id_2 UNNEEDED, struct pubkey *bitcoin_key_1 UNNEEDED, struct pubkey *bitcoin_key_2 UNNEEDED)
{ fprintf(stderr, "fromwire_channel_announcement called!\n"); abort(); }
/* Generated stub for fromwire_channel_update */
bool fromwire_channel_update(const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct sha256_double *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u16 *flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED)
{ fprintf(stderr, "fromwire_channel_update called!\n"); abort(); }
/* Generated stub for fromwire_ipaddr */
void fromwire_ipaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct ipaddr *addr UNNEEDED)
{ fprintf(stderr, "fromwire_ipaddr called!\n"); abort(); }
/* Generated stub for fromwire_node_announcement */
bool fromwire_node_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, u8 **features UNNEEDED, u32 *timestamp UNNEEDED, struct pubkey *node_id UNNEEDED, u8 rgb_color[3] UNNEEDED, u8 alias[32] UNNEEDED, u8 **addresses UNNEEDED)
{ fprintf(stderr, "fromwire_node_announcement called!\n"); abort(); }
/* Generated stub for queue_broadcast */
void queue_broadcast(struct broadcast_state *bstate UNNEEDED,
const int type UNNEEDED,
const u8 *tag UNNEEDED,
const u8 *payload UNNEEDED)
{ fprintf(stderr, "queue_broadcast called!\n"); abort(); }
/* Generated stub for towire_pubkey */
void towire_pubkey(u8 **pptr UNNEEDED, const struct pubkey *pubkey UNNEEDED)
{ fprintf(stderr, "towire_pubkey called!\n"); abort(); }
/* Generated stub for towire_short_channel_id */
void towire_short_channel_id(u8 **pptr UNNEEDED,
const struct short_channel_id *short_channel_id UNNEEDED)
{ fprintf(stderr, "towire_short_channel_id called!\n"); abort(); }
/* Generated stub for towire_u16 */
void towire_u16(u8 **pptr UNNEEDED, u16 v UNNEEDED)
{ fprintf(stderr, "towire_u16 called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */
const void *trc;
int main(void)
{
static const struct sha256_double zerohash;
const tal_t *ctx = trc = tal_tmpctx(NULL);
struct node_connection *nc;
struct routing_state *rstate;
struct pubkey a, b, c;
s64 fee;
struct node_connection **route;
secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
| SECP256K1_CONTEXT_SIGN);
rstate = new_routing_state(ctx, &zerohash);
pubkey_from_hexstr("03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf",
strlen("03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf"),
&a);
pubkey_from_hexstr("0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae",
strlen("0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae"),
&b);
pubkey_from_hexstr("02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06",
strlen("02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06"),
&c);
/* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */
nc = get_or_make_connection(rstate, &c, &b);
nc->active = true;
nc->base_fee = 0;
nc->proportional_fee = 10;
nc->delay = 5;
nc->flags = 1;
nc->last_timestamp = 1504064344;
/* {'active': True, 'short_id': '6989:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */
nc = get_or_make_connection(rstate, &b, &a);
nc->active = true;
nc->base_fee = 0;
nc->proportional_fee = 10;
nc->delay = 5;
nc->flags = 0;
nc->last_timestamp = 1504064344;
/* {'active': True, 'short_id': '6990:2:1/0', 'fee_per_kw': 10, 'delay': 5, 'flags': 0, 'destination': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'source': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'last_update': 1504064344}, */
nc = get_or_make_connection(rstate, &b, &c);
nc->active = true;
nc->base_fee = 0;
nc->proportional_fee = 10;
nc->delay = 5;
nc->flags = 0;
nc->last_timestamp = 1504064344;
/* {'active': True, 'short_id': '6989:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '03c173897878996287a8100469f954dd820fcd8941daed91c327f168f3329be0bf', 'last_update': 1504064344}]} */
nc = get_or_make_connection(rstate, &a, &b);
nc->active = true;
nc->base_fee = 0;
nc->proportional_fee = 10;
nc->delay = 5;
nc->flags = 1;
nc->last_timestamp = 1504064344;
nc = find_route(ctx, rstate, &a, &c, 100000, 1.0, &fee, &route);
assert(nc);
assert(tal_count(route) == 1);
assert(pubkey_eq(&route[0]->src->id, &b));
assert(pubkey_eq(&route[0]->dst->id, &c));
tal_free(ctx);
return 0;
}

119
gossipd/test/run-find_route.c Normal file
View file

@ -0,0 +1,119 @@
#include <common/status.h>
#include <stdio.h>
#define status_trace(fmt, ...) \
do { printf((fmt) ,##__VA_ARGS__); printf("\n"); } while(0)
#include "../routing.c"
struct broadcast_state *new_broadcast_state(tal_t *ctx UNNEEDED)
{
return NULL;
}
/* AUTOGENERATED MOCKS START */
/* Generated stub for fromwire_channel_announcement */
bool fromwire_channel_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *node_signature_1 UNNEEDED, secp256k1_ecdsa_signature *node_signature_2 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_1 UNNEEDED, secp256k1_ecdsa_signature *bitcoin_signature_2 UNNEEDED, u8 **features UNNEEDED, struct sha256_double *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct pubkey *node_id_1 UNNEEDED, struct pubkey *node_id_2 UNNEEDED, struct pubkey *bitcoin_key_1 UNNEEDED, struct pubkey *bitcoin_key_2 UNNEEDED)
{ fprintf(stderr, "fromwire_channel_announcement called!\n"); abort(); }
/* Generated stub for fromwire_channel_update */
bool fromwire_channel_update(const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, struct sha256_double *chain_hash UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, u32 *timestamp UNNEEDED, u16 *flags UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, u64 *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED)
{ fprintf(stderr, "fromwire_channel_update called!\n"); abort(); }
/* Generated stub for fromwire_ipaddr */
void fromwire_ipaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct ipaddr *addr UNNEEDED)
{ fprintf(stderr, "fromwire_ipaddr called!\n"); abort(); }
/* Generated stub for fromwire_node_announcement */
bool fromwire_node_announcement(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, size_t *plen UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED, u8 **features UNNEEDED, u32 *timestamp UNNEEDED, struct pubkey *node_id UNNEEDED, u8 rgb_color[3] UNNEEDED, u8 alias[32] UNNEEDED, u8 **addresses UNNEEDED)
{ fprintf(stderr, "fromwire_node_announcement called!\n"); abort(); }
/* Generated stub for queue_broadcast */
void queue_broadcast(struct broadcast_state *bstate UNNEEDED,
const int type UNNEEDED,
const u8 *tag UNNEEDED,
const u8 *payload UNNEEDED)
{ fprintf(stderr, "queue_broadcast called!\n"); abort(); }
/* Generated stub for towire_pubkey */
void towire_pubkey(u8 **pptr UNNEEDED, const struct pubkey *pubkey UNNEEDED)
{ fprintf(stderr, "towire_pubkey called!\n"); abort(); }
/* Generated stub for towire_short_channel_id */
void towire_short_channel_id(u8 **pptr UNNEEDED,
const struct short_channel_id *short_channel_id UNNEEDED)
{ fprintf(stderr, "towire_short_channel_id called!\n"); abort(); }
/* Generated stub for towire_u16 */
void towire_u16(u8 **pptr UNNEEDED, u16 v UNNEEDED)
{ fprintf(stderr, "towire_u16 called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */
const void *trc;
int main(void)
{
static const struct sha256_double zerohash;
const tal_t *ctx = trc = tal_tmpctx(NULL);
struct node_connection *nc;
struct routing_state *rstate;
struct pubkey a, b, c, d;
struct privkey tmp;
s64 fee;
struct node_connection **route;
secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
| SECP256K1_CONTEXT_SIGN);
rstate = new_routing_state(ctx, &zerohash);
memset(&tmp, 'a', sizeof(tmp));
pubkey_from_privkey(&tmp, &a);
add_node(rstate, &a);
memset(&tmp, 'b', sizeof(tmp));
pubkey_from_privkey(&tmp, &b);
add_node(rstate, &b);
/* A<->B */
add_connection(rstate, &a, &b, 1, 1, 1, 1);
nc = find_route(ctx, rstate, &a, &b, 1000, 1.0, &fee, &route);
assert(nc);
assert(tal_count(route) == 0);
assert(fee == 0);
/* A<->B<->C */
memset(&tmp, 'c', sizeof(tmp));
pubkey_from_privkey(&tmp, &c);
add_node(rstate, &c);
status_trace("A = %s", type_to_string(trc, struct pubkey, &a));
status_trace("B = %s", type_to_string(trc, struct pubkey, &b));
status_trace("C = %s", type_to_string(trc, struct pubkey, &c));
add_connection(rstate, &b, &c, 1, 1, 1, 1);
nc = find_route(ctx, rstate, &a, &c, 1000, 1.0, &fee, &route);
assert(nc);
assert(tal_count(route) == 1);
assert(fee == 1);
/* A<->D<->C: Lower base, higher percentage. */
memset(&tmp, 'd', sizeof(tmp));
pubkey_from_privkey(&tmp, &d);
add_node(rstate, &d);
status_trace("D = %s", type_to_string(trc, struct pubkey, &d));
add_connection(rstate, &a, &d, 0, 2, 1, 1);
add_connection(rstate, &d, &c, 0, 2, 1, 1);
/* Will go via D for small amounts. */
nc = find_route(ctx, rstate, &a, &c, 1000, 1.0, &fee, &route);
assert(nc);
assert(tal_count(route) == 1);
assert(pubkey_eq(&route[0]->src->id, &d));
assert(fee == 0);
/* Will go via B for large amounts. */
nc = find_route(ctx, rstate, &a, &c, 1000000, 1.0, &fee, &route);
assert(nc);
assert(tal_count(route) == 1);
assert(pubkey_eq(&route[0]->src->id, &b));
assert(fee == 2);
tal_free(ctx);
return 0;
}