Replace blob chain info query usage with blob download #2079
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @andresilva91 and the rest of your teammates on |
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
b17f2e0
to
715ccd6
Compare
graphite-app
bot
requested review from
ma2bd,
jvff,
Twey,
afck,
MathieuDutSik and
christos-h
and removed request for
ma2bd
Graphite Automations"Assign reviewers" took an action on this PR • (05/30/24)6 reviewers were added to this PR based on Andre da Silva's automation. |
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
7832aa3
to
c8a3801
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
715ccd6
to
4551e25
Compare
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
c8a3801
to
143c6a4
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
4551e25
to
df695bc
Compare
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
143c6a4
to
26ffb60
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
df695bc
to
adb92b5
Compare
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
26ffb60
to
19ddf6b
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
adb92b5
to
d00c5ae
Compare
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
19ddf6b
to
cba62d9
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
d00c5ae
to
5166833
Compare
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
cba62d9
to
7a20c90
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
5166833
to
10dbe38
Compare
This was referenced May 31, 2024
ndr-ds
force-pushed
the
05-28-grpc_entrypoint_to_download_a_certificate
branch
from
7a20c90
to
eded29e
Compare
ndr-ds
force-pushed
the
05-29-replace_blob_chain_info_query_usage_with_blob_download
branch
from
10dbe38
to
4e2ecb0
Compare
afck
reviewed
Jun 1, 2024
| .await | ||
| .into_iter() | ||
| .flatten() | ||
| .flatten() |
There was a problem hiding this comment.
Shouldn't we fail if one of the blobs can't be found? (Edit: I see, we already aren't doing that.)
afck
reviewed
Jun 1, 2024
| @@ -71,6 +72,9 @@ pub enum LocalNodeError { | |||
|
|
|||
| #[error("The chain info response received from the local node is invalid")] | |||
| InvalidChainInfoResponse, | |||
|
|
|||
| #[error("Got different blob id from downloaded blob when trying to download {blob_id:?}")] | |||
There was a problem hiding this comment.
Suggested change
| #[error("Got different blob id from downloaded blob when trying to download {blob_id:?}")] | |
| #[error("Got different blob ID from downloaded blob when trying to download {blob_id:?}")] |
afck
reviewed
Jun 4, 2024
| { | ||
| return Some(blob); | ||
| for mut node in validators { | ||
| if let Some(blob) = Self::try_download_blob_from(&mut node, blob_id).await? { |
There was a problem hiding this comment.
We shouldn't fail if one faulty/malicious validator sends us the wrong blob. We should continue trying to download it from someone else.
|
Closing in favor of #2098. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Now that we can download blobs, using the chain info query isn't needed anymore
Proposal
Use the download blob entrypoint instead
Test Plan
CI