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

Remove blobs cache #2784

Merged
merged 1 commit into from
Nov 2, 2024
Merged

Remove blobs cache #2784

merged 1 commit into from
Nov 2, 2024

Conversation

ndr-ds
Copy link
Contributor

@ndr-ds ndr-ds commented Nov 1, 2024

Motivation

This cache isn't needed, and actually was incorrect in how we were using it: the cache wrongfully technically gave access to unpublished blobs to any chain in the client, as well as masked a bug (we needed to be checking the chain manager's pending blobs in get_blobs also).

Proposal

Remove the cache

Test Plan

Existing ts in CI should pass and confirm that this indeed wasn't needed.

Release Plan

I feel like this should probably be backported everywhere?

  • These changes should be backported to the latest devnet branch
  • These changes should be backported to the latest testnet branch

Copy link
Contributor Author

ndr-ds commented Nov 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @andresilva91 and the rest of your teammates on Graphite Graphite

@ndr-ds ndr-ds marked this pull request as ready for review November 1, 2024 23:33
@ndr-ds ndr-ds requested review from afck, deuszx, jvff and ma2bd November 1, 2024 23:33
Copy link
Contributor Author

ndr-ds commented Nov 2, 2024

Merge activity

  • Nov 2, 7:17 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Nov 2, 7:17 AM EDT: A user merged this pull request with Graphite.

@ndr-ds ndr-ds merged commit e3736e6 into main Nov 2, 2024
5 checks passed
@ndr-ds ndr-ds deleted the 11-01-remove_blobs_cache branch November 2, 2024 11:17
@ndr-ds ndr-ds linked an issue Nov 4, 2024 that may be closed by this pull request
afck pushed a commit to afck/linera-protocol that referenced this pull request Nov 11, 2024
This cache isn't needed, and actually was incorrect in how we were using it: the cache wrongfully technically gave access to unpublished blobs to any chain in the client, as well as masked a bug (we needed to be checking the chain manager's pending blobs in `get_blobs` also).

Remove the cache

Existing ts in CI should pass and confirm that this indeed wasn't needed.

I feel like this should probably be backported everywhere?

- These changes should be backported to the latest `devnet` branch
- These changes should be backported to the latest `testnet` branch
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.

Fix storage DOS attack in block proposals
2 participants