Network namedtuple constants

peerchemist commented 6 years ago

It greatly simplifies internal state, dropping all mainnet=None arguments of various methods through the codebase.

peerchemist commented 6 years ago

Hi, do you plan to merge this? I guess it needs an update now to reflect the recent changes.

SimoneBronzini commented 6 years ago

There are a couple of parts I need to review thoroughly, as soon as I find the time to do that I'll leave you some feedback

SimoneBronzini commented 6 years ago

All right, I did a quick review of the code, I think it has the following problems:

It removes network checks from the whole library which I do not think is a good idea as users might want to be on the safe side and make sure they will never manipulate a test Address/ExtendedKey/PrivateKey in a mainnet environment and vice versa, unless explicitly specified
It removes all network choices (i.e. I might want to explicitly specify a different network than the one that is currently set) that can be useful when writing tests The idea behind these checks and choices has always been: "btcpy helps you stay safe unless you explicitly specify you don't want to".

This PR would, of course, simplify the code but it would remove a layer of safety that I don't think is a good idea to remove.

On an unrelated note, I noticed you also replaced all relative imports with absolute imports. I am not sure what is the reason behind this.

chainside / btcpy