ink icon indicating copy to clipboard operation
ink copied to clipboard

Provide a `StorageVec` datastructure

Open xermicus opened this issue 2 years ago • 4 comments

Currently, we only provide a Mapping. However, storing things in a Vector (Array) on contract storage is also as thing our users need. Using the rust Vec from the prelude has a fundamental issue: It exhibits packed layout. This makes it a footgun when used on contract storage, easily leading to various sorts of DoS vulnerabilities.

There once was a dedicated storage vec data structure. This data structure would still use classical data structures from the prelude, but used its own StorageEntry struct to wrap it's data in and lazily read or write to/from storage. The approach would be to re-work it to function with the new storage API. On a high level, this should work. It will generate some amount of work implement though (I think it is more involved than just copying over the old code and make some minor changes).

Another thing is, for example, iterating over a Vec with 1000 elements on it, will still cause 1000 storage reads to the contracts pallet, and this will be costly. So I thought about whether we should try to be smart and do some further caching by designing a Lazy data structure that can read and write data in chunks. But it implies some drawbacks as well, e.g. additional complexity and it is not clear to me how we should determine an "optimal" chunk size.

xermicus avatar Feb 24 '23 10:02 xermicus

From another discussion; it could also be useful to provide some kind of "reference" data structure (akin to Box<>), that just holds a reference (the storage key) to a Vec<> or any other data packed data structure on storage. This would specifically allow to store any packed data in a mapping, without risking to blow up the contract because a mapping value got too large to decode.

So, the approach here would be:

  • Implement the reference type
  • Implement the storage vector type describe above

@xgreenx WDYT?

xermicus avatar Mar 22 '23 12:03 xermicus

I didn't get how the "reference" should work. For me, it is similar to the Lazy type that we have=) Could you elaborate more, please?

This would specifically allow to store any packed data in a mapping, without risking to blow up the contract because a mapping value got too large to decode.

Hmm, but Mapping already pulls only the data from one storage cell for packed types. It seems we move the problem from Mapping to the "reference"

xgreenx avatar Mar 22 '23 12:03 xgreenx

You are right, it's very similar. I think what we had in mind was to give the contract authors some way of trying to encode (or decode) the type, so that the error can be handled in case the Packed value gets too large. Do you think this can be built into Lazy?

xermicus avatar Mar 27 '23 10:03 xermicus

You are right, it's very similar. I think what we had in mind was to give the contract authors some way of trying to encode (or decode) the type, so that the error can be handled in case the Packed value gets too large. Do you think this can be built into Lazy?

We shouldn't allow storing values bigger than the buffer to decode them=)

I still didn't get how you want to use Lazy to address the decoding size problem. It will fail if the Lazy value exceeds a buffer. It will fail if the number of Lazy in the Vec<Lazy<SomeType>> is vast enough.

I think it is better to create a new type - StorageVec that uses Mapping inside to manage values and the len field. The type may work similarly to Solidity's storage array. But in our case, we can provide native iterator support because of transparent hashing. Plus some additional new features.

In this case, we have only one problem: the single elements may exceed the buffer, but it is unlikely.

xgreenx avatar Jul 30 '23 16:07 xgreenx