MetacoSA / NBitcoin

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

Dash support? #323

Closed coinpennant closed 6 years ago

coinpennant commented 6 years ago

Do you support also Dash?

Thank you

NicolasDorier commented 6 years ago

If Dash transaction have same shape as BTC transaction there it is doable. I accept PRs, take example on https://github.com/MetacoSA/NBitcoin.Litecoin

coinpennant commented 6 years ago

Do you have time to test it?

https://github.com/coinpennant/NBitcoin.Dash

NicolasDorier commented 6 years ago

I don't. Dash proof of work is scrypt?

logikonline commented 6 years ago

@coinpennant did you test it? Is it forming the addresses correctly?

logikonline commented 6 years ago

@coinpennant I confirmed it is creating the keys and addresses correctly. I use a 3rd party processor to send my transactions, so I cannot confirm if the library sends but Thank you for what I use the extension for and am able to confirm works.

NicolasDorier commented 6 years ago

@coinpennant can you check if you can connect to RPC and P2P network, also are you sure Dash is PoW ? If you are not sure, just remove that, we can improve later.

senzacionale commented 6 years ago

@logikonline which 3rd party processor did you use for tests?

senzacionale commented 6 years ago

@NicolasDorier DASH is not a truly POS but its masternodes work kind off in a POS way.

NicolasDorier commented 6 years ago

So remove https://github.com/coinpennant/NBitcoin.Dash/blob/master/NBitcoin.Dash/DashNetworks.cs#L100

logikonline commented 6 years ago

@senzacionale I use blockcypher.com

I will be conducting my tests this week but I only use this to generate keys. Wish someone would write one for Ripple too

coinpennant commented 6 years ago

@NicolasDorier

Block production in Dash is achieved by proof of work. That is to say miners need to produce valid blocks at the required difficulty to add the next block to the chain.

Within each block, the miner need to include a coinbase tx splitting 45% of the reward to the next winning Masternode from the Masternode list and this is where some people misunderstand Dash as proof of service consensus.

Whilst it's true that part of gaining entry into the Masternode list is proving ownership of 1000 Dash in a UTXO, the Masternode also needs to be providing service to the network on a running fullnode referencing that collateral to be enabled in the MN list.

Therefore, proving a stake alone will not enable any node to receive rewards as in a proof of stake coin, and neither does proving that stake give you any ability to produce a block or decide the contents of a block.

So no, I don't think stating Dash as a POS coin is in anyway a good definition of what is inherently a mined proof of work cryptocurrency.

NicolasDorier commented 6 years ago

Fine for me, just remove stuff no related to Dash like https://github.com/coinpennant/NBitcoin.Dash/blob/master/NBitcoin.Dash/DashNetworks.cs#L100

coinpennant commented 6 years ago

@NicolasDorier done. I removed this part.

dimsumcode commented 6 years ago

@coinpennant thank you for creating this! I tested it out and was able to do a full transaction create/send on TestNet. I'll try to commit some test cases soon.

coinpennant commented 6 years ago

@dimsumcode thank you!

just for info, did you use some 3rd party processor for testing testnet?

dimsumcode commented 6 years ago

@coinpennant no I did everything using NBitcoin plus some RPC commands to the Dash Core Wallet (node).

dimsumcode commented 6 years ago

BTW, I think Dash uses X11, not Scrypt. So the Networks.GetPoWHash() probably needs to be changed. I don't know how to do this yet. If anyone knows, please do.

NicolasDorier commented 6 years ago

@dimsumcode the GetPoWHash is important. Parts of NBitcoin check the PoW for internal stuff. If it is not possible, an idea would be to change NBitcoin to allow to have a Network which skip PoW check.

dimsumcode commented 6 years ago

@coinpennant I added some sample code and submitted PR . @NicolasDorier yes GetPowHash is important no doubt. So I will try to research this and find out how hard it is to implement. If too hard, then we can do as you propose to skip PoW check for now.

coinpennant commented 6 years ago

@dimsumcode thank you. PR accepted.

dimsumcode commented 6 years ago

@NicolasDorier I think approach to implement X11 is like this:

Copy X11 algorithm from here: https://github.com/bonesoul/CoiniumServ/blob/develop/src/CoiniumServ/Algorithms/Implementations/X11.cs

Add dependency in NBitcoin to HashLib: https://www.nuget.org/packages/HashLib/

Does this look right to you?

NicolasDorier commented 6 years ago

@dimsumcode seems good to me. You can test by bootstrapping a Dash Node, and connect to it with Node.Connect then Node.GetChain to get the whole chain of headers.

dimsumcode commented 6 years ago

@NicolasDorier I created a branch that has all this implemented. But it seems that it is forever running in the Node.GetHeadersFromFork() big while loop in Node.cs.

https://github.com/dimsumcode/NBitcoin.Dash/tree/X11

To run the Unit test, just run a Dash Core Wallet like this first:
dash-qt.exe -server -testnet -port=19999 -debug

NicolasDorier commented 6 years ago

@dimsumcode can you wait the loop finish?

Because after getting all the headers, it checks the PoW. Also why the port is needed?

coinpennant commented 6 years ago

@dimsumcode can I merge your changes?

dimsumcode commented 6 years ago

@coinpennant I think we should wait until we can test the PoW function (which I haven't been able to successfully yet). But you could definitely pull the branch local and test it out.

@NicolasDorier the port=19999 is redundant it can be left out if you want. Is it possible to have it check PoW without having to get all headers? I let it run for a few hours and gave up. FYI it always gets caught in this exception catch block:

try
{
  headers = listener.ReceivePayload<HeadersPayload>(headersCancel.Token);
}
catch(OperationCanceledException)
{
  acceptMaxReorgDepth += 6;
  if(cancellationToken.IsCancellationRequested)
    throw;
  break; //Send a new GetHeaders
  // It always gets here over and over in the loop. Is this normal?
}

I also attach a sample extracted from debug.log showing communication between Dash node and our code. Maybe it looks normal or not? logsample.txt

NicolasDorier commented 6 years ago

@dimsumcode I was asking about the port because nbitcoin should use the default that you configure in the NetworkBuilder.

Getting the headers should not take that long. (maybe 1 min maximum) It can be in this loop if there is a big reorg. It can also happen if your node is still synching.

dimsumcode commented 6 years ago

@NicolasDorier I think I know what the problem is. BlockHeader.GetHash() is using Sha256 but Dash uses X11 for hashing. So the genesis block never hashes to the value we set in Networks.Register() and the loop can never operate as intended. This would be a significant effort to fix because we'd have to change some internals in the main NBitcoin library. I don't really want to mess with that yet. Maybe you have some good ideas?

NicolasDorier commented 6 years ago

@dimsumcode yeah I don't want that either this would be a major redesign.

The way Litecoin works is that they kept SHA256d for block and transaction id, only the PoW hash calculation was changed so nothing break. Are you sure Dash is not doing the same?

If Dash is not doing the same, it severly limit the usefulness of NBitcoin.Dash outside of address parsing, key generation, signature creation and RPC calls, as tools working for Litecoin/Bitcoin will not work with dash without recompiling everything with a modified version of NBitcoin for Dash.

dimsumcode commented 6 years ago

@NicolasDorier I agree we should not pursue this more. Dash doesn't follow Litecoin's approach and they definitely use X11 for block hash (see https://github.com/dashpay/dash/blob/master/src/primitives/block.cpp)

I think we can just use the NBitcoin.Dash for the limited things you mention. I was able to create and sign transactions on Dash network so that's pretty useful in itself.

@coinpennant so we don't really need to merge the PoW function since it will be rendered somewhat useless. I guess the only option would be to fork NBitcoin itself and create a modified version that supports X11 hashing. I wouldn't want to try and retrofit that into the main library for fear of breaking it. I won't really have time to do this so if anyone else is motivated for Dash, please lead the way :)

NicolasDorier commented 6 years ago

I think we can just use the NBitcoin.Dash for the limited things you mention. I was able to create and sign transactions on Dash network so that's pretty useful in itself.

I agree, actually NBitcoin.Dash can be used in BTCPay (not in NBxplorer) as there is nothing more than address parsing and transaction parsing, so this is useful.

I advise to remove the PoW stuff.

I plan later to make a new repo where I centralize every altcoins together, and expose it as a nuget package.

dimsumcode commented 6 years ago

@NicolasDorier Sounds good. None of my code related to PoW was ever merged so it should be okay. It's just orphaned off into a branch.

Yeah your BTCPay is really awesome and was mentioned on WCN recently. Nice work! Looking forward to the altcoin repo later.

coinpennant commented 6 years ago

That is great. Then I will remove everything related to PoW. Thank you both for helping and testing!

shtse8 commented 6 years ago

Any updates on this?

nopara73 commented 6 years ago

@shtse8 After this issue is resolved you may be able to build your Dash Framework with NBitcoin: https://github.com/MetacoSA/NBitcoin/issues/371

NicolasDorier commented 6 years ago

BTW, NBXplorer dropped all dependency to PoW verification. This mean that if Dash have:

Then the integration to NBXplorer is pretty easy. If one of those is not true, as @nopara73 said, we need to work on it.

shtse8 commented 6 years ago

I am looking forward to it. I want to build a single node for watching all bitcoin family coins. (just want to get the latest blocktemplate or some transaction information.) at the moment, i need to setup each daemon one by one and some daemons are suck. so im going to build by myself and hope i can use this project as my base and i try to implement each for extension.

NicolasDorier commented 6 years ago

Done https://github.com/MetacoSA/NBitcoin/pull/393