Sync broken on release-0.10 with segwit enabled

[ihavenoface]

Just copy-pasting from discord here.

It happens on testnet, and I'm guessing main net as well.
The sequence I'm trying goes like so (this is all clean slate, with fresh profile dirs);

• start one node
• exec getnewaddress
• exec generatetoaddress 1 <address> (this can be any number of blocks)
• start a second node and connect it to the first one
• get this in debug on the first node:

2021-03-11T15:01:15Z received getdata (1 invsz) peer=0
2021-03-11T15:01:15Z received getdata for: witness-block 17821b487854a6930811b560bc3ca3d6d1b59e8363f6d53f144e3b2e05677cac peer=0

• get this in debug on the second node (or any other nodes connecting later on):

2021-03-11T15:01:15Z Ignoring getheaders from peer=0 because node is in initial block download
2021-03-11T15:01:15Z received: headers (87 bytes) peer=0
2021-03-11T15:01:15Z Synchronizing blockheaders, height: 1 (~100.00%)
2021-03-11T15:01:15Z Requesting block 17821b487854a6930811b560bc3ca3d6d1b59e8363f6d53f144e3b2e05677cac (1) peer=0
2021-03-11T15:01:15Z sending getdata (37 bytes) peer=0
2021-03-11T15:01:15Z received: block (171 bytes) peer=0
2021-03-11T15:01:15Z ProcessMessages(block, 171 bytes): Exception 'CDataStream::read(): end of data: iostream error' (NSt8ios_base7failureB5cxx11E) caught
2021-03-11T15:01:15Z ProcessMessages(block, 171 bytes) FAILED peer=0

Somewhat amusingly; when running the nodes at the same time this will only occur once every other reset.
But as soon as you close the first peer and re-start it with a fresh profile, it starts failing again.

Now given that this seems to be vaguely related to segwit I have tried this patch:

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0d98b5c67..6d50cfff7 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2062,7 +2062,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         if((nServices & NODE_WITNESS))
         {
             LOCK(cs_main);
-            State(pfrom->GetId())->fHaveWitness = true;
+            State(pfrom->GetId())->fHaveWitness = false;
         }
 
         // Potentially mark this peer as a preferred download peer.
diff --git a/src/validation.cpp b/src/validation.cpp
index 773e00266..bbed352e8 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3197,6 +3197,7 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu
 
 bool IsWitnessEnabled(const CBlockIndex* pindexPrev, const Consensus::Params& params)
 {
+    return false;
     return pindexPrev ? IsBTC16BIPsEnabled(pindexPrev->nTime) : false; // pindexPrev == null on genesis block
 }
 
@@ -3385,7 +3386,7 @@ static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& stat
             if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) {
                 return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "bad-witness-merkle-match", strprintf("%s : witness merkle commitment mismatch", __func__));
             }
-            fHaveWitness = true;
+            fHaveWitness = false;
         }
     }
 

That makes it work, but it seems to be like a bad idea to just disable segwit for what appears to be a minor networking issue.

This same sequence does not fail on the release-0.9 branch.

[backpacker69]

can you please check on the latest 0.10?

[ihavenoface]

IIRC I tested with DEFAULT_ADDRESS_TYPE{OutputType::LEGACY} and it did not work, but yes I'll try again, just to make sure.

[backpacker69]

i think issue only affected testnet and case when chain is recreated and synched using headers only sync, so the fix of copying nFlags to blockindex from header could have effect on the subject issue.

[ihavenoface]

Testing on the latest commit gives the same issue described above.

There appears to be a small chance a block gets through to the connecting node, but this results in this error being logged

2021-03-27T12:34:17Z Requesting block 7db91a16b4f6280e436c286d9471a6ed38c7ec07a1cf50af8b552fceb5e811e7 (1) peer=0
2021-03-27T12:34:17Z sending getdata (37 bytes) peer=0
2021-03-27T12:34:17Z received: block (174 bytes) peer=0
2021-03-27T12:34:17Z received block 7db91a16b4f6280e436c286d9471a6ed38c7ec07a1cf50af8b552fceb5e811e7 peer=0
2021-03-27T12:34:17Z BlockChecked: block hash=7db91a16b4f6280e436c286d9471a6ed38c7ec07a1cf50af8b552fceb5e811e7 state=bad-txnmrklroot, hashMerkleRoot mismatch
2021-03-27T12:34:17Z Misbehaving: seed peer=0 (3436900 -> 3437000)
2021-03-27T12:34:17Z ERROR: ProcessNewBlock: AcceptBlock FAILED (bad-txnmrklroot, hashMerkleRoot mismatch)

[ihavenoface]

I just realized that I should actually test this on main net as well. I doubt anyone would have fun with fresh 0.10 nodes mining / minting new blocks and then potentially being unable to share them to other fresh 0.10 peers.
Do you reckon it's fine to allow any difficulty when generating blocks or would that somehow mess with the results?

[peerchemist]

Please do same testing procedure on mainnet with rc5 and report back. Thank you.

[ihavenoface]

The same procedure requires me to produce new blocks, which will not be possible on current main net difficulty, unless you have a couple of months to spend.

[peerchemist]

Do you experience sync issues with rc5 on mainnet maybe?

[ihavenoface]

I do not I have not fully tested sync with the new client on the main net yet, but how many similar nodes (running rc5) on the mainnet are there?

I believe the main concern with this issue right now is that's unclear where it stems from. It's likely a relay issue, but what if it happens right at block creation?

[peerchemist]

but how many similar nodes (running rc5) on the mainnet are there?

I see three here: https://chainz.cryptoid.info/ppc/#!network

More rc5 nodes should come online during this week.

[ihavenoface]

I just ran https://github.com/peercoin/electrumx-docker with no modifications for ease of use and here is some output:

peercoind_1  | 2021-03-31 08:00:58 Misbehaving: 178.49.167.228:9901 peer=4 (0 -> 100) BAN THRESHOLD EXCEEDED
peercoind_1  | 2021-03-31 08:00:58 ERROR: ProcessNewBlock: AcceptBlock FAILED (hashMerkleRoot mismatch)
peercoind_1  | 2021-03-31 08:00:58 Misbehaving: 104.54.203.174:9901 peer=7 (0 -> 100) BAN THRESHOLD EXCEEDED
peercoind_1  | 2021-03-31 08:00:58 ERROR: ProcessNewBlock: AcceptBlock FAILED (hashMerkleRoot mismatch)

Looking at https://chainz.cryptoid.info/ppc/#!network right now the node 104.54.203.174 is running /Satoshi:0.20.1/Peercoin:0.10.0(v0.10.0ppc.rc5 Scarab)/ so this seems in line with #557 (comment).

[sandakersmann]

RC5 does not include this commit: 4c36519

[peerchemist]

@ihavenoface do you have any further reports?

[ihavenoface]

@peerchemist
I haven't had the time to look into this further up until now, but try running a 0.9 node now, and see it banning 0.10 nodes upon sight:

This whole thing seems sporadic if anything. I have synced a 0.10 node just yesterday, launched it again today, only to find no connections. Only after a restart the 0.10 node was able to find peers again.

[7h0m45]

I'd like to attest to this issue as well. It looks like a critical problem. In my experience v0.9 nodes inevitably fail to sync with v0.10 nodes. I agree that it seems sporadic so it's difficult to pinpoint the problem. It's easy to demonstrate though if you attempt to launch a v0.9 node only connected to v0.10 nodes.

[7h0m45]

@peerchemist

I'm concerned from your remark on Discord that you are no longer tracking this issue. It's significant and reproducible. The protocol layer may still be stable but the network layer is broken.

v0.9 nodes are banning v0.10 nodes for the following:
ERROR: ProcessNewBlock: AcceptBlock FAILED (proof of work failed)

Currently, using the official builds, it appears that v0.9 nodes will only accept new blocks from other v0.9 nodes.

[backpacker69]

@peerchemist

I'm concerned from your remark on Discord that you are no longer tracking this issue. It's significant and reproducible. The protocol layer may still be stable but the network layer is broken.

v0.9 nodes are banning v0.10 nodes for the following:
ERROR: ProcessNewBlock: AcceptBlock FAILED (proof of work failed)

Currently, using the official builds, it appears that v0.9 nodes will only accept new blocks from other v0.9 nodes.

can you please try building latest 0.9 branch to see if it solves the problem?

[ihavenoface]

I have cherry-picked that commit to my repo. Builds are running here https://gitlab.com/noface/peercoin/-/pipelines/306456014 and will be available here https://gitlab.com/noface/peercoin/-/releases/v0.9.0initNflags once done.

Obviously the usual safety-precautions should be taken, so run those in a VM, if you choose to use them.

[ihavenoface]

Still getting

2021-05-20 11:01:49 Misbehaving: 176.37.82.11:9901 peer=1 (0 -> 100) BAN THRESHOLD EXCEEDED
2021-05-20 11:01:49 ERROR: ProcessNewBlock: AcceptBlock FAILED (hashMerkleRoot mismatch)

with that build.
And this right off the bat.

[backpacker69]

Still getting

can you please check with latest commit in release-0.10?

[7h0m45]

All systems nominal on v0.10.1
Thank you!

[ihavenoface]

Looks good on my end.
Feel free to close this as you see fit.

[ihavenoface]

Or maybe not.
I'm getting 2021-05-28T23:01:56Z ERROR: AcceptBlockHeader: Consensus::CheckBlockHeader: 514e141ef5778299506b779145c9c94d78b0603281246741cb31f112c6635365, high-hash, proof of work failed on sync now and when trying to import a bootstrap.dat created with the same client version (0.10.1).

[backpacker69]

can you please provide step by step instructions to reproduce, this shouldn't happen on the correct 0.10.1 tag

[ihavenoface]

1. Go to https://github.com/peercoin/peercoin/releases/tag/v0.10.1ppc
2. Select peercoin-0.10.1-win64-setup-unsigned.exe and download it
3. Open %appdata% in Windows Explorer, find and delete the directory called Peercoin
4. Execute the downloaded installer
5. Open the recently installed Peercoin application
6. Let it sit syncing for a while
7. Open %appdata%\Peercoin\debug.log
8. Notice the error mentioned above or in this log https://gitlab.com/noface/peercoin/-/jobs/1303009055#L380
9. Download the produced bootstrap.dat ( https://gitlab.com/noface/peercoin/-/jobs/1303009055/artifacts/browse ) and use that in conjunction with -loadblock on yet another fresh node, in a vm with no networking enabled to have it getting stuck at around blockheight ~6000

This all works fine on 0.9.

[peerchemist]

Problem can be in that boostrap you use.

[ihavenoface]

Most certainly so. But given that it is produced by the 0.10.1 client, and then fails to be reimported by that same client, I think it's reasonable to assume that there is something afoot when it shouldn't be.
That is, either in the bootstrap generation, or higher up in the client that writes the blocks beforehand.

[ymgve]

Might be connected to a serialization issue I'm seeing - I noticed that a node with version "/Satoshi:0.20.1/Peercoin:0.10.0(v0.10.0.0-0e0fa7bbc Scarab)/" and service flags 1033 would give this data when requesting the genesis block: 010000000000000000000000000000000000000000000000000000000000000000000000c293592c05905698290c89eb6ddef0cf8aa5a148c68c55ac7ad1b4fa858f2d3c7f5b2c50ffff001dab82e5810101000000a2592c50010000000000000000000000000000000000000000000000000000000000000000ffffffff5404ffff001d020f274b4d61746f6e69732030372d4155472d3230313220506172616c6c656c2043757272656e6369657320416e642054686520526f61646d617020546f204d6f6e65746172792046726565646f6dffffffff010000000000000000000000000000

while a node with version '/Satoshi:0.20.1/Peercoin:0.10.0(v0.10.1ppc Scarab)/' and service flags 1033 would give this data:
010000000000000000000000000000000000000000000000000000000000000000000000c293592c05905698290c89eb6ddef0cf8aa5a148c68c55ac7ad1b4fa858f2d3c7f5b2c50ffff001dab82e581000000000101000000a2592c50010000000000000000000000000000000000000000000000000000000000000000ffffffff5404ffff001d020f274b4d61746f6e69732030372d4155472d3230313220506172616c6c656c2043757272656e6369657320416e642054686520526f61646d617020546f204d6f6e65746172792046726565646f6dffffffff010000000000000000000000000000

The difference seems to be that the former doesn't include the header flags in the serialization for some reason.

[backpacker69]

and the reason for it c2cb099