The sync verification thread paniced.

[5chdn]

Similar to #441, was running the bitcoin cash branch. Forgot to report.

2017-08-18 15:01:57  INFO sync "15:01:57" Processed 51 blocks in 60.07 seconds. Peers: [active:9, idle:0, bad:1]. Chain: [sch:19088 -> req:419 -> vfy:0 -> stored: 397319]
2017-08-18 15:01:57  WARN sync Failed to get requested block from peer#13 in 25.50230588800332 seconds
2017-08-18 15:02:17  WARN sync Failed to get requested block from peer#13 in 20.00112151200301 seconds
2017-08-18 15:02:38  WARN sync Failed to get requested block from peer#13 in 20.001366622993373 seconds
2017-08-18 15:02:58  INFO sync "15:02:58" Processed 20 blocks in 60.00 seconds. Peers: [active:9, idle:0, bad:1]. Chain: [sch:18960 -> req:527 -> vfy:0 -> stored: 397339]
2017-08-18 15:03:38  WARN sync Failed to get requested block from peer#13 in 24.23408322800242 seconds
2017-08-18 15:03:58  INFO sync "15:03:58" Processed 44 blocks in 60.00 seconds. Peers: [active:6, idle:3, bad:1]. Chain: [sch:18896 -> req:547 -> vfy:0 -> stored: 397383]
2017-08-18 15:04:48  WARN sync Failed to get requested block from peer#13 in 28.738724727998488 seconds
thread 'Sync verification thread' panicked at 'Block 000000000000000001a0d83f073624df259568683bee0dc2b1628b7f231aad5b insertion failed with error CannotCanonize', sync/src/synchronization_client_core.rs:1105:4
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: sync::synchronization_verifier::AsyncVerifier::verification_worker_proc
             at sync/src/synchronization_client_core.rs:1105
             at sync/src/synchronization_client_core.rs:717
             at sync/src/synchronization_verifier.rs:155
             at sync/src/synchronization_verifier.rs:132
Aborted (core dumped)

[svyatonik]

Definitely some --verification-level=none -related issue. Will try to reproduce, but as I mentioned before - this level should never be used in any production system. No verification at all means that non-valid block with non-valid transactions could be inserted to database. And this could lead to errors like this one (and even worse). This implies that you should either have absolute trust in your peers, or be ready to ignore panics or even wipe database at some point.
The only reason why I have added this level is to sync new db asap and drop it after sync is complete.

[5chdn]

Yes I'm using the non-verification to test the bitcoin cash and segwith2x branches.

[svyatonik]

So I have synchronized to ~250k about 20 times without validation in last 3 days.
Haven't seen any issue. Code analysis shows that this error occurs when:

1. block which is currently canonizing, is not in db
2. parent of block which is currently canonizing is not the best block
3. following is true for one of previous outputs for one of canonizing block transactions:
   3.1) either it is not in db
   3.2) or it is from non-canonized block
4. block which is currently decanonizing, is not in db
5. following is true for one of previous outputs for one of block transactions:
   5.1) either it is not in db
   5.2) or it is from non-canonized block

Here are my thoughts on each case:
1 if appending block to canon chain: IMPOSSIBLE because every canonize() call is preceded with insert() call
1 if reorganizing: IMPOSSIBLE because canonization route is built of existing db-blocks
2 if appending block to canon chain: IMPOSSIBLE because CanonChain origin is only returned when block.parent is best block
2 if reorganizing: IMPOSSIBLE because block.parent is the last block in canonization route && parent canonization occurs before this block is canonized
3.1 + 3.2: POSSIBLE when block contains 'bad' transactions
4: IMPOSSIBLE because decanonization route is built of existing db-blocks
5.1 + 5.2: POSSIBLE when block contains 'bad' transactions
It would be great if either @debris, or @NikVolf could check my logic above.

So here are 2 possible cases, which can be merged to single one - someone has provided a bad block to us.
Normally, this block would be rejected by our validation, but in this case it isn't => panic on db level.
So as I said before, it is normal for this case (--validation-level=none) to expect panic-s.

As for solution, I have 3 options (sorted by priority from mine perspective):

1. leave --validation-level=none, but display a big warning at the beginning (like @debris suggested there: SecretStore: bunch of fixes and improvements openethereum/parity-ethereum#6168 (comment))
2. leave --validation-level=none for ony pbtc, built with special flag (something like --features expert)
3. remove --validation-level=none option at all

@5chdn what do you prefer? Maybe you could provide your own option?

[svyatonik]

Also it would be great if you could reproduce this with RUST_LOG=sync=trace,verification=trace,db=trace env var set. This should help to find a reason.

[5chdn]

@5chdn what do you prefer? Maybe you could provide your own option?

Not removing but hiding the --verification-level from the help menu will only expose this option to experts who at least read the code.

I don't think an --i-know-what-i-am-doing flag will help, but I have seen it recently in fdisk, so it seems not uncommon.

Displaying a warning is useful in any case.

Also it would be great if you could reproduce this with RUST_LOG=sync=trace,verification=trace,db=trace env var set. This should help to find a reason.

On it, haven't seen it so far.