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

channels: fix kick/resubscribe loop caused by flagging #3209

Merged
merged 6 commits into from
Feb 1, 2024

Conversation

arthyn
Copy link
Member

@arthyn arthyn commented Feb 1, 2024

PR Checklist

  • Includes changes to desk files
  • Describes how you tested the PR locally (test ship vs livenet)
  • If a new feature, includes automated tests
  • Comments added anywhere logic may be confusing without context

This fixes LAND-1528 which was causing a kick/resubscribe loop because our scry caused a crash. This changes the scry so that it no-ops if it doesn't find the content and also changes the ca-abed path to no-op if a channel is not found.

Copy link

linear bot commented Feb 1, 2024

@arthyn arthyn changed the base branch from develop to staging February 1, 2024 01:46
@@ -575,6 +575,7 @@
==
++ ca-abed
|= n=nest:c
?. (~(has by v-channels) n) ca-core
Copy link
Member Author

@arthyn arthyn Feb 1, 2024

Choose a reason for hiding this comment

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

@Fang- @midsum-salrux I don't think this is actually good and could end up doing weird things. I tried moving up to line 541 and doing some no-oping there but I'm not sure that's right either.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah this changes the semantics of +ca-abed enough to make a lot of the code here "clearly wrong".

Was gonna say that, what we want instead, is a %u scry for channel existence, but we already have it! Whoever is doing a scry that fails on the got:by here should first be doing the %u version of the scry.

Copy link
Collaborator

@midsum-salrux midsum-salrux left a comment

Choose a reason for hiding this comment

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

LGTM

@arthyn arthyn merged commit b9b6972 into staging Feb 1, 2024
1 check passed
@arthyn arthyn deleted the hm/fix-missing-channel-flagging branch February 1, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants