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

ex_abi can't decode string type #15

Closed
ayrat555 opened this issue Mar 12, 2019 · 8 comments
Closed

ex_abi can't decode string type #15

ayrat555 opened this issue Mar 12, 2019 · 8 comments

Comments

@ayrat555
Copy link
Collaborator

The issue is related to the latest PR (#14). Now data byte length is not considered for string type (maybe for some other dynamic types)

@volnyvolnyvolny
Copy link

@ayrat555, could you please clarify your statement?

Maybe I missed something, but I see nothing criminal in the decoding of the string type. I hoped that random testing would show that when decoding some previously encoded values it will not match the original but had no luck in this direction (PR #16).

@ayrat555
Copy link
Collaborator Author

@zergera try setting the latest version of ex_abi to blockscout. you'll a bunch of tests failing. they're related to string encoding

@volnyvolnyvolny
Copy link

@ayrat555, yes, setting version of ex_abi to 0.2.0 produces tests failings. What I see at the moment is that in all the failed tests there is exactly one pattern: you expect [string] type, but compare it to the value of [(string)].

For ex.:

#ethereum_jsonrpc
test "correctly decodes string types" do
  result =
    "0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000441494f4e00000000000000000000000000000000000000000000000000000000"

# either use:
# result =
  # "0x000000000000000000000000000000000000000000000000000000000000000441494f4e00000000000000000000000000000000000000000000000000000000"

  selector =
    %ABI.FunctionSelector{
        function: "name",
        types: [],
        returns: [:string]

        # or:
        # returns: [{:tuple, [:string]}]
    }

  assert Encoder.decode_result(%{id: "storedName", result: result}, selector)
      == {"storedName", {:ok, ["AION"]}}
  # and:
  #   == {"storedName", {:ok, [{"AION"}]}}
end

So we either use

0x0000000000000000000000000000000000000000000000000000000000000004 # number of bytes
  41494f4e00000000000000000000000000000000000000000000000000000000 # "AION"

as a result string (without offset that makes it (string) type), or expect every string result to be wrapped in a tuple (like [{"AION"}]).

As so, I think that it is not an issue of the ex_abi library. This library decodes string exactly as it should. The problem is in the interpretation of the function call result in the blockscout ethereum_jsonrpc component when it's string.

Of course, a bypass clause can be added to the apps/ethereum_jsonrpc/lib/ethereum_jsonrpc/encoder.ex:

def decode_result(result, %{returns: [:string]} = fs) do
  case decode_result(result, %{fs | returns: [{:tuple, [:string]}]}) do
    {id, {:ok, [{string}]}} ->
      {id, {:ok, [string]}}

    els ->
      els
  end
end

but usually this is not the best idea, and it is not related to the ex_abi.

As so, I suggest to close this issue and continue this discussion in the blockscout/blockscout#1546.

@ayrat555
Copy link
Collaborator Author

@zergera I think [string] vs [(string)] is exactly the problem of ex_abi library. Because we can't change abi interfaces of smart contracts

@volnyvolnyvolny
Copy link

@ayrat555.

At the moment, your library follows the letter of the specification, except for the decoding of the address type. According to the specification,

0x0000000000000000000000000000000000000000000000000000000000000004
  41494f4e00000000000000000000000000000000000000000000000000000000

is the encoded representation of the value "AION" (type string).

And

0x0000000000000000000000000000000000000000000000000000000000000020
  0000000000000000000000000000000000000000000000000000000000000004
  41494f4e00000000000000000000000000000000000000000000000000000000

is the encoded representation of the value ("AION") (type (string)).

If you'll try to decode the second byte sequence as a value of type string,

0x0000000000000000000000000000000000000000000000000000000000000020

will be interpreted as a byte length of the following string (32 bytes) and

0000000000000000000000000000000000000000000000000000000000000004

as a string itself.

  41494f4e00000000000000000000000000000000000000000000000000000000

will be considered as the next data piece. If you decide to change the decoding behavior, you will rely on the existence of this tail (rest) data. It doesn't feel like good practice.

@ayrat555
Copy link
Collaborator Author

@zergera can you please create PR in blockscout fixing all errors?

@volnyvolnyvolny
Copy link

@ayrat555, quick fix is blockscout/blockscout#1824 .

@vbaranov
Copy link
Collaborator

Decoding of string types fixed in 0.3.0 version of library #24

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

Successfully merging a pull request may close this issue.

3 participants