Reconsider the design of StorageMap and StorageVec to avoid the use of __get_storage_key (while staying backward compatible)

[mohammadfawaz]

When storage references get implemented, we should move away from using __get_storage_key. This was a hacky way to get StorageMap working but now that we're about to have storage references, I think we can do better.

Currently, the method insert looks like:

    fn insert(self, key: K, value: V) {
        let key = sha256((key, __get_storage_key()));
        store::<V>(key, value);
    }

and is called using storage.map.insert(key, value), which is equivalent to ~StorageMap::insert(storage.map, key, value).

When we have storage references, we can imagine something like:

    fn insert(ref self, key: K, value: V) { // ref self or `StorageRef<Self>` or something like that
        let key = /* extract this from the reference to self */
        store::<V>(key, value);
    }

and the syntax for calling insert would remain the same: storage.map.insert(key, value) and would be equivalent to ~StorageMap::insert(storage.map.ref(), key, value).

Of course, a lot of details and the syntax have to be worked out, but I think this is a much more robust way of implementing StorageMap that does not actually break existing code. This also enables passing storage maps around to functions and also writing:

let map_ref = storage.map.ref();
map_ref.insert(key, value);

One more motivation for the above is that I actually have doubts about the intrinsic __get_storage_key if the method using it (like insert() or get()) is not inlined 🤔

Note

Equivalent Rust code for a simple struct would look like

struct S {
    x: u64,
}

impl S {
    fn foo(&self) -> u64 {
        self.x
    }
}

fn main() {
    let s = S { x: 5 };

    // These two are equivalent
    let x = s.foo(); // Effectively taking the ref here before calling `foo()`
    let x = S::foo(&s);
}

[mohammadfawaz]

@mitchmindtree touched on this subject a bit here #2585

[sezna]

I like this -- the only handwavy part we need to sort out is the "extract the storage key from the reference". I suppose that'd be through StorageRef's stdlib API, which would perhaps use __get_storage_key()?

[mohammadfawaz]

I like this -- the only handwavy part we need to sort out is the "extract the storage key from the reference". I suppose that'd be through StorageRef's stdlib API, which would perhaps use __get_storage_key()?

I think the reference itself will contain the key(s) needed depending on how we spec it out. Basically, the key is now an argument so that if insert is not inlined, we don't have any issues.