nknorg / nkn-client-js

[Deprecated, use nkn-sdk-js instead] JavaScript implementation of NKN client
https://www.nkn.org
Apache License 2.0
55 stars 19 forks source link

Direct HTTP requests to the RPC node that is in use #68

Closed losnappas closed 4 years ago

losnappas commented 4 years ago

Currently getSubscribers and some other requests are being directed to the seedRpcServerAddr that was used in initializing the client.

By default, how about directing those at the node that we are connected to, instead? Is that something we can do without problems?

If that is something that can be done, then in D-Chat I would also configure the wallet to send requests to the same node. It is better that way because you either have everything break down, or everything will work. Currently I see people not getting subscribed to chats, and while it's hard to pinpoint the cause, I think that this could be one of them.

Also, I think dropping "Origin" and "Referer" headers from requests is not a terrible idea if they aren't needed, since some will argue that those lower privacy.

yilunzhang commented 4 years ago

By default, how about directing those at the node that we are connected to, instead? Is that something we can do without problems?

We can try doing so, but it will not guarantee to work because RPC is using port 30003 while websocket is using port 30002. RPC port 30003 is not required for a node to fully work properly while port 30002 is required.

Currently I see people not getting subscribed to chats, and while it's hard to pinpoint the cause, I think that this could be one of them.

We can try debugging but I doubt whether it's related to the seed nodes since they are quite stable AFAIK. But there is one thing that can definitely reduce failure regardless what it is (unless it's the client side bug): sending the same transaction to multiple nodes concurrently.

Also, I think dropping "Origin" and "Referer" headers from requests is not a terrible idea if they aren't needed, since some will argue that those lower privacy.

Are you talking about the response header from JSON RPC? If so then the Access-Control-Allow-Origin is needed if anyone wants to call JSON RPC using js from any website.

yilunzhang commented 4 years ago

Ah I forgot to mention, subscribe and all other actions that need to sign transaction are done by nkn wallet rather than nkn client.

losnappas commented 4 years ago

We can try doing so, but it will not guarantee to work because RPC is using port 30003 while websocket is using port 30002. RPC port 30003 is not required for a node to fully work properly while port 30002 is required.

I see, probably not worth it then, unless we build fallback mechanisms for the connection failure case as well.

Doesn't this mean that we will more or less forever depend on hubs like nkn-org for these actions? I mean, the pubsub is not functional at all if we can't get the subscriber list from a node, and if one cannot assume that the node we are connected to will have that open, they'll be directing that traffic to nkn-org or their own node, probably.

Are you talking about the response header from JSON RPC? If so then the Access-Control-Allow-Origin is needed if anyone wants to call JSON RPC using js from any website.

I was talking about the request header, because for a Firefox web extension, these are actually UUID's that don't change, although there's hope that Firefox itself will fix that sometime. Screenshot from 2019-12-05 15-29-21

yilunzhang commented 4 years ago

Doesn't this mean that we will more or less forever depend on hubs like nkn-org for these actions? I mean, the pubsub is not functional at all if we can't get the subscriber list from a node, and if one cannot assume that the node we are connected to will have that open, they'll be directing that traffic to nkn-org or their own node, probably.

I don't think that's a problem. Basically for any decentralized system or p2p system, there is always some prior knowledge required in order to join the system. Typically it's at least one node currently in the system. This is true for NKN as well: node needs to know a seed node to join the system, client needs to know a seed node to get its position, wallet needs to know a seed node to send txn. But it does not mean the system is centralized, because the seed node can literally be ANY node in the system. A few examples are:

  1. Official seed node
  2. Each application developer can use a bunch of his own nodes, and since the nodes are mining, the cost are mostly covered by itself
  3. Client/wallet can remember the last known node list and use them on next start up.

So if the official seed node or hub node becomes a problem, people can switch to other solutions really easily.

I was talking about the request header, because for a Firefox web extension, these are actually UUID's that don't change, although there's hope that Firefox itself will fix that sometime.

I don't think the request headers are used at all so there is no concern on the node side.

losnappas commented 4 years ago

Right, about that

  1. Client/wallet can remember the last known node list and use them on next start up.

An ideal situation would be that you use a hard-coded seed node once, then go "off-grid" by following the previously used nodes, but if nodes stop taking certain transactions, it will be rough.

Previously:

We can try debugging but I doubt whether it's related to the seed nodes since they are quite stable AFAIK. But there is one thing that can definitely reduce failure regardless what it is (unless it's the client side bug): sending the same transaction to multiple nodes concurrently.

We should definitely do that; sending the same tx to multiple nodes. The seed nodes might be quite stable, but even still, when they do fail us, it's really quite hard to prepare for that, and it is catastrophic to the application.

yilunzhang commented 4 years ago

An ideal situation would be that you use a hard-coded seed node once, then go "off-grid" by following the previously used nodes, but if nodes stop taking certain transactions, it will be rough.

This can only be done in the application layer because the sdk itself is stateless, i.e. it does not have any storage capacity. But the sdk can provide API to save and load seed node

losnappas commented 4 years ago

Right, but if I were to

  1. Get the node that I'm supposed to connect to, from an nkn.org seed node
  2. Store that connection node's IP address
  3. Next time I come online, I'd use the IP address as my seed, instead of nkn.org

If that seed node then doesn't accept getSubscribers and other RPC requests, then what?

A solution to this could be to make nkn-client-js accept an array of seed nodes, and then it would pick one that can deal with RPC requests and send them there, if the current connection-node doesn't.

yilunzhang commented 4 years ago

A solution to this could be to make nkn-client-js accept an array of seed nodes, and then it would pick one that can deal with RPC requests and send them there, if the current connection-node doesn't.

Yeah that's actually the design, nkn node and go sdk can already accept an array of seed nodes, but we haven't updated js sdk to this way yet.

PS: currently node cannot configure itself not to accept some RPC unless re-compile. Most of the times the problem is in the network part: port 30003 may not be open at all 😂

yilunzhang commented 4 years ago

Closed because nkn-client-js is deprecated in favor of nkn-sdk-js. We can continue our discussion there if needed.