Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

perf: early return in EthCodec derive #2314

Merged
merged 2 commits into from
Apr 1, 2023

Conversation

DaniPopes
Copy link
Collaborator

@DaniPopes DaniPopes commented Mar 29, 2023

Motivation

Currently we go straight into decoding which can become expensive when called continuously from a big enum, eg foundry HEVM or HardhatConsole. We have the ParamType to decode so we can calculate very cheaply a minimum bound in bytes to check against

Solution

  • Add a minimum bound check for the given bytes to early return for in derive(EthCodec)
  • (drive by) Fix tuple implementations of Tokenizable
    • these would panic if given a Token::Tuple of the wrong length
    • also we have ownership of the Vec so into_iter is better than drain

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

variant.span(),
"EthAbiType cannot be derived for enum variants with multiple fields",
))
}
Copy link
Collaborator Author

@DaniPopes DaniPopes Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was written weirdly, with also an unreachable block (fields.len() == 1 guarantees fields.iter().next() is Some, but an else block was present)

@@ -205,45 +201,44 @@ fn tokenize_enum<'a>(

let mut into_tokens = TokenStream::new();
let mut from_tokens = TokenStream::new();
for (idx, variant) in variants.into_iter().enumerate() {
let last = variants.size_hint().1.unwrap().saturating_sub(1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consume in the last call instead of cloning

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gakonst gakonst merged commit 6e648d5 into gakonst:master Apr 1, 2023
@DaniPopes DaniPopes deleted the perf/codec-derive branch April 1, 2023 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants