Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update vsys-mainnet.conf #214

Closed
wants to merge 3 commits into from
Closed

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Jul 11, 2020

This moves us closer to having sane defaults for users by adding network features that are recommended for stable, fast operation of VSYS nodes.

It will be accompanied by a bash script for Debian 10 machines that runs VSYS in the exact recommended manner.

PR Details

Update vsys-mainnet.conf with recommended default values from #210 in preparation for the release of a script that installs vsys in the exact recommended manner.

Description

Add new network settings.

Related Issue

Motivation and Context

Addresses node stability and performance when combined with a script that runs vsys from the JAR file in exactly the right manner.

How Has This Been Tested

On my VSYS node.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.

  • My change requires a change to the documentation.

  • I have updated the documentation accordingly.
    Documentation will be updated upon completion of the bash script.

  • I have added tests to cover my changes.

  • All new and existing tests passed.

This moves us closer to having sane defaults for users by adding network features that are recommended for stable, fast operation of VSYS nodes.  

It will be accompanied by a bash script for Debian 10 machines that runs VSYS in the exact recommended manner.
@codecov-commenter
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #214   +/-   ##
=======================================
  Coverage   63.64%   63.64%           
=======================================
  Files         199      199           
  Lines        5744     5744           
  Branches      297      297           
=======================================
  Hits         3656     3656           
  Misses       2088     2088           

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...8c0c56a. Read the comment docs.

faddat added 2 commits July 12, 2020 09:47
RPC API to 127.0.0.1
Remove wallet from default configuration, remove default wallet password
@faddat
Copy link
Contributor Author

faddat commented Jul 12, 2020

I am now running a second test, with the wallet removed and RPC API set to localhost.

@faddat
Copy link
Contributor Author

faddat commented Jul 12, 2020

VSYS still automatically generates a seed and prints it to the log, even with the wallet section of vsys-mainnet.conf removed.

@faddat faddat mentioned this pull request Jul 12, 2020
9 tasks
Copy link
Member

@ncying ncying left a comment

Choose a reason for hiding this comment

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

I can not approve of most of the changes in this PR. The only part I think is necessary is the bind-address in the API part.

For the wallet setting part, it is useless here and it is exactly the default one. I can not understand why the log can not print the seed directly if the user asks the node to randomly generate the seed. If one could get the seed from your server, he/she should get the wallet.dat file too. I think I answer this "issue" clearly in #208

For the network part, the default setting is for most of the machine which can not support large connections and buffers, my suggestions in #194 are only for some rare cases. We tested the node software in many cheaper machines, it works well with the default settings, if you change the connection to a larger one, it may cause some other issues for these machines. BTW, it should update some network code if we need to solve this issue clearly. Change configures should consider more situations!!

@ncying
Copy link
Member

ncying commented Jul 14, 2020

  1. wallet setting is not used, removed seed from the logs in Refactor wallet log and default conf #220
  2. default network setting should be kept
  3. bind API to local interface only and also disable the node's REST API in Refactor wallet log and default conf #220

@ncying ncying closed this Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants