Skip to content

Commit e89b21c

Browse files
committed
chanbackup: unbias our random peer selection.
We picked one node and iterated. This means we would only sent to 1, not 2 nodes if we picked the last. But it also means we would generally send to the same pair of nodes, which isn't great for redundancy. Rework to be more random. Signed-off-by: Rusty Russell <[email protected]>
1 parent 1381557 commit e89b21c

File tree

1 file changed

+40
-12
lines changed

1 file changed

+40
-12
lines changed

Diff for: plugins/chanbackup.c

+40-12
Original file line numberDiff line numberDiff line change
@@ -551,13 +551,47 @@ static struct command_result *after_send_scb_single_fail(struct command *cmd,
551551
return notification_or_hook_done(cmd);
552552
}
553553

554+
static bool already_have_node_id(const struct node_id *ids,
555+
const struct node_id *id)
556+
{
557+
for (size_t i = 0; i < tal_count(ids); i++)
558+
if (node_id_eq(&ids[i], id))
559+
return true;
560+
561+
return false;
562+
}
563+
564+
static const struct node_id *random_peers(const tal_t *ctx,
565+
const struct chanbackup *cb)
566+
{
567+
struct peer_map_iter it;
568+
const struct node_id *peer;
569+
struct node_id *ids = tal_arr(ctx, struct node_id, 0);
570+
571+
/* Simple case: pick all of them */
572+
if (peer_map_count(cb->peers) <= NUM_BACKUP_PEERS) {
573+
for (peer = peer_map_first(cb->peers, &it);
574+
peer;
575+
peer = peer_map_next(cb->peers, &it)) {
576+
tal_arr_expand(&ids, *peer);
577+
}
578+
} else {
579+
while (peer_map_count(cb->peers) < NUM_BACKUP_PEERS) {
580+
peer = peer_map_pick(cb->peers, pseudorand_u64(), &it);
581+
if (already_have_node_id(ids, peer))
582+
continue;
583+
tal_arr_expand(&ids, *peer);
584+
}
585+
}
586+
return ids;
587+
}
588+
554589
static struct command_result *send_to_peers(struct command *cmd)
555590
{
556591
struct out_req *req;
557592
size_t *idx = tal(cmd, size_t);
558593
u8 *serialise_scb, *data;
559-
struct peer_map_iter it;
560-
const struct node_id *peer;
594+
const struct node_id *peers;
561595
struct chanbackup *cb = chanbackup(cmd->plugin);
562596

563597
if (!cb->send_our_peer_backup)
@@ -582,14 +616,13 @@ static struct command_result *send_to_peers(struct command *cmd)
582616
}
583617
serialise_scb = towire_peer_storage(cmd, data);
584618

585-
*idx = 0;
586-
for (peer = peer_map_pick(cb->peers, pseudorand_u64(), &it);
587-
peer;
588-
peer = peer_map_next(cb->peers, &it)) {
619+
peers = random_peers(tmpctx, cb);
620+
*idx = tal_count(peers);
621+
for (size_t i = 0; i < tal_count(peers); i++) {
589622
struct info *info = tal(cmd, struct info);
590623

591624
info->idx = idx;
592-
info->node_id = *peer;
625+
info->node_id = peers[i];
593626

594627
req = jsonrpc_request_start(cmd,
595628
"sendcustommsg",
@@ -599,12 +632,7 @@ static struct command_result *send_to_peers(struct command *cmd)
599632

600633
json_add_node_id(req->js, "node_id", &info->node_id);
601634
json_add_hex_talarr(req->js, "msg", serialise_scb);
602-
(*info->idx)++;
603635
send_outreq(req);
604-
605-
/* Only send to NUM_BACKUP_PEERS for each update */
606-
if (*info->idx == NUM_BACKUP_PEERS)
607-
break;
608636
}
609637

610638
if (*idx == 0)

0 commit comments

Comments
 (0)