`AutoKey` produces inconsistent values

[SkymanOne]

Background

I had been working on the example for delegate_call() when stumbled across a "bug" that, given two identical storage layouts with the same field names and order, the automatically generated keys for Mapping appear to be different.

Problem

Look at two different contracts with different names, but same storage layouts:

• https://github.com/paritytech/ink/blob/4843f45999f94b01cfe44937b946db801c0799c6/integration-tests/delegatecall-bug/lib.rs#L13
• https://github.com/paritytech/ink/blob/4843f45999f94b01cfe44937b946db801c0799c6/integration-tests/key-reproducer/lib.rs#L7

When compiling both contracts in release mode. The metadata shows that Mapping have different keys:

From delegatecall-bug:

  "storage": {
    "root": {
      "layout": {
        "struct": {
          "fields": [
            {
              "layout": {
                "root": {
                  "layout": {
                    "leaf": {
                      "key": "0x9ac42f14",
                      "ty": 0
                    }
                  },
                  "root_key": "0x9ac42f14",
                  "ty": 1
                }
              },
              "name": "values"
            },
            {
              "layout": {
                "leaf": {
                  "key": "0x00000000",
                  "ty": 9
                }
              },
              "name": "value"
            }
          ],
          "name": "DelegateCallBug"
        }
      },
      "root_key": "0x00000000",
      "ty": 10
    }
  },

From key-reproducer:

  "storage": {
    "root": {
      "layout": {
        "struct": {
          "fields": [
            {
              "layout": {
                "root": {
                  "layout": {
                    "leaf": {
                      "key": "0xa06565a3",
                      "ty": 0
                    }
                  },
                  "root_key": "0xa06565a3",
                  "ty": 1
                }
              },
              "name": "values"
            },
            {
              "layout": {
                "leaf": {
                  "key": "0x00000000",
                  "ty": 9
                }
              },
              "name": "value"
            }
          ],
          "name": "KeyReproducer"
        }
      },
      "root_key": "0x00000000",
      "ty": 10
    }
  },

Potential Solutiom

My intuition is that the name of the storage plays a role in the auto key generation, but for Solidity devs this behaviour is unexpected and can be a footgun in the future.

[HCastano]

This is expected behaviour, the ident is used as part of the storage key calculation process.

https://github.com/paritytech/ink/blob/97f90ca6d032ecea1b0ff173ddde5ba0c18e1b30/crates/ink/codegen/src/generator/storage_item.rs#L213-L219

[SkymanOne]

This is expected behaviour, the ident is used as part of the storage key calculation process.

https://github.com/paritytech/ink/blob/97f90ca6d032ecea1b0ff173ddde5ba0c18e1b30/crates/ink/codegen/src/generator/storage_item.rs#L213-L219

This implies that devs working with multi-contract projects with the same storage layout need to define keys for all fields manually. Is this desirable behaviour? What is the motivation behind this design decision?

[deuszx]

This is expected behaviour, the ident is used as part of the storage key calculation process. https://github.com/paritytech/ink/blob/97f90ca6d032ecea1b0ff173ddde5ba0c18e1b30/crates/ink/codegen/src/generator/storage_item.rs#L213-L219

This implies that devs working with multi-contract projects with the same storage layout need to define keys for all fields manually. Is this desirable behaviour? What is the motivation behind this design decision?

How did you come to these conclusions @SkymanOne (i.e. that devs working with multi-contract projects need to define keys manually) ?

[SkymanOne]

This is expected behaviour, the ident is used as part of the storage key calculation process. https://github.com/paritytech/ink/blob/97f90ca6d032ecea1b0ff173ddde5ba0c18e1b30/crates/ink/codegen/src/generator/storage_item.rs#L213-L219

This implies that devs working with multi-contract projects with the same storage layout need to define keys for all fields manually. Is this desirable behaviour? What is the motivation behind this design decision?

How did you come to these conclusions @SkymanOne (i.e. that devs working with multi-contract projects need to define keys manually) ?

This implies that devs working with multi-contract projects with the same storage layout need to define keys for all fields manually. What I meant is the case where there is a multi-contract projects with cross-contract calls. You can try it out yourself and witness the behaviour.

[deuszx]

@SkymanOne , I just don't understand why you came to this conclusion. Why having ident affect the key would mean devs (of multi-contract projects) have to define them manually. Why would they be interested in having the same storage layout?

[SkymanOne]

@deuszx see https://github.com/paritytech/ink/issues/1825#issuecomment-1606925832 from Hernando.

Having an identical storage layout is required for delegate calls when contract B delegates to operate on contract A's storage. In this case, contract B must have an identical storage layout for layout-full storage and matching keys for the Lazy and Mapping fields.