Check event is not rejected

[S7evinK]

Companion PR to matrix-org/gomatrixserverlib#421

[codecov]

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (1b124fe) 65.14% compared to head (013b684) 65.13%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3243      +/-   ##
==========================================
- Coverage   65.14%   65.13%   -0.02%     
==========================================
  Files         507      507              
  Lines       57173    57199      +26     
==========================================
+ Hits        37246    37254       +8     
- Misses      16027    16045      +18     
  Partials     3900     3900              

Flag	Coverage Δ
unittests	49.47% <19.23%> (-0.03%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
roomserver/storage/shared/room_updater.go	61.53% <100.00%> (+0.60%)	⬆️
roomserver/state/state.go	69.67% <50.00%> (-0.14%)	⬇️
roomserver/internal/input/input_missing.go	64.93% <0.00%> (-0.54%)	⬇️
roomserver/internal/query/query.go	58.16% <0.00%> (-0.53%)	⬇️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kegsay]

I have concerns here that because we do not recalculate rejected-ness, we may cascade fail later events incorrectly.

A reminder of what sets rejected = true:

• Passes authorization rules based on the event’s auth events, otherwise it is rejected.
• Passes authorization rules based on the state before the event, otherwise it is rejected.

Rejected-ness is not a pure yes/no (unlike say hash checks) because the server may fail to fetch the auth events / state before the event due to connectivity issues. At that point, the event is neither accepted nor rejected, it's indetermined.

It's unclear what the best course of action is when there are network connectivity issues as this can become an attack vector. It's clearly not correct to accept it (else an adversary in control of the network can just firewall off the server and send rubbish to it, and have the server merrily accept it). The opposite is a denial of service attack though: valid events can be made invalid by preventing the server talking to other servers. This isn't the only place where this can happen in federation today (/gme dishing up prev_events is another) so it's less bad I guess, but we definitely do need to keep retrying to fetch the events if we cannot.