Skip to content
New issue

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

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

Already on GitHub? # to your account

Inconsistent ABI metadata types with aliased types #6488

Closed
petertonysmith94 opened this issue Sep 2, 2024 · 4 comments · Fixed by #6494
Closed

Inconsistent ABI metadata types with aliased types #6488

petertonysmith94 opened this issue Sep 2, 2024 · 4 comments · Fixed by #6494
Assignees
Labels
ABI Everything to do the ABI, especially the JSON representation bug Something isn't working

Comments

@petertonysmith94
Copy link

petertonysmith94 commented Sep 2, 2024

Summary

When defining alias types in Sway, we obtain inconsistent results in the generated ABI between inline and alias types.

Consider the following contract:

type AliasedTuple = (u64, u64);

abi MyContract {
    fn tuple(arg1: (u64, u64)); // Inline
    fn aliased tuple(arg1: AliasedTuple); // Alias
}

This came from the following forum post.

Expected

We expect both metadata types to be equal, with a type of (_, _).

Actual

We actual get two distinct metadata types, with types as follows:

  • Inline: (_, _)
  • Aliased: (u64, u64)

Reproduction

A minimal reproduction can be found here.

@petertonysmith94 petertonysmith94 added bug Something isn't working ABI Everything to do the ABI, especially the JSON representation labels Sep 2, 2024
@IGI-111
Copy link
Contributor

IGI-111 commented Sep 2, 2024

These types are valid, just don't contain the same amount of information, I don't see a bug here.

If you want to compare them for equality or something, you have to use the concrete types. The metadata is produced with best effort from the compiler and not guaranteed to be this consistent.

@IGI-111
Copy link
Contributor

IGI-111 commented Sep 2, 2024

I suppose there is an issue in that this is producing types that the SDK isn't used to handling. But I'm not sure if we should make the alias types give you less direct information or have the SDK handle the more complete ones.

@esdrubal esdrubal self-assigned this Sep 2, 2024
@petertonysmith94
Copy link
Author

From my understanding of the spec, the metadata type for a tuple would be (_, _) and the concrete type would (u64, u64). I would have thought a type alias would follow the same principles.

@IGI-111
Copy link
Contributor

IGI-111 commented Sep 3, 2024

Good point, the spec actually enforces this. We should fix this then.

esdrubal added a commit that referenced this issue Sep 4, 2024
Before this push using an alias in a contract would produce an ABI with
two distinct metadata types for tuples, one for `(_,_)` and another one
for `(u64,u64)`.

With this change alias are bypassed and we only produce metadata for the
string type of the inner alias type. If alias is `(u64, u64)` we will
produce a metadatatype for `(_,_)`.

Fixes #6488
esdrubal added a commit that referenced this issue Sep 4, 2024
Before this push using an alias in a contract would produce an ABI with
two distinct metadata types for tuples, one for `(_,_)` and another one
for `(u64,u64)`.

With this change alias are bypassed and we only produce metadata for the
string type of the inner alias type. If alias is `(u64, u64)` we will
produce a metadatatype for `(_,_)`.

Fixes #6488
esdrubal added a commit that referenced this issue Sep 4, 2024
Before this push using an alias in a contract would produce an ABI with
two distinct metadata types for tuples, one for `(_,_)` and another one
for `(u64,u64)`.

With this change alias are bypassed and we only produce metadata for the
string type of the inner alias type. If alias is `(u64, u64)` we will
produce a metadatatype for `(_,_)`.

Fixes #6488
@IGI-111 IGI-111 closed this as completed in cd95cdc Sep 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
ABI Everything to do the ABI, especially the JSON representation bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants