-
Notifications
You must be signed in to change notification settings - Fork 21
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
Decode transaction body #76
Conversation
lib/ethers/transaction.ex
Outdated
tx_body = | ||
%{ | ||
type: tx_type, | ||
chain_id: decode_key(tx, "chainId"), |
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.
Maybe I miss it, are we missing value
field? :P
lib/ethers/transaction.ex
Outdated
@@ -210,4 +261,13 @@ defmodule Ethers.Transaction do | |||
|
|||
defp trim_leading(<<0, rest::binary>>), do: trim_leading(rest) | |||
defp trim_leading(<<bin::binary>>), do: bin | |||
|
|||
defp decode_key(tx, key) do | |||
decoded_value = Map.get(tx, key) |> Ethers.Utils.hex_to_integer() |
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.
Guess this may already be thought about. Would it be possible that key has value nil
, will the next_to_integer()
throw error in this case
nice catch @lishawnl The response after decoding should look like this now
|
lib/ethers/transaction.ex
Outdated
%{ | ||
type: tx_type, | ||
chain_id: decode_key(tx, "chainId"), | ||
nonce: decode_key(tx, "nonce"), |
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.
One thing that I forgot is that all values in this Transaction
struct is expected to by either a binary or a hex encoded binary (0x..
) as stated in the type spec . Decoding those values to integers will break that contract!
I see two possible solutions here:
- Use raw binary values to construct the
Transaction
struct and have another function (e.g.expand_values
) which actually decodes these values to integers. - Move this logic into
Ethers.post_process
and return decoded maps instead of aTransaction
struct.
lib/ethers/transaction.ex
Outdated
hash: binary() | nil, | ||
input: binary() | nil, | ||
transaction_index: binary() | nil, | ||
v: binary() | nil, |
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 believe v
== signature_recovery_id
. Can you confirm this and if so, we can just use signature_recovery_id
for consistency.
424d1da
to
f804a45
Compare
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.
Thanks for the updates 💟
d12589e
to
83d526d
Compare
Per the conversation
Decode the transaction body, need a bit more discussion.