fix: silently drop invalid private messages instead of crashing PXE sync#21273
Closed
nchamo wants to merge 3 commits into
Closed
fix: silently drop invalid private messages instead of crashing PXE sync#21273nchamo wants to merge 3 commits into
nchamo wants to merge 3 commits into
Conversation
Contributor
|
Sir, I already have PR for this open here and you reviewed it last week 😆 I am just waiting there for Nico's review here. Weird that the issue there didn't get linked given that it's in the PR description. I guess it explicitly needs to mention "Fixes" or "Closes". I also didn't assign myself the issue there as it was only tackling the TS side. This is really a bad coincidence. Will be more careful with that next time. Given that my PR there already went through the first round of review and I have there some other useful code but you also added a test here which seems valuable I propose that:
Sounds good? |
Contributor
Author
|
Lol haha, my bad. My context is cleared on weekends 😅 Sounds good 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
We need to be careful when handling messages and, in this particular task, we looked into event and note messages. Contracts can create invalid messages that don't have a matching note/event, so we need to make sure we can handle them gracefully. Notes already apply a filter with
nonce_discovery, but events don't. We were able to build an E2E test to trigger a scenario for events that crashed PXE's syncNow, on both the notes and event service, we will gracefully handle an invalid notes instead of throwing an error
Fixes F-343
Fixes F-361