-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: added get_blob_by_hash #10987
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
base: master
Are you sure you want to change the base?
feat: added get_blob_by_hash #10987
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, now we just need to make this a bit nicer
crates/anvil/src/eth/api.rs
Outdated
pub async fn anvil_get_blob_by_tx_hash( | ||
&self, | ||
hash: B256, | ||
) -> Result<Option<alloy_consensus::TxEip4844WithSidecar>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this we should be able to reuse this logic:
foundry/crates/anvil/src/eth/backend/mem/mod.rs
Lines 2903 to 2909 in d592b3e
let (info, block) = { | |
let storage = self.blockchain.storage.read(); | |
let MinedTransaction { info, block_hash, .. } = | |
storage.transactions.get(&hash)?.clone(); | |
let block = storage.blocks.get(&block_hash).cloned()?; | |
(info, block) | |
}; |
we should add a helper fn for this to Backend as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just used the entire fn..not sure..can you please check te new one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, a few more suggestions
if let Some(sidecar) = typed_tx.sidecar() { | ||
for blob in sidecar.sidecar.clone() { | ||
let versioned_hash = B256::from(blob.to_kzg_versioned_hash()); | ||
if versioned_hash == hash { | ||
return Ok(Some(sidecar.clone())); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can simplify this a bit with:
because we want to avoid cloning all sidecars for this since this is a bit expensive
we also want a new helper blob_by_versioned_hash
on the sidecar types in alloy, do you want to upstream this?
/// Handler for RPC call: `anvil_getBlobByTransactionHash` | ||
pub fn anvil_get_blob_by_tx_hash( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be getBlobs
let tx = self.mined_transaction_by_hash(hash).unwrap(); | ||
let typed_tx = TypedTransaction::try_from(tx).unwrap(); | ||
if let Some(sidecar) = typed_tx.sidecar() { | ||
return Ok(Some(sidecar.clone())); | ||
} | ||
Ok(None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can fail, so we should handle this gracefully and only return the blobs Vec here
closes #10978