how to test for storage leaks in pallets

Question

Storage leaks in runtimes can be very damaging and cleaning up with a runtime update migration is non-trivial. I'd like to ensure the absence of storage leaks already in unit testing.

My strategy would be:

1. measure size of TestExternalities
2. execute a few dispatchables
3. call a cleanup dispatchable that is supposed to purge all storage that has been created by the last step
4. measure size of TestExternalities again and compare

How can I measure state size before and after a unit test?

Would it even be possible to get a diff, such that debugging is more comfortable?

Answer 1

You can use the StorageNoopGuard to assert that your storage changes sum up to a No-OP.
This does the same as assert_storage_noop! but for a code block. It is not measuring the state size, but comparing the root hash of the Trie. This exactly tells us whether something changed.
As example:

#[test]
fn cleanup_works() {
    use frame_support::{StorageNoopGuard, storage::unhashed::*};

    new_test_ext().execute_with(|| {
        let _guard = StorageNoopGuard::default();
        
        // Here you are modifying the storage.
        put(b"key", b"value");
        // And here is the cleanup - which you did not forget ;)
        kill(b"key");
        // The guard will not panic because the state did not change since its construction.
    });
}

It works by calculating the storage root has in the beginning with storage_root(StateVersion::V1) and then again in the end. The state did not change iff they are equal.
You can change the key in kill to b"key1" to simulate an error and see it fail with:

thread 'tests::cleanup_works' panicked at 'assertion failed: `(left == right)`
  left: `[124, 55, 174, 141, 90, 56, 20, 107, 57, 116, 85, 28, 128, 109, 139, 53, 156, 61, 56, 117, 109, 227, 115, 247, 235, 180, 147, 215, 81, 174, 223, 144]`,
 right: `[235, 68, 37, 78, 47, 204, 154, 99, 92, 28, 105, 55, 55, 229, 121, 115, 249, 23, 80, 192, 245, 244, 195, 146, 103, 68, 36, 175, 217, 246, 86, 194]`:
StorageNoopGuard detected wrongful storage changes.'

I don't think there is a nice state-differ available in FRAME. But it could be done, by storing all keys and traversing the tree and following the differing keys.

PS: I think these stale-state errors happen the easiest when the pallet is refactored, not when it is originally created. Anyway, probably better than not doing it.