virtualeconomy / v-systems

V Systems Reference Full Node
Other
115 stars 45 forks source link

Refactor wallet log and default conf #220

Closed ncying closed 4 years ago

ncying commented 4 years ago

PR Details

Description

Refactor wallet log and default conf

Related Issue

215 #208

How Has This Been Tested

All unit test passed

Types of changes

Checklist

codecov-commenter commented 4 years ago

Codecov Report

Merging #220 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #220   +/-   ##
=======================================
  Coverage   63.64%   63.64%           
=======================================
  Files         199      199           
  Lines        5744     5744           
  Branches      297      297           
=======================================
  Hits         3656     3656           
  Misses       2088     2088           
Impacted Files Coverage Δ
src/main/scala/vsys/wallet/Wallet.scala 92.59% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 77e9f87...c915c72. Read the comment docs.

ncying commented 4 years ago

Config Files

On VSYS, it is hard to monitor sync progress from the log, and not possible to set up a supernode and monitor sync without API access.

I think that the best default setting for us would be API enabled, on 127.0.0.1

This will avoid making the user do additional configuration work. Since they need the API to even know if the node is shining correctly. If they want to securely access the web UI of a vsys node remotely, they can use an introspection tool like ngrok instead of binding to 0.0.0.0 (which is fine in some circumstances, but generally should be avoided).

Another alternative would be adding clear, explicit updates on sync progress to the log, every 1000 or 10,000 blocks, and shipping with the API disabled, since updates in the log on sync progress would allow the user to know if the node is syncing correctly without having the API enabled.

Seed in Log

This looks like it is fully resolved.

It is important to personalize the conf file before running the node. (wallet, API, some other related settings)

I think it is fine that users need to set the API key(need to change) and enable the API if they need to access the node data.

faddat commented 4 years ago

When running a full node (our most common scenario), users should not need to change anything in the config file, and it should be secure.

It should "just work".

In this case, especially because of #194 we have two options:

One Add a sync monitor to the log like:

Block 10,000 synced Block 20,000 synced Block 30,000 synced

So that users can monitor the progress of sync without using the API or web UI. That would make shipping with the API disabled by default totally acceptable, because the user would be able to verify that the node is synced. Iinstead of actually doing configuration, many users will get frustrated and stop using our product. so, it is more desirable to ship a product that works perfectly in a known good configuration.

Two

Ship with the API enabled and on localhost by default. This is still very safe. having the API on by default is only a problem when we are binding it to 0.0.0.0

faddat commented 4 years ago

btcd - zeroconf geth - zeroconf ava-labs/gecko - zeroconf Gaia - need to add seeds and genesis file and it's a pain. Binaries should ship with embedded seeds and those seeds should be suppressed if the user wishes to set up seeds. Genesis file should be downloaded via an embedded ipfs content address. Peercoin - zeroconf for just running a node

Node count is a quantifiable way of measuring the health of our network. Making it very, very easy to set up a full node, almost foolproof, will improve the health of the v systems network.

On the other side of that, changing the default configuration to one that does not allow the user to monitor the sync process of their new node, is not very helpful to users , because many users likely will not follow the documentation and configuration process but instead just run the node.