Dash DIP 3 testnet - Githubissues

pozdnyakovasp commented 5 years ago

Hi.

I try work with Dash core testnet (13.0.0 cr6)

RpcClient.GetBlock(273999);  - OK

// After block 273999 i get exception
RpcClient.GetBlock(274000); next exception

в NBitcoin.BitcoinStream.ReadWriteBytes(Byte[]& data, Int32 offset, Int32 count) в NBitcoin.BitcoinStream.ReadWriteNumber(UInt64& value, Int32 size) в NBitcoin.BitcoinStream.ReadWrite(UInt32& data) в NBitcoin.TxIn.ReadWrite(BitcoinStream stream) в NBitcoin.BitcoinStream.ReadWrite[T](T& data) в NBitcoin.BitcoinStream.ReadWriteArray[T](T[]& data) в NBitcoin.BitcoinStream.ReadWriteList[TList,TItem](TList& data) в NBitcoin.Transaction.ReadWrite(BitcoinStream stream) в NBitcoin.BitcoinStream.ReadWrite[T](T& data) в NBitcoin.BitcoinStream.ReadWriteArray[T](T[]& data) в NBitcoin.BitcoinStream.ReadWriteList[TList,TItem](TList& data) в NBitcoin.Block.ReadWrite(BitcoinStream stream) в NBitcoin.BitcoinSerializableExtensions.ReadWrite(IBitcoinSerializable serializable, Stream stream, Boolean serializing, ConsensusFactory consensusFactory, Nullable`1 version) в NBitcoin.Block.Parse(String hex, ConsensusFactory consensusFactory) в NBitcoin.Block.Parse(String hex, Network network) в NBitcoin.RPC.RPCClient.d87.MoveNext() --- Конец трассировка стека из предыдущего расположения, где возникло исключение --- в System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) в System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) в NBitcoin.RPC.RPCClient.d90.MoveNext() --- Конец трассировка стека из предыдущего расположения, где возникло исключение --- в System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) в System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) в NBitcoin.RPC.RPCClient.GetBlock(Int32 height)

Does it need a fix in NBitcoin.Altcoins?

NicolasDorier commented 5 years ago

Ping @bazooka70 , @bdangh , @snogcel , somebody can fix it?

snogcel commented 5 years ago

Thanks for posting this issue @pozdnyakovasp and for the heads-up @NicolasDorier.

It does appear a fix will be needed for NBitcoin, we've very recently completed similar work for dashcore-lib (forked from bitcore-lib) seen here: https://github.com/dashevo/dashcore-lib/commits/fix-collateral.

I suspect that it's getting stuck parsing new block components (Special Transactions), for example: https://github.com/dashevo/dashcore-lib/blob/73b0fa438b9697d1f9a621d8abe1e72cb7e1a1ec/test/transaction/transaction.js#L1328-L1342. I'm not a .NET developer myself but can provide guidance on this task. @bazooka70 @bdangh do either of you two have the bandwidth to look at this?

snogcel commented 5 years ago

More details on compatibility / implementation here: https://github.com/dashpay/dips/blob/master/dip-0002.md#compatibility

bazooka70 commented 5 years ago

Sorry guys. Im on vacation and will be back to civilization only next year :)

snogcel commented 5 years ago

Enjoy your vacation @bazooka70!

As a quick update to anyone tracking this issue -- no deployment date has been set for Dash 13.0 on mainnet so we have some time yet to fix this. I'm currently examining NBitcoin test coverage to figure out everything affected and scope of work involved, will keep you all posted.

BenjaminNitschke commented 5 years ago

WIll take a look and fix if easy to reproduce

BenjaminNitschke commented 5 years ago

@pozdnyakovasp @NicolasDorier @snogcel ok, fixed with a dummy hack to support the new extraPayloadSize at the end of txs, more details in my fork, but you probably don't want all my messy test code: https://github.com/DeltaEngine/NBitcoin

Anyway, the current Dash testnet only goes to block 7950, but the same issue as the poster here starts happening in block 7000 when "extraPayloadSize": 38, is used for the first time. Many blocks after that also use it, currently we just check for nVersion => 65539, which is 4 bytes: 3 for version, 0, 1 or 5 mostly, 0 (just poking in the dark, the console commands say extraPayloadSize should be int, but it is in reality mostly a byte or 1 byte+1 short). Here is the testnet explorer to check: https://testnet-insight.dashevo.org/insight/

And finally the only code that needs to change for this hack to work (no more crashes), the real implementation will be done later, but no crashes is good enough for now: Transaction.cs:1443 (very end of the Transaction.ReadWrite method)

// Dummy support for Dash 0.13 extraPayloadSize (1-3 byte)+ extraPayload (based on length of extraPayloadSize)  
if (nVersion >= 65539)//see above, might also be: 327683 or higher  
{  
    // Extra payload size is a byte if below 253, but 1+2 bytes (short) if bigger, doc says int, but it isn't  
    byte extraPayloadSize = 0;  
    stream.ReadWrite(ref extraPayloadSize);  
    ushort totalExtraPayloadSize = extraPayloadSize;  
    if (extraPayloadSize == 253)  
        stream.ReadWrite(ref totalExtraPayloadSize);  
    byte dummy = 0;  
    for (int i = 0; i < totalExtraPayloadSize; i++)  
        stream.ReadWrite(ref dummy);  
}

Hope this helps

NicolasDorier commented 5 years ago

so someone just have to change the DashBlock

BenjaminNitschke commented 5 years ago

so someone just have to change the DashBlock

yep, can do that next (probably early 2019), now that I know where the issue is (that did take most time), still not sure about all the other data in 0.13 transactions, probably a few more changes required due to chainlocks and this extra payload data

Cofresi commented 5 years ago

the console commands say extraPayloadSize should be int, but it is in reality mostly a byte or 1 byte+1 short).

@BenjaminNitschke actually extraPayloadSize is a compact unsigned integer https://dash-docs.github.io/en/developer-reference#compactsize-unsigned-integers https://github.com/dashpay/dips/blob/master/dip-0002.md#special-transactions

I'm not sure, because don't have any .net environment, but maybe you can use stream.ReadWriteAsCompactVarInt(ref extraPayloadSize); to work with it?

BenjaminNitschke commented 5 years ago

Makes sense, just saw some docs for the testnet and it was mentioning int, thus I used int and it didn't work, then I tried byte, which worked up till block 7090, then I found out it is var_int in c or CompactVarInt in C#. You are right it is correctly stated in DIP2 and var_int or the compactsize_uint helps (ReadWriteAsCompactVarInt is the correct way to use this datatype). https://www.deadalnix.me/2017/01/08/variable-size-integer-encoding/