MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.88k stars 847 forks source link

Network class should have Blockchain and APIPort properties #309

Closed andrasfuchs closed 6 years ago

andrasfuchs commented 6 years ago

Looking at the Stratis source code, it would be good to extend the Network class with two properties:

string/enum Blockchain - indicating if the network is Bitcoin, Stratis, or something else (Main and Testnet would have the same value)

int RESTAPIPort - indicating the default port number where the daemon's REST API is published (similarly to RPCPort for RPC)

Both would benefit any future forks as well.

NicolasDorier commented 6 years ago

I would like to support a more extensible way for the Network class. (Like a Network.Get/SetExtension<T>()`)

string/enum Blockchain - indicating if the network is Bitcoin, Stratis, or something else (Main and Testnet would have the same value)

Unsure what you mean, the Network.Name reflects already the chain. Also, the Network object can itself be used to compare what chain it is.

andrasfuchs commented 6 years ago

The extensions sound good!

Well, I probably used a wrong property name, but I thought that it would be beneficial, if BitcoinMain and BitcoinTestNet and BitcoinRegTest networks had a property to indicate that they are Bitcoin networks. So they all have a enum property telling that they are Bitcoin networks.

Similarly the Stratis networks would have different names for main,testnet and regtest, but they had a property with the same enum value "Stratis".

NicolasDorier commented 6 years ago

I am busy on some other thing so if you want to work on it, I would merge. The best would be a Network.GetExtensions<T>() where T new() internally it would use a ConcurrentDictionary which use GetOrAdd to be sure the extensions are singleton.

If you need that for the stratis project, you would then create an extension called NetworkFamily with a property Value which resolve to an enum of all family supported by Breeze. Such class would not be needed inside NBitcoin.

The way I did until now when I needed additional information is to have a wider Network class (say NetworkInformation) which encapsulate NBitcoin's Network + relevant information. But having extensions as explained above is way better.

andrasfuchs commented 6 years ago

Thank you @NicolasDorier!

Alright, I'll continue with my project, and we'll see. If I really need to implement it, I'll create a pull request for NBitcoin too.

NicolasDorier commented 6 years ago

@andrasfuchs worth mentioning that you can work around that now by having a global dictionary doing it for you, and then you can add extension methods to Network.