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

Adding askrene-disable-channel #7693

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Lagrang3
Copy link
Collaborator

@Lagrang3 Lagrang3 commented Sep 23, 2024

I can think of a way to effectively disable a channel in askrene: by calling askrene-inform-channel and set the maximum_msat to 0. But it seems like a workaround way. I think since we already have an askrene-disable-node command, we should also
support an askrene-disable-channel command as well.

I have also refactored the way channels are marked as disabled.
Previously we had disabled channels by using gossmap_local_updatechan call which would produce a plugin crash
when calling getroutes if we disable a channel that either doesn't exist or it is defined in a layer that we don't use
in that call. Now instead we build a bitmap of disabled channels that we pass on to the MCF solver. The bitmap uses the indexes of the channels relative to the final gossmap, ie. the public gossip plus all localmods coming from the layers.

To be able to write a route_exclusion to a json stream.

Changelog-None.

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Disabled nodes and channels are now saved into a tal_arr of type
strut route_exclution.

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
@Lagrang3 Lagrang3 marked this pull request as ready for review September 25, 2024 07:48
@Lagrang3 Lagrang3 added this to the v24.11 milestone Sep 25, 2024
@Lagrang3 Lagrang3 mentioned this pull request Sep 25, 2024
1 task
@ShahanaFarooqui
Copy link
Collaborator

@Lagrang3 I have updated the third commit (d4cb5b8) for addressing the CI failures in the Pre-build checks, which were caused by missing updates to schema.json and index.rst.

Changelog-EXPERIMENTAL: askrene: add askrene-disable-channel RPC

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Use a bitmap to mark disabled channels instead of tweaking values in
localmods.

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
@rustyrussell
Copy link
Contributor

rustyrussell commented Sep 29, 2024

Definitely need this interface, I agree.

But I prefer to fix the gossmap_update_localchan semantics, otherwise there is a potential hole if we apply a modification and then the channel in the underlying gossip_store is removed: we would have the same crash.

Also, we should not combine nodes and channels in the report output: harsh experience has lead us to now avoid things which don't have simply types (grpc interface has strong types, making this awkward).

@Lagrang3 Lagrang3 marked this pull request as draft September 29, 2024 09:04
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