core/state: fix database leak tests

[zcheng9]

The Leak Test intend to demonstrate state.IntermediateRoot doesn't actually write cache into cacheDB, while Commit did. However,in both test, it did not commit the trie database to persistent database. Which means the persistent database will always be empty in any scenario. Thus, the original test would always iterator empty persistent database no matter you use state.Commit or state.IntermediateRoot. Which makes these test meaningless. Here added the trie database commit method and make this test match its intention.

[holiman]

Interesting find, thanks!

Just a thought: would it perhaps be better to add a third check, a map[common.Hash]common.Hash that we also put things into, to verify that the stuff we expect are actually in the tries?

Or, even simpler, that we count the number of iterations in the two keysets, and check them against an expected number (should be 255?)

[zcheng9]

@holiman I think this kind of check has been done in other test. Here I can add one more check to see if the value associate to the same key in both persistent database is same. Because if the persistent database hold the same value, the trie retrieve from the database must be same. Do you intend to to this?

[zcheng9]

I add one more check for the leak test to check if the value of the persistent database matches. Also for the TestCopy(), I notice you do not test the copy of a copy stateDB can work as well as copy stateDB independently. This is described here

func (self *StateDB) Copy() *StateDB {
        ...
	// Above, we don't copy the actual journal. This means that if the copy is copied, the
	// loop above will be a no-op, since the copy's journal is empty.
	// Thus, here we iterate over stateObjects, to enable copies of copies
	for addr := range self.stateObjectsDirty {
		if _, exist := state.stateObjects[addr]; !exist {
			state.stateObjects[addr] = self.stateObjects[addr].deepCopy(state)
			state.stateObjectsDirty[addr] = struct{}{}
		}
	}
        ...
}

I add such test case in the updated commit.

[holiman]

If I'm reading it correctly, I don't think you understood what I meant. I did not mean to test that the stored value is correct (which probably is covered by another test), I meant a verification that the test does what it is supposed to do.

So the test earlier 'passed' because it actually didn't write anything. If it had contained a little counter, then it would have detected that it was in practice a useless test. So can we just add a sanity check that the test actually does what is intended? (e.g. a counter)

[zcheng9]

@holiman Ok, I get you. The trie store 255 accounts, and when commit this trie to the persistent database, it would compress the trie as 943 pair key-value store. So I can make a new trie to update this 255 address account and then use a counter to count the number of key-value pair in persistent database(which is 943 in this test). And compare this number to what we got from the database in the test to make sure they are same. Does this is what you suggest?

[holiman]

LGTM. Could @fjl or @karalabe please weigh in ?

[holiman]

@fjl PTAL