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

Make gossipd more forgiving of temporary routing failures #886

Merged
merged 2 commits into from
Feb 3, 2018

Conversation

ZmnSCPxj
Copy link
Collaborator

@ZmnSCPxj ZmnSCPxj commented Feb 2, 2018

Fixes: #867

Policy: disable failing channels for 20 seconds, or the next channel_update.

Implementation: Use a u64 unroutable_until, a UNIX-time variable, which prevents a channel from being used for routing if less than the current time in UNIX epoch. Set this to 20+current time if routing failure reported. Set to 0 on construction or channel_update.

@ZmnSCPxj
Copy link
Collaborator Author

ZmnSCPxj commented Feb 2, 2018

Rebased to fix conflict.

@rustyrussell rustyrussell self-requested a review February 2, 2018 23:48
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Great work, I really like this idea, and the implementation is elegant!

Minor nitpick just because I couldn't find anything else to complain about :)

@@ -341,6 +343,12 @@ static void bfg_one_edge(struct node *node, size_t edgenum, double riskfactor)
}
}

/* Determine if the given node_connection is routable */
static bool nc_is_routable(const struct node_connection *nc, u64 now)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use time_t here; it's u64 where it matters and is self-documenting nicely.

/* Call time_now() once at the start, so that our tight loop
* does not keep calling into operating system for the
* current time */
u64 now = time_now().ts.tv_sec;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too....

@@ -1041,15 +1054,17 @@ get_out_node_connection_of(struct routing_state *rstate,
*/
static void routing_failure_on_nc(struct routing_state *rstate,
enum onion_type failcode,
struct node_connection *nc)
struct node_connection *nc,
u64 now)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too...


/* If greater than current time, this connection should not
* be used for routing. */
u64 unroutable_until;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too...

@ZmnSCPxj
Copy link
Collaborator Author

ZmnSCPxj commented Feb 3, 2018

Fixed as per review.

@cdecker
Copy link
Member

cdecker commented Feb 3, 2018

ACK d9e4203

@cdecker cdecker merged commit a57a2dc into ElementsProject:master Feb 3, 2018
@ZmnSCPxj ZmnSCPxj deleted the temporary-ban branch February 4, 2018 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants