-
Notifications
You must be signed in to change notification settings - Fork 43
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
Handle Events with same topic hash properly #115
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.
Looking good. a couple of comments
with {:ok, method_id, _rest} <- Util.split_method_id(topic1), | ||
{:ok, selector} when not is_nil(selector) <- | ||
Util.find_selector_by_method_id(function_selectors, method_id) do |
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 think this function can be removed
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 function is still used when we call functions in the ABI
iex> File.read!("priv/dog.abi.json")
...> |> Jason.decode!
...> |> ABI.parse_specification
...> |> ABI.find_and_decode("b85d0bd200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001" |> Base.decode16!(case: :lower))
{%ABI.FunctionSelector{type: :function, function: "bark", input_names: ["at", "loudly"], method_id: <<184, 93, 11, 210>>, returns: [], types: [:address, :bool], state_mutability: :non_payable}, [<<0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1>>, true]}```
lib/abi/util.ex
Outdated
@@ -21,4 +21,24 @@ defmodule ABI.Util do | |||
{:error, :no_matching_function} | |||
end | |||
end | |||
|
|||
def find_selector_by_event_id(function_selectors, method_id_target, topics_length) do |
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.
let's add a couple of tests for this function
lib/abi/event.ex
Outdated
@@ -67,11 +67,12 @@ defmodule ABI.Event do | |||
@spec find_and_decode([FunctionSelector.t()], topic, topic, topic, topic, binary) :: | |||
{FunctionSelector.t(), [event_value]} | {:error, any} | |||
def find_and_decode(function_selectors, topic1, topic2, topic3, topic4, data) do | |||
input_topics = [topic2, topic3, topic4] | |||
topics_length = Enum.count(input_topics, fn x -> x != nil end) |
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.
topics_length
is only used in the find_selector_by_event_id
. Let's pass input_topics as the third parameter into find_selector_by_event_id
and count there
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.
done
lib/abi/util.ex
Outdated
Enum.find(function_selectors, fn | ||
%{type: :event, method_id: ^method_id_target, inputs_indexed: inputs_indexed} -> | ||
# match the length of topics and indexed_args_length | ||
topics_length = Enum.count(input_topics, fn x -> x != nil end) |
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.
you're calculating topics_length
for every function selector.
Let's move it outside of Enum.find
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.
done
Thank you! |
I added more context regarding events that have same topic but different indexed arguments here
This change allows the library to handle Transfer event that has multiple variants of arguments properly.
Transfer(address from, address to, uint256 tokenId)
Transfer(address indexed from, address indexed to, uint256 indexed tokenId)
Transfer(address indexed from, address indexed to, uint256 tokenId)
This change is creating a different function for an Event to check that number of Topics should be equal to the length of the indexed arguments.