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

feat: Include full module path to structs in abi #2296

Closed

Conversation

spalladino
Copy link
Contributor

@spalladino spalladino commented Aug 11, 2023

Continues the work from #2266 and prepends the full module path to struct type names in the ABI. The name is composed as package_name::module_1::...::module_N::struct_name.

For example:

    {
      "name": "entrypoint",
      "functionType": "secret",
      "isInternal": false,
      "parameters": [
        {
          "name": "payload",
          "type": {
            "kind": "struct",
-           "name": "EntrypointPayload",
+           "name": "noir_aztec::entrypoint::EntrypointPayload",
            "fields": [
              {
                "name": "flattened_args_hashes",
                "type": {
                  "kind": "array",
                  "length": 4,
                  "type": {
                    "kind": "field"
                  }
                }
              },

Note that the crate_name added to CrateData is optional, since some instantiations of crates seem to be coming from not-a-package. Also, I was unsure as to emit the name of the package or the name of the dependency as registered in the contract package TOML. I decided to go with the former, since this ABI may be consumed from a location that doesn't know the mapping from dependency names to packages.

Now, if this feels like we're spreading too much data across internal types, I'm fine closing this PR and looking for an alternate solution.

Fixes #2238

@phated
Copy link
Contributor

phated commented Aug 14, 2023

Now, if this feels like we're spreading too much data across internal types, I'm fine closing this PR and looking for an alternate solution.

Yeah, this feels like we're being leaky here. I need to look deeper than just the diff to see if this can be built up without cloning strings everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give the full path of each Struct used in the Abi
3 participants