Enabling connections with websocket client #222
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
==========================================
- Coverage 99.70% 98.86% -0.85%
==========================================
Files 77 78 +1
Lines 4477 4562 +85
==========================================
+ Hits 4464 4510 +46
- Misses 13 52 +39 ☔ View full report in Codecov by Sentry. |
spec/eth/client_spec.rb
Outdated
| @@ -425,4 +437,8 @@ | |||
| expect(geth_ipc.is_valid_signature(contract, hashed, signature)).to be true | |||
| end | |||
| end | |||
|
|
|||
| describe ".send" do | |||
| pending("using the mock websocket server") | |||
There was a problem hiding this comment.
looking good so far. let me know when you are ready so I can review it.
|
📝 This test is failing on Ubuntu only. under investigation. |
lib/eth/client/ws.rb
Outdated
| # | ||
| # @param payload [Hash] the RPC request parameters. | ||
| # @return [Integer] Number of bytes sent by this method. | ||
| def send(payload) |
lib/eth/client/ws.rb
Outdated
|
|
||
| @ws.on :close do |e| | ||
| puts "-- websocket close (#{e.inspect})" | ||
| exit 1 |
There was a problem hiding this comment.
we should not exit here as we are only providing a library and not an application
There was a problem hiding this comment.
exit 1 has been deleted.
lib/eth/client/ws.rb
Outdated
| @ws = WebSocket::Client::Simple.connect @host | ||
|
|
||
| @ws.on :message do |msg| | ||
| puts ">> #{msg.data}" |
There was a problem hiding this comment.
let's not have stray puts in the code
There was a problem hiding this comment.
puts method has been removed.
spec/eth/client_spec.rb
Outdated
|
|
||
| sleep 0.001 | ||
| geth_dev_ws.send(payload) | ||
| sleep 0.001 |
There was a problem hiding this comment.
is there a way to avoid sleeping here?
There was a problem hiding this comment.
I'll look into ways to avoid using sleeping.
Co-authored-by: Afri <58883403+q9f@users.noreply.github.com>
Co-authored-by: Afri <58883403+q9f@users.noreply.github.com>
|
@q9f Thanks for the review. I've just taken in the fixes, please look at it again! |
|
tests fail due to shanghai (or lack thereof) |
There was a problem hiding this comment.
Is there a way we can abstract the way websockets work away from the client?
I.e., I would like to be able just to call chain_id and get a result.
lg = Logger.new(STDOUT, level: Logger::FATAL)
ws = Client.create("ws://127.0.0.1:8546", logger: lg)
ws.chain_id
# TypeError: no implicit conversion of Integer into String
# from /usr/lib/ruby/3.0.0/json/common.rb:216:in `initialize'
ws.default_account
# TypeError: no implicit conversion of Integer into String
# from /usr/lib/ruby/3.0.0/json/common.rb:216:in `initialize'Do you think this is possible?
| subject(:geth_ipc) { Client.create geth_ipc_path } | ||
| subject(:geth_http) { Client.create geth_http_path } | ||
| subject(:geth_http_authed) { Client.create geth_http_authed_path } | ||
|
|
||
| # it expects an $INFURA_TOKEN in environment | ||
| let(:infura_api) { "https://mainnet.infura.io/v3/#{ENV["INFURA_TOKEN"]}" } | ||
| subject(:infura_mainnet) { Client.create infura_api } | ||
| let(:logger) { Logger.new(STDOUT, level: Logger::FATAL) } | ||
| subject(:geth_dev_ipc) { Client.create geth_dev_ipc_path } | ||
| subject(:geth_dev_http) { Client.create geth_dev_http_path } |
|
|
||
| describe ".send" do | ||
| it "should set up the WebSocket connection" do | ||
| expect(geth_dev_ws.instance_variable_get("@ws")).to be_instance_of(WebSocket::Client::Simple::Client) |
| break if geth_dev_ws.instance_variable_get("@ws").open? || (Time.now - start_time > 3) | ||
| end | ||
|
|
||
| geth_dev_ws.send_request(payload) |
There was a problem hiding this comment.
while this is fine for a test, it would be good if it supports all APIs out of the box (instead of using send_request)
| # Wait for the response to be received | ||
| start_time = Time.now | ||
| loop do | ||
| break if received_data || (Time.now - start_time > 3) |
There was a problem hiding this comment.
It might be good to add a TIMEOUT to the client, so we don't have to deal with this externally
| expect(received_data["result"]).to start_with("0x") | ||
|
|
||
| contract = Eth::Contract.from_file(file: "spec/fixtures/contracts/dummy.sol") | ||
| geth_http.deploy_and_wait(contract) |
There was a problem hiding this comment.
| geth_http.deploy_and_wait(contract) | |
| geth_dev_ws.deploy_and_wait(contract) |
Okay, as a result, it appears that the number of bytes is returned.This is because the send method returns the number of bytes sent.Unlike HTTP, WebSocket is stateful in nature, so I may need to wait for the response before parsing it. I will think about it and try to implement it. 😄 From: /Users/kurotaki/ghq/github.com/q9f/eth.rb/lib/eth/client.rb:487 Eth::Client#send_command:
479: def send_command(command, args)
480: args << "latest" if ["eth_getBalance", "eth_call"].include? command
481: payload = {
482: jsonrpc: "2.0",
483: method: command,
484: params: marshal(args),
485: id: next_id,
486: }
=> 487: binding.pry
488: output = JSON.parse(send_request(payload.to_json))
489: raise IOError, output["error"]["message"] unless output["error"].nil?
490: output
491: end
[1] pry(#<Eth::Client::Ws>)> payload
=> {:jsonrpc=>"2.0", :method=>"eth_chainId", :params=>[], :id=>1}
[2] pry(#<Eth::Client::Ws>)> output = JSON.parse(send_request(payload.to_json))
TypeError: no implicit conversion of Integer into String
from /Users/kurotaki/.rbenv/versions/3.2.0/lib/ruby/3.2.0/json/common.rb:216:in `initialize'
[3] pry(#<Eth::Client::Ws>)> send_request(payload.to_json)
=> 79 |
|
I'm back! 😄 |
Enabling connections via Websocket. Connect using only Ruby libraries.
fix: #86
Sample procedure
Eth::Client::Ws.new
Output
cli.send
Output