Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AutoKey produces inconsistent values #1825

Open
SkymanOne opened this issue Jun 24, 2023 · 6 comments
Open

AutoKey produces inconsistent values #1825

SkymanOne opened this issue Jun 24, 2023 · 6 comments
Labels
A-ink_metadata [ink_metadata] Work item C-bug Something isn't working

Comments

@SkymanOne
Copy link
Contributor

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:

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.

@SkymanOne SkymanOne added C-bug Something isn't working A-ink_metadata [ink_metadata] Work item labels Jun 24, 2023
@HCastano
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link

deuszx commented Jul 25, 2023

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
Copy link
Contributor Author

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
Copy link

deuszx commented Aug 31, 2023

@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
Copy link
Contributor Author

@deuszx see #1825 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ink_metadata [ink_metadata] Work item C-bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants