Support provhdr on Stratis AltCoin

[mikedennis]

When receive provhdr messages on Stratis AltCoin ask for header using getheader so can sync properly when new blocks are mined.

This involved extending ConsensusFactory to be able to provide custom behaviors to handle the new message.

@dangershony

[NicolasDorier]

Why do you need it btw? It seems those header are only interesting for software who wants to validate the headers. Clients who does not validate header can still use the old headers message right?

And your full node does not depends on NBitcoin.

[mikedennis]

Yes the client can still use the old headers message but new blocks are only announced using proven headers. Without this code the client doesn't get the headers for new blocks as their mined, but if it asks for it then it will get it.

[NicolasDorier]

new blocks are only announced using proven headers you mean inv messages are not sent?

[mikedennis]

This came up trying to get CanScanUTXOSet() test to work in NBXplorer. This was failing when running strat node

tester.RPC.EnsureGenerate(1);
tester.WaitSynchronized(); 

I did some logging and this is the P2P when BTC runs those lines:

TEST [0] Receiving message: inv (MSG_TX)
TEST [0] Sending message getdata : GetDataPayload
TEST [0] Receiving message: tx (TxPayload)
TEST [0] Receiving message: headers (HeadersPayload)
TEST [0] Sending message getheaders : GetHeadersPayload
TEST [0] Sending message getdata : GetDataPayload
TEST [0] Receiving message: headers (HeadersPayload)
TEST [0] Receiving message: block (BlockPayload)

And this is what was happening with the strat node:

TEST [0] Receiving message: inv (MSG_TX)
TEST [0] Sending message getdata : GetDataPayload
TEST [0] Receiving message: tx (TxPayload)
TEST [0] Receiving message: tx (TxPayload)
TEST [0] Unknown command received provhdr
TEST [0] Receiving message: provhdr (UnknowPayload)

[NicolasDorier]

You should definitively announce blocks with header first.
Stratis should be able to detect if the client does not support provhdr (typically with the bytes) and fall back on header first.

Basically so much software is built on the assumption that header first exist that even with this hack, you will get problem down the road. This need to be fixed at Stratis node level.

[NicolasDorier]

Another idea:

Instead of using a behavior, use a node.Filters which convert the provhdr into a normal header message down the stream.

Filters have been done for this, and it means you don't have to add a new type of payload.

[mikedennis]

Yes node.Filters sounds exactly what I need. I will give them a try and update this PR if I can get it to work. Thanks!!

[mikedennis]

Filters looks like they would work but I have a problem. Filters are implemented using a ConcurrentDictionary which does not guarantee the ordering of the items. I need my filter to be executed first but because SlimChainBehavior and the other chain behaviors are also implemented using filters. Their filter gets executed before my filter gets a chance to preprocess it even though mine is added first. I have no way of ensuring my filter is the first to be executed.

I think we need to replace the collection used for filters (ie ThreadSafeCollection) with something else so the order can be defined and the sequencing of filters is predictable and controllable? What do you think? Is there anything like that already in use in NBitcoin?

[NicolasDorier]

@mikedennis yes I agree. Or refactoring ThreadSafeCollection to be order conscious.

[mikedennis]

#774

[dangershony]

Straits fullnode also has a way to use header first for regular header only using the gateway flag (the flag we use to bridge old and new protocols), this flag basically sets the node to trust the headers that are sent form a peer (the whole idea of proven headers is that the extra information to verify the header is sent together with the header itself) I assume interactions of NBitcoin libs with a node will only use trusted peers (as NBitcoin cannot verify consensus rules).

Perhaps using the gateway flag will be sufficient (I would even rename it to something that is not explicitly used for gateway nodes)

Note: this flag may needs to come with a -whitelist and -addnode arguments