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

Use event index from contract instead of local counter #488

Merged
merged 2 commits into from
Jul 22, 2024

Conversation

jannikluhn
Copy link
Contributor

No description provided.

We used to have our own counter for encrypted txs added to the sequencer
contract. Under normal circumstances, this value is the same as the
`txIndex` parameter on the TransactionSubmittedEvent. Using the latter
is more resistant to reorgs, so we use that now.
SELECT event_count FROM transaction_submitted_event_count
WHERE eon = $1
LIMIT 1;
SELECT max(index) + 1 FROM transaction_submitted_event
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be true with the reorg resistance but could probably be false beforehand

Copy link
Contributor

@fredo fredo left a comment

Choose a reason for hiding this comment

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

Left one nitpick but otherwise looks good

We used to explicitly count the number of txs inserted into the db. This
is the value we reset the tx pointer if key generation fails. However,
if for some reason, eg due to a reorg, some tx events are missed, this
value is a bad choice because the other keypers will likely disagree.

Now we use the index of the latest inserted tx instead which is
automatically corrected with every new tx, irrespective of what happened
in the past.
@jannikluhn jannikluhn merged commit 5d83851 into gnosis Jul 22, 2024
7 of 12 checks passed
@jannikluhn jannikluhn deleted the contract-event-index branch August 6, 2024 08:10
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.

None yet

2 participants