Skip to content

Conversation

@pxrl
Copy link
Collaborator

@pxrl pxrl commented Aug 5, 2025

There's no need to filter based on the complete set of unique attributes -
in practice it's more desirable to cast the net wide and discard anything
matching the re-orged blockHash.

nb. This change only discards events by blockHash before they've been
read into SpokePoolClient state by SpokePoolClient.update(). At that
point the blockHash is discarded so it's no longer possible to identify
the event by its blockHash; this needs a separate solution.

pxrl added 3 commits August 5, 2025 19:53
In order to make a subsequent refactoring diff smaller.
There's no need to filter based on the complete set of unique attributes
- in practice it's more desirable to cast the net wide and discard
  anything matching the re-orged blockHash.
Base automatically changed from pxrl/refactor1 to master August 6, 2025 10:00
}
let removedEventIdx: number;
do {
removedEventIdx = pendingEvents.findIndex(({ blockHash }) => blockHash === event.blockHash);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this require the event which was reorged to be reincluded in this block? What if the event was removed by a reorg and not mined again yet?

Copy link
Collaborator Author

@pxrl pxrl Aug 12, 2025

Choose a reason for hiding this comment

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

Yeah; these dropped events are currently reported per-event over the event subscription. Per current implementation we require one notification per event, per blockHash. The change implemented here is to permit a single removed event notification to drop all stored events for a given blockHash, so we basically cast the net a little wider, which is beneficial in case one of the removed event notifications doesn't materialise (for whatever reason).

There's a follow-on change required which will actually track block hashes and reconcile when a re-org happens by finding a common ancestor and invalidating everything inbetween. That's still pending.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Makes sense.

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.

4 participants