vipnode / vipnode.org

Connect instantly with your Ethereum Light Client to a premium vipnode.
https://vipnode.org/
MIT License
45 stars 6 forks source link

Allow for changing the registered enodeID from the paying owner address #3

Closed ligi closed 5 years ago

ligi commented 6 years ago

As far as I understand the current approach you attach value to the key behind the node. Currently the key for the node is not part of the keystore and after a discussion with @fjl @holiman and @zsfelfoldi it should not be. But it is a problem as e.g. in WallETH the key would now be in the cache and if the user deletes the cache - the key is gone. A solution I see would be that a real account holds the value and can authorize enodes. I think this would even save some gas as we do not have to save enode strings on the chain. So a contract just stores which addresses payed money - and the key behind this account can then be used to authorize enodes. - cc @karalabe

shazow commented 6 years ago

Hm, do you have any more discussion I could read about why we want to avoid correlating the enodeID with the client?

Wouldn't the user deleting their storage be equivalent to losing their private keys as well?

You're right that the current smart contract is a simple enodeID -> expiration lookup, but it doesn't need to be that way. In fact, the registration mechanism doesn't need to be a smart contract at all, it could just be a standard SaaS-style signup backend.

Also given that it's pay-what-you-want for now, it doesn't really matter if the client needs to re-register after nuking their cache.

For a contract backend, I am open to allowing an address "own" the registration and be able to change the enodeID it's representing. Perhaps for the next iteration, if it doesn't add too much complexity. :)

ligi commented 6 years ago

Wouldn't the user deleting their storage be equivalent to losing their private keys as well?

No e.g. private keys can be easily backup'ed - node-keys not so much as they do not live in the keystore. Also for the keystore you can specify a separate directory - e.g. using the app directory in WallETH for the keystore and the cache-dir for the rest (e.g. the part where the node key lives)

shazow commented 6 years ago

node-keys not so much as they do not live in the keystore.

Interesting. Is this by design? Sounds like a bug.

ligi commented 6 years ago

No - it is by design. But the design also implies that the node-key holds no value.

shazow commented 6 years ago

Is there any discussion I could read up more on this design decision?

I'm actually in the process of making a proposal to lean more heavily on node keys for things like ethstats. Right now all the stats is completely unauthenticated which makes ethstats servers very brittle and unreliable.

I feel like there is some positive value in semi-persisting node keys at least for maintaining positive behaviour/reputation on the network (or for credit-based systems like vipnode). At the same time, the node keys don't need to be super permanent as long as they can be easily updated from the corresponding owner address key.

ligi commented 6 years ago

Is there any discussion I could read up more on this design decision?

not sure if there is any reading material - I got this info in a verbal conversation with @fjl @zsfelfoldi @holiman and @karalabe

shazow commented 6 years ago

Got it.

I thiiiiink what I have in mind is compatible with what we want, but I'd love any more details if folks can provide it. :)

holiman commented 6 years ago

In ~/.ethereum/geth/, there's a file called nodekey, which contains the private key for the node. I don't think there are any design documents... The node needs the private key all the time, and it wasn't attached to any value, so I guess there was no point in having keystore-type encryption around it.

If the nodekey is missing, a new one is generated.

shazow commented 6 years ago

@holiman Hypothetically, any feelings on whether a PR that moves the nodekey into the keystore would be objectionable?

zsfelfoldi commented 6 years ago

@shazow I am working on an RPC API extension for configuring LES servers that allows different levels of prioritization so that server operators can offer different levels of service either based on some payment model or any other criteria. (I believe this priority model can be intergrated with your VIP node payment model too) We had a discussion about this with the Geth team last week and as @ligi suggested, we should not assign value directly to the node ID. It is a matter of choice but this is how the system was designed, node IDs are not supposed to be critical. What we decided to do is an authorization at handshake using a permanent key from the key store. This is something I am planning to implement very soon.

zsfelfoldi commented 6 years ago

Authorization of any node ID has the additional advantage of being able to use your VIP account from multiple devices with different node IDs (maybe not at the same time).

shazow commented 6 years ago

What we decided to do is an authorization at handshake using a permanent key from the key store.

@zsfelfoldi That makes sense, and that is my line of thinking also (to use a keypair challenge/signature handshake to authenticate a node) but why create yet another key instead of graduating the nodekey to permanent storage? If we do create yet another keypair that lives in permanent storage, is there still a point in retaining the nodekey?

Authorization of any node ID has the additional advantage of being able to use your VIP account from multiple devices with different node IDs (maybe not at the same time).

I think that's true with reusing nodekeys, too? Should be able to use the same nodeID across different clients, just not at the same time.

fjl commented 6 years ago

There are a few issues with putting the node key into the wallet key store using the same format used by all other keys:

@ligi reports that there are issues with node key handling on Android, but these issues are related to the file system location of the node key file. We can move the file somewhere else to solve that.

shazow commented 6 years ago

Node keys are unencrypted. This is important because users would need to enter their password on startup otherwise.

Great point!

If we would store the node key in the key store, it would show up as a wallet.

Does this mean that the "permanent key from the key store" which @zsfelfoldi referred to is actually a wallet private key, not a special type of key? That would make more sense.

fjl commented 6 years ago

Yes, that's what he meant.

shazow commented 6 years ago

Got it. In that case, I'm very interested in this RPC API extension, @zsfelfoldi. :) Please share any details/progress if possible.

shazow commented 6 years ago

Progress on the revised smart contract is here which tracks clients by wallet address and has a list of whitelisted nodeIDs that can be changed: https://github.com/vipnode/vipnode-contract/blob/master/contracts/VipnodePool.sol

I am still considering other more-or-less-on-chain schemes, feedback welcome.

Note that the payment mechanism is going to be a fairly pluggable aspect of the vipnode pool framework.

shazow commented 5 years ago

This is done in master, next release will have a full working demo with this feature.