Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

Possibility to specify IPC path & RPC over HTTP support #420

Closed
wants to merge 9 commits into from
Closed

Possibility to specify IPC path & RPC over HTTP support #420

wants to merge 9 commits into from

Conversation

tomusdrw
Copy link

Based on #393

  1. With this PR it would be possible to specify path to ipc socket (--rpc cli option)
  2. There is also mock version of net.Socket which uses HTTP as a transport.

I would really love to get rid of that: https://github.com/ethereum/mist/compare/develop...tomusdrw:rpc?expand=1#diff-a31b48952900046cc9945315da84b939R22
It seems that the responses are stuck somewhere if the requests are sent synchronously (setImmediate or lower timeouts are also not sufficient). But the response is genererated by server and also quiting the application causes the response to be received and processed (sic!). Any hints?

@frozeman
Copy link
Contributor

frozeman commented Apr 4, 2016

I will first merge @hiddentao PR and will then look at yours.

@frozeman
Copy link
Contributor

Can you rebase your changes on develop?

Conflicts:
	main.js
	modules/ipc/ipcProviderBackend.js
	modules/ipc/nodeConnector.js
@hiddentao
Copy link
Contributor

Would be nice to get this one in.

@@ -9,8 +9,12 @@ const log = require('../utils/logger').create('getIpcPath');

module.exports = function() {
var p = require('path');
var path = global.path.HOME;
var providedPath = global.rpcUri;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why you create this variable here? Why not simply if(global.rpcUri) ?

@tomusdrw
Copy link
Author

tomusdrw commented May 4, 2016

Thanks for comments. Updated the PR. Any hint what might be wrong with:
https://github.com/ethereum/mist/pull/420/files#diff-a31b48952900046cc9945315da84b939R22
(perhaps some better way to resolve this?)

@frozeman
Copy link
Contributor

frozeman commented May 5, 2016

Try timeout with 0 or 1 instead of 50. But not sure what could cause this. I will have a look if i find time

@alexvandesande
Copy link
Collaborator

@hiddentao I can't assign you this issue, but I'll leave this to you. Can you please review, and solve the merge issues? If you consider it useful then I'll merge it

@tomusdrw
Copy link
Author

Merged latest develop branch. Was trying lower values in setTimeout but the behaviour was inconsistent.

@hiddentao
Copy link
Contributor

Sure I'll take a look at this. Actually my other PR changes a whole bunch of stuff so this might need some rewrites anwway. I hope to have that PR ready for review tomorrow.

@hiddentao
Copy link
Contributor

hiddentao commented May 17, 2016

I think I'd like to take these changes one a time, so first merge in #601 to enable custom IPC paths. And then look at the possibility of a non-IPC connection to geth, as enabled by the HttpCompatSocket in this PR.

#559 introduces a dedicated Socket class which wraps around net.Socket so introducing a non-IPC socket should be much easier with that.

@hiddentao hiddentao self-assigned this May 18, 2016
@alexvandesande
Copy link
Collaborator

@hiddentao have your changes made this one redundant?

@hiddentao
Copy link
Contributor

Not quite. This one also suggest enabling the use of HTTP instead of IPC for RPC purposes. That part is still not integrated into our code.

-- 
Ramesh Nair

Director
HiddenTao Ltd (UK company no. 6807289)

http://hiddentao.com
https://github.com/hiddentao
https://www.linkedin.com/in/hiddentao

On 2 June 2016 at 03:57:44, Alex Van de Sande (notifications@github.com) wrote:

@hiddentao have your changes made this one redundant?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@hiddentao
Copy link
Contributor

Good idea would be to popup a warning message when user's use a HTTP connector, informing them that it's more risky than IPC.

@hiddentao
Copy link
Contributor

Replaced by #871

@hiddentao hiddentao closed this Jun 22, 2016
@lock
Copy link

lock bot commented Apr 1, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot unassigned hiddentao Apr 1, 2018
@lock lock bot locked and limited conversation to collaborators Apr 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants