fix: also parsing calldata of commitBatchesSharedBridge #103
Conversation
| @@ -595,18 +601,27 @@ impl L1Fetcher { | |||
|
|
|||
| pub async fn parse_calldata( | |||
| l1_block_number: u64, | |||
| commit_blocks_fn: &Function, | |||
| commit_candidates: &Vec<Function>, | |||
There was a problem hiding this comment.
| commit_candidates: &Vec<Function>, | |
| commit_candidates: &[Function], |
There was a problem hiding this comment.
ah yes, clippy already told me... :-)
| return Err(ParseError::InvalidCalldata("too short".to_string())); | ||
| } | ||
|
|
||
| let commit_fn = commit_candidates |
There was a problem hiding this comment.
How about using a HashMap mapping the output of the short_signature to the
corresponding function instead?
There was a problem hiding this comment.
IMHO 1 or 2 candidates is too few for anything but an array...
There was a problem hiding this comment.
I guess, but I'm not a big fan of this iterator/find stuff. I'd much rather have
the calldata[..4] be mapped directly via some lookup table
There was a problem hiding this comment.
But the lookup table will have additional allocations (e.g. HashMap has arrays for its buckets, but it has multiple buckets), and constructing a custom structure (i.e. a perfect hash, or just pre-computing the signatures into a second vector) is way too much hassle for something that is far from a demonstrated performance bottleneck...
There was a problem hiding this comment.
I'm thinking this more of as a what would be the ideal should we need to expand upon this in the future, rather than what would result in the least allocations. However, for now, I'm okay with merging this as is.
There was a problem hiding this comment.
Well, the obvious generalization is to get the parsing code from the incoming data, removing the boojum_mode flag - but whether that will be possible for the next expansion remains to be seen... :-)
No description provided.