Cosmos-sdk: append() may cause unexpected behavior in KVStore

[mossid]

In golang, builtin function append() does not copy the base slice. It reuses the space of the base slice and overrides to space next to it. Explained in SO (https://stackoverflow.com/questions/35276022/unexpected-slice-append-behaviour), tested here(https://play.golang.org/p/xQbfcVDDiH0).

I found this in prefixStore: the base slice of append was fixed slice prefixStore.prefix, so append() always use the same space. The root node of the iavl tree also referred this slice, making it change every time when we access to the prefixStore.

It could be prevented with explicit copying or allocation.

[cwgoes]

I think we can just allocate a new slice for PrefixStore-like usage; the overhead is insignificant compared to the overhead of IAVL iteration and disk I/O.

[ValarDragon]

Ideally we should be minimizing the disk I/O post launch, so in post-launch we should reconsider the memory allocation situation. Agreed that we should allocate new slices prelaunch, unless safety is clear from context.

[ValarDragon]

Found another interesting edge case with slices: https://play.golang.org/p/tVgShRaCmUo

basically when expanding a slice via append in another function, the underlying data gets mutated. However if you call len in the calling function afterwards, length didn't change. (This is because a slice is basically a size field plus a pointer to an array. size and pointer get copied into second function, value at pointer gets updated, but size was pass by value so it doesn't)

[alexanderbez]

@ValarDragon yes, but you didn't assign to x in the main function.

[cwgoes]

@mossid Has this been fixed by the prefix store PR we just merged? I think so...