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

Event decoding for unnamed parameters is wrong #206

Open
vkgnosis opened this issue Mar 4, 2021 · 3 comments
Open

Event decoding for unnamed parameters is wrong #206

vkgnosis opened this issue Mar 4, 2021 · 3 comments

Comments

@vkgnosis
Copy link
Member

vkgnosis commented Mar 4, 2021

A solidity event like

event MyEvent(uint8, uint16, uint32, uint64, uint128);

is compiled into an artifact that has a list with these parameters with each one having an empty "" name. In ethabi's event decoding code Event::parse_log parameters are stored in a hash map of event name to token. This leads to every parameter using a clone of the same Token. In this particular case it can cause for example the uint8 to have a value that is larger than 255 because it uses the same token as the uint128.

@mattsse
Copy link
Contributor

mattsse commented Apr 1, 2021

This line will cause the problem for indexed event parameters. Also this chains (topics_named_tokens.chain(data_named_tokens)) first named and then data tokens, which caused issues when I implemented some derive macros, in ethers-rs, what I did instead was:
filter the params in indexed and not indexed, then decode the all, but then put them in the correct order. The way this can work is:

let mut tokens = Vec::with_capacity(topic_tokens.len() + data_tokens.len());
for input in &event.inputs {
  if input.indexed {
      tokens.push(topic_tokens.remove(0));
 } else {
      tokens.push(data_tokens.remove(0));
 }
}

I could fix this by copying this to Event::parse_log

@gakonst
Copy link
Contributor

gakonst commented Feb 6, 2022

@vkgnosis @nlordell are there any plans on fixing this? we hit that in Foundry as seen above recently

@vkgnosis
Copy link
Member Author

vkgnosis commented Feb 7, 2022

I don't remember the details of this after I reported it a year ago. We'd surely accept a PR that fixed this but no one is actively working on it.

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

No branches or pull requests

3 participants