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

(OTS) Mempool GC Does Not Correctly Filter by Timestamp #879

Open
l-monninger opened this issue Nov 20, 2024 · 4 comments
Open

(OTS) Mempool GC Does Not Correctly Filter by Timestamp #879

l-monninger opened this issue Nov 20, 2024 · 4 comments
Assignees
Labels
Source: Audit Issues discovered by audit. suzuka:safety

Comments

@l-monninger
Copy link
Collaborator

Summary

Mempool GC does not correctly filter out transactions by timestamp. Each entry key has a specific format: {priority}:{timestamp}:{seqnum}:{id}which means that the key passed to the upper_bound call here does not make sense since it is formatted as {timestamp_threshold}: here. Which, instead of filtering out transactions based on timestamp will end filtering out transactions with priority<=timestamp_threshold.

@l-monninger
Copy link
Collaborator Author

@mzabaluev I think this may have resulted from #749 and #628 being in-flight at the same time.

@l-monninger
Copy link
Collaborator Author

@khokho

@mzabaluev
Copy link
Collaborator

@mzabaluev I think this may have resulted from #749 and #628 being in-flight at the same time.

Yes, I suppose this is exactly what happened.

I see the following alternative ways to remedy this:

  • Erase the mempool priority altogether.
  • Don't rely on global range indexing to implement GC.
    • A brute-force pass through all entries is the simplest solution, but may be inefficient if the number of pooled transactions is large.
    • What we can try to do instead is the combination of a cursor pass enumerating priority buckets as the outer loop, and prefix range sweeps for timestamp < timestamp_threshold in each priority bucket.

@mzabaluev
Copy link
Collaborator

And another one:

  • Redesign the database so that there is the column family with timestamp as the top-level key, and another one with the total ordering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Source: Audit Issues discovered by audit. suzuka:safety
Projects
None yet
Development

No branches or pull requests

2 participants