search

MetacoSA / NBitcoin

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

Added LitecoinUrlBuilder #1064

Closed su8898 closed 3 years ago

su8898 commented 3 years ago

Litecoin also supports BIP0021

NicolasDorier commented 3 years ago

Rather than making new type, just add a new property in the Network class UrlScheme which default to bitcoin, and make the BitcoinUrlBuilder use this instead of hardcoded bitcoin.

knocte commented 3 years ago

Rather than making new type, just add a new property in the Network class UrlScheme which default to bitcoin, and make the BitcoinUrlBuilder use this instead of hardcoded bitcoin.

Mmm, but there is a constructor (the empty one) that doesn't receive a Network object. Should we have a default Network object in this case?

NicolasDorier commented 3 years ago

@knocte I think we should default to bitcoin mainnet and [Obsolete] if no network is passed.

knocte commented 3 years ago

@NicolasDorier I had a look into this today and I got very confused, because Litecoin class is not a Network subclass, but a NetworkSetBase subclass. On top of that, as far as I understand, the subnetworks of the same currency would share the same UrlScheme so I don't understand how it would help placing this property there (in the Network class, as you suggest), and I wouldn't know where to override the value of this property in the litecoin case.

Please let us know how to deal with the above so that we can move this forward.

knocte commented 3 years ago

Ok I had another look, and looks like this approach would work and would be more in line with what you suggested @NicolasDorier: have the UrlScheme property in INetworkSet, which would be accessible thanks to the NetworkSet property of the Network class. This would have the default value of "bitcoin", which could be overriden by the Litecoin class. If you like it, let us know and we will amend.

NicolasDorier commented 3 years ago

I prefer not adding to interfaces, adding things to interface is a big breaking change and impose lot's of work on the implementers. I just make another PR. #1067

NicolasDorier commented 3 years ago

made new package version