Sanity tests are a great tool for debugging the blockchain state. Running them automatically can allow to catch a problem early before it becomes too difficult to manage.
We have to check the consistency of the data of all the storage items of all the pallets, for the moment we check very few, if someone wants to list them, just look at all the occurrences of ::storage in our codebase and in substrate pallets that we use.
The sanity tests are played every 24 hours by a scheduled pipeline on gitlab against my RPC node, and I get an email when they fails.
After batch transfer to addresses with less than existential deposit (@poka’s ĞDev5 did not include the genesis fix) I ran the sanity tests on my node, we still have one error:
running 1 test
Run sanity tests against ĞDev at last best block
accounts: 25848.
identities: 7218.
identity_index_of: 7218.
identities.len() != identity_index_of.len().
Error: Storage corrupted: 1 errors.
thread 'main' panicked at 'assertion failed: `(left == right)`
left: `1`,
right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/4ca19e09d302a4cbde14f9cb1bc109179dc824cd/library/test/src/lib.rs:185:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test main ... FAILED
I think it’s a bug, I will fix it until the end of December.
This was an easy bug (copy-paste typo), but I also have to complete the tests. See MR !146 (wip).
@tuxmain do you see a reason not to use tests with #[test] in the sanity tests and instead implement a strange .assert() method?
I added a sanity test (btw it’s the worst code I’ve seen from elois, he must have been in a hurry when he wrote this).
$ cargo sanity-gdev
Compiling duniter-live-tests v3.0.0 (/home/hugo/dev/duniter-v2s/live-tests)
Finished test [unoptimized + debuginfo] target(s) in 3.52s
Running tests/sanity_gdev.rs (target/debug/deps/sanity_gdev-6170703764f15d0f)
running 1 test
Run sanity tests against ĞDev at last best block
accounts.len(): 25948.
identities.len(): 7098.
identity_index_of.len(): 7096.
identities.len(7098) != identity_index_of.len(7096).
identity number 2457 with owner key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz is mapped to identity index 7238
identity number 3594 with owner key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 is mapped to identity index 3595
Error: Storage corrupted: 3 errors.
test main ... FAILED
failures:
failures:
main
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.77s
error: test failed, to rerun pass `-p duniter-live-tests --test sanity_gdev`
Assertions are for end-user runtime checks on potentially dynamic input (or for enabling compiler optimisations but not in this case), while cargo tests are for testing the code’s behaviour on determined input.
It makes sense to check blockchain state using assertions. The custom assert it just more convenient than the standard panicking assert as it enables seeing all the errors directly.
Ok, but apart from the intended use of cargo tests, it could be more convenient to use a proper test suite instead of custom one.
New test gives:
Run sanity tests against ĞDev at last best block
accounts.len(): 25948.
identities.len(): 7098.
identity_index_of.len(): 7096.
address 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 is the owner_key of 2 identities
address 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz is the owner_key of 2 identities
duplicate key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 at position 3595
duplicate key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz at position 2457
duplicate key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 at position 3594
duplicate key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz at position 7238
identities.len(7098) != identity_index_of.len(7096).
identity number 2457 with owner key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz is mapped to identity index 7238
identity number 3594 with owner key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 is mapped to identity index 3595
Error: Storage corrupted: 9 errors.
Two addresses have multiple identities. This should never happen. @ManUtopiK is the other one.
This problem was already present at the genesis:
$ WS_RPC_ENDPOINT=wss://gdev.coinduf.eu:443/ws AT_BLOCK_NUMBER=0 cargo sanity-gdev
Run sanity tests against ĞDev at block #0.
accounts.len(): 25843.
identities.len(): 7220.
identity_index_of.len(): 7218.
Account 5HJJL5kZWVPwz1Wzgrwosee24tWw87aBB7AgGhJbZXhyZKBr not respect existential deposit rule.
[...]
Account 5DTAhrSz26tRe79WnciL1grHWXF9AHEcABMPBbXew23UX1Mc not respect existential deposit rule.
address 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 is the owner_key of 2 identities
address 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz is the owner_key of 2 identities
duplicate key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 at position 3595
duplicate key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 at position 3594
duplicate key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz at position 2453
duplicate key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz at position 2457
identities.len(7220) != identity_index_of.len(7218).
identity number 3594 with owner key 5DUjwHRqPayt3tAZk1fqEgU99xZB9jzBHKy2sMSTNcc7m9D1 is mapped to identity index 3595
identity number 2453 with owner key 5Dq8xjvkmbz7q4g2LbZgyExD26VSCutfEc6n4W4AfQeVHZqz is mapped to identity index 2457
Error: Storage corrupted: 891 errors.
so we have to find out whether it’s a problem of genesis parsing or genesis produced by @poka’s june-migrator.
[edit] I do not see any problem in https://g1-migrator.axiom-team.fr/genesis/gdev.json data. It could have been introduced later (for example a copy-paste error when adding session keys), or less likely a parsing issue. We should look in raw chainspecs for that, but that’s not convenient
About Manutopik, this is my fault, he was present 2 times in smiths array, removed just after here.
I thought it was ok if genesis parsing says all good, that it took only once. But I don’t know about your idtyIndex, this human error was only about ManutopiK as I know. We should check it in rawspec but I don’t know how to interpret this format yet…
We should probably add theses kind of test in genesis parsing, no ?