Skip to content

Conversation

@jtimon
Copy link
Contributor

@jtimon jtimon commented Jun 4, 2015

Several improvements related to chainparams were accumulating in #5229, but they don't need to add a new option for them to make sense, so I'm taking them ot for discussion.
Since there are several somewhat-orthogonal changes here, the scope of the PR will probably be reduced after discussion.
There are several changes:

  • The use of a more conventional factory is interesting for separating the stateless parts of chainparams later like @theuni planned to do (the stateless factory could remain where it is).
  • Replacing the enum with constant strings (suggested by @laanwj ) may not seem very useful by itself, but it allows to unify some things (also simplifies things for the factory).

The most controversial thing is proposing to unify some things for testnet that are already unified for regtest.
The first step would be to replace the user-facing strings (including windows' direct access names) to fit the chain names described in bip70 (use "test" instead of "testnet").
We could also unify the default name for the data directory. That would mean either change the directory file from "testnet3" to "test" (not retro-compatible) or changing the chain name "test" to "testnet3" in BIP70. I don't understand why "test" was chosen instead of "testnet" for bip70 in the first place so my preference is the later.
For the user-facing strings, that would be like having changed directly from "testnet" to "testnet3" (without passing through the current bip70-compatible "test").

Copy link

Choose a reason for hiding this comment

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

IMHO there are no regtest payment requests ;).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe that's the reason, but why not support it for free then?
I can further simplify the code but not making a special case of regtest here. Besides, if it needs to be a special case then we need another field in chainparams, not treating regtest specially without doing so explicitly.

@jtimon jtimon force-pushed the chainparams-bip70 branch from ed6278b to 8c415c6 Compare June 25, 2015 19:50
@jtimon jtimon force-pushed the chainparams-bip70 branch from 8c415c6 to edc08a3 Compare June 25, 2015 19:53
@jtimon
Copy link
Contributor Author

jtimon commented Jun 25, 2015

EDIT:
Closing until it can be expressed in a few lines some #6359 or a subset of it is merged.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants