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

Support type aliases as contracttypes #1063

Open
karol-bisztyga opened this issue Aug 18, 2023 · 7 comments
Open

Support type aliases as contracttypes #1063

karol-bisztyga opened this issue Aug 18, 2023 · 7 comments
Assignees

Comments

@karol-bisztyga
Copy link

What did you do?

I have a project that uses rust workspaces (each contract is a separate workspace member).
I want to share some code between them, mostly types.

repro: here is a repro https://github.com/karol-bisztyga/soroban-test

The problem is, it builds and deploys but when I invoke the contract I get MissingEntry("DataType") error. When I replace the type with just soroban_sdk::String, it works, but when I get the type from the shared crate, it breaks (it's still just a string just aliased). Looks like during the build or deployment the imported code's not attached to the contract.

To reproduce (more or less):

git clone https://github.com/karol-bisztyga/soroban-test.git
cd soroban-test
docker run --rm -it -d -p 8000:8000 --name stellar stellar/quickstart:soroban-dev@sha256:ed57f7a7683e3568ae401f5c6e93341a9f77d8ad41191bf752944d7898981e0c --standalone --enable-soroban-rpc
./run.sh

What did you expect to see?

Working invoke.

What did you see instead?

MissingEntry("DataType") error.

Please let me know if it's the right place to post this, if not, pls point me to the right one.

@karol-bisztyga karol-bisztyga added the bug Something isn't working label Aug 18, 2023
@karol-bisztyga
Copy link
Author

Hey! Seems like even if the type is defined in the same file, it doesn't work so it looks like soroban requires that we do not use any type aliases for the contract API. Do you think this could be changed?

ref: karol-bisztyga/soroban-test@f2f35e8

@karol-bisztyga
Copy link
Author

So something like this does not work:

type DataType = soroban_sdk::String;
#[contract]
pub struct MyContract;
#[contractimpl]
impl MyContract {
  pub fn foo(env: Env, arg: DataType) -> u32 { // Result::unwrap()` on an `Err` value: MissingEntry("DataType")
    22
  }
}

@leighmcculloch leighmcculloch self-assigned this Aug 21, 2023
@leighmcculloch leighmcculloch transferred this issue from stellar/stellar-cli Aug 21, 2023
@leighmcculloch
Copy link
Member

leighmcculloch commented Aug 21, 2023

Transferred this issue from stellar/soroban-tools#870 to #1063 because the limitation preventing this from working is in the SDK. I believe the contract spec is extendable to address this problem. I've assigned the issue to myself and will investigate.

@leighmcculloch
Copy link
Member

leighmcculloch commented Aug 25, 2023

From best I can tell with the experiment I ran, types can live in other libraries and be imported. See #1070 for an example of it working.

I think the issue that's occurring in the example above is that a type alias is being used. Soroban contracttype's don't support a type alias.

type DataType = soroban_sdk::String;

We could add support for it, although it would be somewhat noisy in contract interfaces. Contract interfaces would have to store the original type, and the alias type. So the contract will be smaller and cheaper to execute if the alias was ommitted. If it's inefficient, folks will probably avoid it, so it's questionable if we should add logic and complexity to handle something that will be avoided.

Thoughts anyone? @stellar/contract-committers?

leighmcculloch added a commit that referenced this issue Aug 25, 2023
…ib (#1070)

### What
Add test vector for workspace setups where contract types live in a lib.

### Why
In #1063 it was
mentioned that folks were having trouble with using types in other libs.
Added this test vector to confirm if that was an issue or not. Appears
to work fine, and this is a use case that is important, so committing
the test vector.
@leighmcculloch leighmcculloch changed the title Cannot use shared code between contracts Support type aliases as contracttypes Aug 25, 2023
@leighmcculloch leighmcculloch removed their assignment Aug 25, 2023
@leighmcculloch
Copy link
Member

I've looked into this a bit more and I think the effort to implement is significant.

For user-defined-types I think this is possible. The contracttype macro would need to be changed so that for aliases it imported the contract spec from the other crate.

For SDK types things get interesting because the contractimpl macro will not recognize that the DataType (in #1063 (comment)) is a SDK type and will assume it's a user defined type. It's unclear how, without rigging up static constructors with the ctor crate so track the types.

The complexity this introduces to the SDK is significant, and effort is significant. There hasn't been much fan fare about the feature request. It could be that folks want it but haven't indicated so. At the moment I'm leaning towards closing the feature until we get more signal on utility.

@leighmcculloch
Copy link
Member

As a another data point for evaluating adding this feature, this feature was requested in Discord:

@leighmcculloch
Copy link
Member

For SDK types things get interesting because the contractimpl macro will not recognize that the DataType (in #1063 (comment)) is a SDK type and will assume it's a user defined type.

This might not be as much of a problem as I first thought.

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

No branches or pull requests

3 participants
@leighmcculloch @karol-bisztyga and others