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

use fixed version of ex_abi #1519

Merged
merged 1 commit into from
Jun 1, 2020
Merged

Conversation

ayrat555
Copy link
Contributor

@ayrat555 ayrat555 commented May 20, 2020

the latest version of ex_abi introduced a couple of bugs. The PR uses the patched version of ex_abi which fixes those bugs

@ayrat555 ayrat555 marked this pull request as draft May 20, 2020 06:58
@coveralls
Copy link

coveralls commented May 20, 2020

Coverage Status

Coverage increased (+0.08%) to 78.31% when pulling 73450b4 on ayrat555/use-fixed-version-of-ex_abi into 07774c8 on master.

@InoMurko
Copy link
Contributor

InoMurko commented May 20, 2020

Perhaps another thing we could now do - since ex_abi works - is to fetch ABI.FunctionSelector from plasma contracts for these two:
https://github.com/omisego/elixir-omg/blob/master/apps/omg_eth/lib/omg_eth/root_chain.ex#L61-L98
wdyt @ayrat555 ?

@ayrat555
Copy link
Contributor Author

@InoMurko Some abi encodings have failed in integration tests. I'll look into it

@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch 3 times, most recently from 12da1b3 to 2fc09a9 Compare May 22, 2020 11:33
@ayrat555
Copy link
Contributor Author

Perhaps another thing we could now do - since ex_abi works - is to fetch ABI.FunctionSelector from plasma contracts for these two:
https://github.com/omisego/elixir-omg/blob/master/apps/omg_eth/lib/omg_eth/root_chain.ex#L61-L98
wdyt @ayrat555 ?

@InoMurko How exactly these functions should be changed?

@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch from 2fc09a9 to 9dbf458 Compare May 22, 2020 14:09
@InoMurko
Copy link
Contributor

Perhaps another thing we could now do - since ex_abi works - is to fetch ABI.FunctionSelector from plasma contracts for these two:
https://github.com/omisego/elixir-omg/blob/master/apps/omg_eth/lib/omg_eth/root_chain.ex#L61-L98
wdyt @ayrat555 ?

@InoMurko How exactly these functions should be changed?

We can put both standardExits(uint160[]) and inFlightExits(uint160[]) ABI.FunctionSelector definitions into OMG.Eth.RootChain.AbiFunctionSelector.

When we have that, we can rewrite these two functions:

##
  ## these two cannot be parsed with ABI decoder!
  ##
  @doc """
  Returns standard exits data from the contract for a list of `exit_id`s. Calls contract method.
  """
  def get_standard_exit_structs(exit_ids) do
    contract = Configuration.contracts().payment_exit_game

    return_types = [
      {:array, {:tuple, [:bool, {:uint, 256}, {:bytes, 32}, :address, {:uint, 256}, {:uint, 256}]}}
    ]

    # TODO: hack around an issue with `ex_abi` https://github.com/poanetwork/ex_abi/issues/22
    #       We procure a hacky version of `OMG.Eth.Client.call_contract` which strips the offending offsets from
    #       the ABI-encoded binary and proceeds to decode the array without the offset
    #       Revert to `call_contract` when that issue is resolved
    call_contract_manual_exits(
      contract,
      "standardExits(uint160[])",
      [exit_ids],
      return_types
    )
  end

  @doc """
  Returns in flight exits of the specified ids. Calls a contract method.
  """
  def get_in_flight_exit_structs(in_flight_exit_ids) do
    contract = Configuration.contracts().payment_exit_game
    {:array, {:tuple, [:bool, {:uint, 256}, {:bytes, 32}, :address, {:uint, 256}, {:uint, 256}]}}

    # solidity does not return arrays of structs
    return_types = [
      {:array, {:tuple, [:bool, {:uint, 64}, {:uint, 256}, {:uint, 256}, :address, {:uint, 256}, {:uint, 256}]}}
    ]

    call_contract_manual_exits(
      contract,
      "inFlightExits(uint160[])",
      [in_flight_exit_ids],
      return_types
    )
  end

into something like RootChain blocks/1 or next_child_block/0, get_mined_child_block/0 so that the response data gets decoded via:

defp get_external_data(contract_address, signature, args) do
    {:ok, data} = Rpc.call_contract(contract_address, signature, args)
    Abi.decode_function(data, signature)
  end

@InoMurko
Copy link
Contributor

Both ABIs are defined in PaymentExitGame.json. You can run make init-contracts in the root of elixir-omg and search for the file.
find . -name PaymentExitGame.json

You can extract the abi defintion if you do this:

abi_path_to_payment_exit_game_json
      |> File.read!()
      |> Jason.decode!()
      |> Map.fetch!("abi")
      |> ABI.parse_specification(include_events?: true)

@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch 3 times, most recently from 0ef1f60 to 6556f3e Compare May 28, 2020 08:43
@ayrat555 ayrat555 marked this pull request as ready for review May 28, 2020 08:43
@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch 3 times, most recently from d93cfd3 to 73450b4 Compare May 29, 2020 05:16
@InoMurko
Copy link
Contributor

InoMurko commented Jun 1, 2020

OH this is amazing and it cleanups rootchain!

@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch from 73450b4 to 7a29622 Compare June 1, 2020 17:17
@ayrat555 ayrat555 force-pushed the ayrat555/use-fixed-version-of-ex_abi branch from 7a29622 to 3b46ad5 Compare June 1, 2020 17:42
@ayrat555 ayrat555 merged commit 1feed63 into master Jun 1, 2020
@ayrat555 ayrat555 deleted the ayrat555/use-fixed-version-of-ex_abi branch June 1, 2020 18:54
@unnawut unnawut added the bug Something isn't working label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants