IpcClient Implementation #40

hswick · 2018-10-04T02:12:37Z

I found a neat way to do #8 using no dependencies, just :gen_tcp.

hswick · 2018-10-04T02:28:25Z

Looks like it isn't finding the IPC socket path in the CircleCI build.

Perhaps this may be a solution https://serverfault.com/questions/881875/expose-a-unix-socket-to-the-host-system-from-inside-from-a-docker-container

ayrat555 · 2018-10-05T07:26:48Z

lib/ethereumex/ipc_client.ex

@@ -0,0 +1,55 @@
+defmodule Ethereumex.IpcClient do


There seems to be a lot of duplication in Ethereumex.IpcClient and Ethereumex.HttpClient. Can we get rid of this duplication? Maybe we can move common logic to separate module

ayrat555 · 2018-10-05T07:43:16Z

lib/ethereumex/ipc_server.ex

+  end
+
+  def handle_call({:request, request}, _from, %{socket: socket} = state) do
+    :ok = :gen_tcp.send(socket, request)


Can we somehow handle socket/request errors here? Maybe we can pass those errors to Ethereumex.IpcClient.post_request/2 and return them to a user. What do you think about it?

ayrat555

@hswick great work! A couple of comments, otherwise this LGTM.

ayrat555 · 2018-10-05T08:01:05Z

@hswick There is a bounty for this issue on gitcoin. You should submit your PR (I don't know the details, never used it).

ayrat555 · 2018-10-05T08:22:10Z

@hswick Some thoughts on IpcServer. In the current implementation, HttpClient is started by default. I think we can configure which client to start (or both of them) using configuration parameters. IpcServer and IpcClient can be started together on the same level of the supervision tree

hswick · 2018-10-06T16:31:38Z

Hi @ayrat555, I made the changes you recommended.

ayrat555 · 2018-10-06T16:34:05Z

lib/ethereumex/client/base_client.ex

+
+      @spec single_request(map(), []) :: {:ok, any() | [any()]} | error
+      def single_request(payload, opts \\ []) do
+	payload


something's wrong with formatting

Oops. I always forget to run format

ayrat555 · 2018-10-06T16:44:09Z

lib/ethereumex/config.ex

@@ -25,4 +25,25 @@ defmodule Ethereumex.Config do
  def request_timeout do
    Application.get_env(:ethereumex, :request_timeout, 5000)
  end
+
+  def setup_children do
+    setup_children(Application.get_env(:ethereumex, :client, :http))


What do you think about setting up children directly in Ethereumex module and using Config module only for getting configuration params?

ayrat555 · 2018-10-06T16:44:45Z

lib/ethereumex/ipc_client.ex

+defmodule Ethereumex.IpcClient do
+  use Ethereumex.Client.Macro
+  use Ethereumex.Client.BaseClient
+  # import Ethereumex.Config


unused import

ayrat555 · 2018-10-06T16:45:48Z

config/config.exs

@@ -2,4 +2,8 @@ use Mix.Config

 config :ethereumex, request_timeout: 5000

+config :ethereumex, :client, :ipc
+
+config :ethereumex, :ipc_path, "/.local/share/io.parity.ethereum/jsonrpc.ipc"


I think this value is used for testing?

ayrat555 · 2018-10-06T16:55:53Z

@hswick Thanks again. Tomorrow I will test these changes locally, will look into eth_get_block_by_number (it's one of the most used API methods). I want this PR to get merged as soon as possible

hswick · 2018-10-06T17:00:56Z

@ayrat555 you are very welcome! Thanks for all of the quick feedback. Excited to use this with ExW3. Best of luck with debugging!

ayrat555 · 2018-10-07T11:02:11Z

@hswick Can you add me to your fork? I think I found the solution to the truncated json problem. I want to push changes to your branch

ayrat555 · 2018-10-07T16:44:13Z

.circleci/config.yml

@@ -11,7 +11,7 @@ jobs:
      - run: echo > passfile
      - run: ./parity --chain 2>&1 &


@hswick you can start parity with /parity --jsonrpc-interface all --no-warp. It starts ipc and http servers
`

CHANGELOG.md

hswick · 2018-10-07T18:48:14Z

Good news is I got the build working.

Bad news any version of parity above 1.10 makes the tests fail, but I think that should be created on a separate issue.

ayrat555 · 2018-10-07T18:50:06Z

README.md

@@ -37,6 +37,24 @@ config :ethereumex,
  request_timeout: 10_000 # default is 5000 ms
 ```

+If you want to use IPC you will need to a few things in your config.


need to set?

yep... good catch

ayrat555 · 2018-10-07T18:53:05Z

Bad news any version of parity above 1.10 makes the tests fail, but I think that should be created on a separate issue.

I ran manually compiled parity from the latest master branch. And it seems to work fine.
@hswick Can you create a separate issue?

hswick · 2018-10-07T18:54:01Z

Bad news any version of parity above 1.10 makes the tests fail, but I think that should be created on a separate issue.

I ran manually compiled parity from the latest master branch. And it seems to work fine.
@hswick Can you create a separate issue?

#43

ayrat555 · 2018-10-07T19:00:52Z

@hswick 🔥 Amazing work!

ayrat555 · 2018-10-07T19:28:55Z

https://hex.pm/packages/ethereumex/0.4.0

hswick added 7 commits October 3, 2018 14:02

Created failing test cases

91b2593

Got a number of tests passing

d4e8c14

0 failures on eth tests

408018d

Eth and Batch tests passing

cb89af6

Using system home to get ipc path

712dc90

Ran mix format

65d7e2b

Added module_doc tag

671f8aa

hswick force-pushed the ipc branch from 8f4a178 to 671f8aa Compare October 4, 2018 21:48

ayrat555 reviewed Oct 5, 2018

View reviewed changes

ayrat555 approved these changes Oct 5, 2018

View reviewed changes

hswick added 4 commits October 6, 2018 10:38

Abstracted client behavior into BaseClient

98b52ea

IpcServer passing error up to client

75f3372

IpcClient handling IpcServer errors

c8f0bd2

Configure which client to use on startup

059e820

ayrat555 reviewed Oct 6, 2018

View reviewed changes

Format

a8b36be

hswick mentioned this pull request Oct 6, 2018

Implement Ipc Client #8

Closed

ayrat555 reviewed Oct 6, 2018

View reviewed changes

hswick added 2 commits October 6, 2018 11:53

Moved setup_children to ethereumex.ex

bc06b0c

Removed unused import + format

62de256

hswick and others added 11 commits October 7, 2018 11:00

setup_children uses Config.ipc_path

c0c9cac

Handle ipc path config more gracefully

664dc9d

Updated README and CHANGELOG

c4252a7

Use precompiled parity binary

3ad2c72

Swap out kill command

e2c3ed1

fix base client macro test

96e3fc3

Try lsof as root

fd30f82

Merge branch 'ipc' of github.com:hswick/ethereumex into ipc

8ca08f1

Install lsof command

c5df300

Remove sudo

acb07f2

Update apt-get

980c138

ayrat555 reviewed Oct 7, 2018

View reviewed changes

Dont do lsof as root

70e42ea

ayrat555 reviewed Oct 7, 2018

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update version numbers

24dc8d0

hswick force-pushed the ipc branch 2 times, most recently from b62fe70 to c21712a Compare October 7, 2018 18:15

Tests passing on circle ci

df91c02

hswick force-pushed the ipc branch 2 times, most recently from bbbac21 to df91c02 Compare October 7, 2018 18:32

Appeassed dialyzer

a2ce65f

ayrat555 reviewed Oct 7, 2018

View reviewed changes

Typo fix

1c1ddc1

hswick mentioned this pull request Oct 7, 2018

Tests fail on latest version of Parity #43

Closed

ayrat555 merged commit cfc2732 into mana-ethereum:master Oct 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IpcClient Implementation #40

IpcClient Implementation #40

hswick commented Oct 4, 2018 •

edited by ayrat555

Loading

hswick commented Oct 4, 2018 •

edited

Loading

ayrat555 Oct 5, 2018

ayrat555 Oct 5, 2018

ayrat555 left a comment

ayrat555 commented Oct 5, 2018 •

edited

Loading

ayrat555 commented Oct 5, 2018

hswick commented Oct 6, 2018

ayrat555 Oct 6, 2018

hswick Oct 6, 2018

ayrat555 Oct 6, 2018

ayrat555 Oct 6, 2018

ayrat555 Oct 6, 2018

ayrat555 commented Oct 6, 2018

hswick commented Oct 6, 2018

ayrat555 commented Oct 7, 2018

ayrat555 Oct 7, 2018

hswick commented Oct 7, 2018

ayrat555 Oct 7, 2018

hswick Oct 7, 2018

ayrat555 commented Oct 7, 2018 •

edited

Loading

hswick commented Oct 7, 2018

ayrat555 commented Oct 7, 2018

ayrat555 commented Oct 7, 2018

		@@ -11,7 +11,7 @@ jobs:
		- run: echo > passfile
		- run: ./parity --chain 2>&1 &

IpcClient Implementation #40

IpcClient Implementation #40

Conversation

hswick commented Oct 4, 2018 • edited by ayrat555 Loading

hswick commented Oct 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayrat555 left a comment

Choose a reason for hiding this comment

ayrat555 commented Oct 5, 2018 • edited Loading

ayrat555 commented Oct 5, 2018

hswick commented Oct 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayrat555 commented Oct 6, 2018

hswick commented Oct 6, 2018

ayrat555 commented Oct 7, 2018

Choose a reason for hiding this comment

hswick commented Oct 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayrat555 commented Oct 7, 2018 • edited Loading

hswick commented Oct 7, 2018

ayrat555 commented Oct 7, 2018

ayrat555 commented Oct 7, 2018

hswick commented Oct 4, 2018 •

edited by ayrat555

Loading

hswick commented Oct 4, 2018 •

edited

Loading

ayrat555 commented Oct 5, 2018 •

edited

Loading

ayrat555 commented Oct 7, 2018 •

edited

Loading