search

Anoncoin / oldrepo-backup

Old repo, keept for issues. Use "anoncoin" which don't link back to litecoin and suggests PR to be merged there.
https://anoncoin.net
MIT License
66 stars 32 forks source link

I2P settings should be self-managed #98

Closed ssua6 closed 8 years ago

ssua6 commented 8 years ago

The current implementation requires that users initialize the I2P settings within the anoncoin.conf file after downloading the AC client. Furthermore they are required to copy both the private key from the built-in generator to that file.

1) The user should not be required to manually copy this information over; the software should handle this 2) The settings should not be stored within a plain text file

Cryptoslave commented 8 years ago

OK

Cryptoslave commented 8 years ago

Solved for windows: https://github.com/Anoncoin/anoncoin/commit/bd55fa77fd4e792ce421534f53f62699e5e9fa2a

The installation shall be done with a running I2P router in default configuration and SAM bridge The installer at the end of installation launch anoncoin in generatei2pdestination and onlynet=i2p. Anoncoin.conf.sample also pop up. A windows open which tells the privatekey and other info. One click on the button copy privatekey, one past in the open anoncoin.conf.sample, saving into anoncoin.conf in the newly created data directory and at next launch the anoncoin-qtc is running in onlynet=i2p with static address!

Anoncoin-qtt was removed from the setup.

For linux I think the users can make it themselves with a setup. The new anoncoin.conf.sample also explain how to do it in text mode for them.

ssua6 commented 8 years ago

I do not agree that this should be closed just yet. We should still store all I2P settings within an external binary file for any OS. The whole point of this ticket (item 1) was to remove pretty much any setup needed by the user, and currently that still needs to occur.

Cryptoslave commented 8 years ago

already with the " -i2p.options.enabled=1 -onlynet=i2p -generatei2pdestination -stfu=1 " launched after setup it generate the i2p address automatically, only need to redirect to a file and read from it at startup.

Shall be this priority order if i2p enabled: 1) anoncoin.conf privatekey= 2) i2pkey.dat 3) generate a dynamic address

ssua6 commented 8 years ago

Branch created for this effort: issue98_i2pconfigfile

Cryptoslave commented 8 years ago

Test setup:

Test running anoncoin-qtc:

So the logic is this:

The setting static= in anoncoin.conf is hence not used in this case, but only the presence of the file.

In that case the i2pkey.dat is not used. The setting is then the same than actual configuration.

So the outcome is first decided by the presence of the privatekey in anoncoin.conf, then by the presence of the I2Pkey.dat file.

Test running anoncoin-qtc in the absence of anoncoin.conf:

Furthermore the doc/anoncoin.conf.txt will open on setup to let the user have the possibility to change setting and save it in the %appdata%/anoncoin directory, in case he does not use a normal I2P configuration or need to set his RPC user:pass or other options.

Cryptoslave commented 8 years ago

This is the commit: https://github.com/Anoncoin/anoncoin/commit/47f93cb72c8acddebc1dd3fa27324256b831ca43

Cryptoslave commented 8 years ago

So ssuag did you check this?

Also: will you use it in your future git commit? I saw you did a lot of recent commits ;)

CS

ssua6 commented 8 years ago

Thanks for looking into this. Probably not the i2pkey.dat stuff since I've completed all of that already and it conflicts with how you do use it here.

Once my testing is done I'll look more closely at how I can integrate your solution into mine. For something not solidified like this, next time I would use a separate branch.

Ssua6

ssua6 commented 8 years ago

Halfway done with non-QT testing:

Cryptoslave commented 8 years ago

Nice!

Yeah, feel free to check the code I did to resolve conflict between user configured (in anoncoin.conf) and auto-generated static keys. If you check the commit you will see there are in init.cpp and in i2pwrapper.cpp two routines that do the checks. It is quite self explanatory, just check the commit https://github.com/Anoncoin/anoncoin/commit/47f93cb72c8acddebc1dd3fa27324256b831ca43 and use whatever you want in your own solution...

Good job!

Cryptoslave commented 8 years ago

BTW when you say:

hardcoded default values (values taken from anoncoin.conf).

Do you mean you copied anoncoin.conf that I gave https://wiki.anoncoin.net/Anoncoin-0.9.6.10.mainnet.private.HARDFORK.conf ? Or another one? Or none?

I do not understand, if it is hardcoded, it is not in the anoncoin.conf file...

Also if you took my default anoncoin.conf (which is probably not the best for release), how do you generate the rpcuser and rpcpassword? There is a routine to auto generate them in anoncoind, but maybe you ask for the user? Or give him the choice to do what he prefer? Finally what will be the role of anoncoin.conf? will it override the hardcoded value? will it override i2p data file?

Can you provide more details, I do not get a complete overview of what you are doing...

Thx, CS

Cryptoslave commented 8 years ago

Well I saw in the i2pdata file in your commit it concern only the value in anoncoin.conf at heading [i2p.mydestination] and [i2p.options]

I do not think the work will be complete without a setting of ALL the necessary values taken from anoncoin.conf the most important and necessary are as follow (default value):

daemon (1) server (1) listen (1) discover (0) rpcuser (username) rpcpassword (password) rpcport (9376)

onlynet (i2p). Can be i2p, tor, darknet, ipv4, ipv6, several are possible at the same time. Right now it is disabled by default but I think it shall be enabled to "i2p" per default in the new release, if the i2p setting and sam router are right, but the user shall be asked if he want IPv4 connectivity (mixed mode, onlynet is disabled then) or pure i2p. stfu (0) 0 per default but some user such as miner have to set it to 1) upnp (1) whitelist (other local IP) (disabled but important if using on a LAN with multiple nodes) bind (own local IP) (disabled but important if using on a LAN with multiple nodes) txindex (0) but shall be 1 if the user want to use multisig addresses, which also need a future GUI btw)

maybe shareaddr (static are shared, dynamic are not, here the user can force sharing or remove it; probably we shall in fact not include it to force the default behaviour, special case can set up manually in anoncoin.conf)

The others from anoncoin.conf are for more specific case such as debuging, testnet, shareaddr also maybe, etc..

What do you think ssuag?

ssua6 commented 8 years ago

I agree with you that the configuration of these settings should be capable via the GUI, but I do not think it should be a part of this effort. This ticket is specifically for I2P-related control. I think we should do the above, but it should not gate us from the next release.

The other stuff that you mention related to debugging, testnet, etc should probably remain there since the general user should not tamper with those settings unless they absolutely wish.

ssua6 commented 8 years ago 
Also if you took my default anoncoin.conf (which is probably not the best for release), 
how do you generate the rpcuser and rpcpassword? 
There is a routine to auto generate them in anoncoind, but maybe you ask for the user? 
Or give him the choice to do what he prefer?

Are these needed for I2P functionality? If not, then they should follow with my above response. 

Finally what will be the role of anoncoin.conf? 
Will it override the hardcoded value? will it override i2p data file?

In my mind, most if not all I2P configuration will be obsoleted from anoncoin.conf. The i2p data file will always take precedence for i2p related settings. As you described above, some of the settings should be moved to GUI control. The rest can remain there. Essentially users should not need to ever open that configuration file unless they wish.

yhaenggi commented 8 years ago 
Are these needed for I2P functionality? If not, then they should follow with my above response.

No, rpcuser + rpcpassword is needed for running anoncoind or when you want to do rpc calls to your anoncoin-qt. With the QT you dont need it to be set.

ssua6 commented 8 years ago
Cryptoslave commented 8 years ago

ssua6, I tried your patch, it works OK on windows. ...but you shall apply part of my patch because you dismiss anoncoin.conf setting and then because of that it cannot become static address. You shall force it to static like I did in my patch. For this you shall change both routine in init.cpp and in i2pwrapper.

See https://github.com/Anoncoin/anoncoin/commit/47f93cb72c8acddebc1dd3fa27324256b831ca43 and https://github.com/Anoncoin/anoncoin/commit/0f9b8229e3d44ba757f51a81ba298f7c30239857

In the last one is the hardfork bug patch that you have not applied and also I had I think the i2p only bug. This last one is is: the wallet run i2p only but it report that it is also on clearnet. When I ran it it reported running on clearnet but I dont think so, I had a similar issue too, corrected in the commit above.

ssua6 commented 8 years ago

A new dialog has been under the settings menu: The I2P Settings Menu. See revision https://github.com/Anoncoin/anoncoin/commit/135c654870a19b0d972807953d527f3986d7db2e if curious. Here's how it operates:

My enhancement is affect only the Anoncoin GUI client. The CLI and daemon clients should not and will not be affected.

Cryptoslave commented 8 years ago

Did you include the command line too? It was not last time I tried...

does "C:\Program Files\Anoncoin\anoncoin-qtc.exe" -i2p.options.enabled=1 -onlynet=i2p and "C:\Program Files\Anoncoin\anoncoin-qtc.exe" -i2p.options.enabled=1 -onlynet=i2p -generatei2pdestination both works without any anoncoin.conf and neither I2p.dat present (ie straight new config)?

ssua6 commented 8 years ago

Does not work, but I know where the problem is. I'll fix it soon.

ssua6 commented 8 years ago

Tested with and without anoncoin.conf, using -i2p.options.enabled=1 -onlynet=i2p and -i2p.options.enabled=1 -onlynet=i2p -generatei2pdestination. Works as expected.

To include these changes in the build, run ./configure with option --enable-i2psettings. Default is disabled for now.

Closing issue as the majority of this effort has been completed. Any new problems should be addressed under a new issue once this has been merged to mainline.