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

[Feature Request] Introduce Visibility to Struct #157

Open
jolestar opened this issue Mar 29, 2022 · 7 comments
Open

[Feature Request] Introduce Visibility to Struct #157

jolestar opened this issue Mar 29, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@jolestar
Copy link
Contributor

jolestar commented Mar 29, 2022

🚀 Feature Request

Motivation

Currently, Move's structs use implicit visibility, e.g.

struct Foo{
   f1: u64,
}

is equivalent to.

public struct Foo{
   private f1: u64, 
}

Also, an implicit global storage access restriction is used to ensure that a struct can only be borrowed in the module in which it is defined.

This is actually equivalent to a conditional visibility constraint and has the same effect as adding a private visibility constraint to the struct.

private struct Foo has key{
}

So I propose to introduce the visibility of structs. This leaves it up to the smart contract developer to decide on access control for the global storage and the struct fields.

For example.

module MyModule {

   public struct Foo has key{
        public f1:u64,
   }

   private struct Bar has key{
        f1:u64,
   }
  
   public fun do_some(){
       let foo = borrow_global<Foo>();
       let bar = borrow_global<Bar>();
      // Both of these work 
   }
}

module XXX {
    use MyModule::Foo;
    public fun so_some(){
        let foo = borrow_global<Foo>();
        let f1 = foo.f1

        let bar = borrow_global<Bar>();
        // the first one will succeed, the second one will fail
    }
}

Of course, compatibility is a bigger problem, this is just an idea. If it works, I can do more work on it.

@jolestar jolestar added the enhancement New feature or request label Mar 29, 2022
@tnowacki
Copy link
Contributor

Definitely has been on our radar! If/when this happens, private structs would likely happen first, with field visibility coming later

@sblackshear
Copy link
Contributor

Is the idea here that external modules can still see private structs (e.g., include StructHandle's that refer to private structs), but not perform sensitive operations (pack, unpack, borrow_field, ...) on them?

@jolestar
Copy link
Contributor Author

jolestar commented Apr 1, 2022

Is the idea here that external modules can still see private structs (e.g., include StructHandle's that refer to private structs), but not perform sensitive operations (pack, unpack, borrow_field, ...) on them?

I prefer that external modules can not see a private struct, and can not use a private struct in an external module. This is the only way to ensure that TypeTable's borrow is as safe as borrow_global.

@tnowacki
Copy link
Contributor

tnowacki commented Apr 1, 2022

I'll be a bit more concrete about how the rule for private structs would shake out.

  • Much like private functions, a private struct handle cannot appear in another module
    • This means that another module cannot refer to this type at all
    • even if it is just to declare an empty local, e.g. let unused: OtherModule::PrivateStruct;

So to @sblackshear's question, I don't think there would be any seeing of private structs in other modules.


But things are different if we ever get to private/public fields. In a way, what we have today is all public structs and all private fields. So much like Rust, I would be in favor of exposing borrowing on public fields, AND exposing pack/unpack on a public struct if ALL fields are public.

@sblackshear
Copy link
Contributor

The strongest motivation I can see for private structs (or a feature like it) is the ability to use private as a type constraint (similar to phantom). This would allow us to define native functions with the same encapsulation protection as bytecode operations, which gives adapters a lot more power.

For example, there's been a lot of debate about whether the bytecode language should include move_to<T>(address, T). I think it's very reasonable for folks to want this, but they shouldn't need a language change to have it. It would be great to be able to write something like

public native fun move_to<private T>(a: address, t: T)

, where private T means that T can only be bound to StructDefinitionIndex's (i.e., types declared in the current module, just as we do for bytecodes).

Similarly, Sui has some native operations like transfer_object and emit_event that we would like the option to constrain in this way: MystenLabs/sui#19

@jolestar
Copy link
Contributor Author

jolestar commented Apr 3, 2022

The strongest motivation I can see for private structs (or a feature like it) is the ability to use private as a type constraint (similar to phantom). This would allow us to define native functions with the same encapsulation protection as bytecode operations, which gives adapters a lot more power.

Yes, as I mention in #158, the private Struct allows the Move contract developer to control the encapsulation protection, so we can use the native method or TypeTable to replace the global storage instruction.

@tnowacki
Copy link
Contributor

tnowacki commented Apr 4, 2022

The strongest motivation I can see for private structs (or a feature like it) is the ability to use private as a type constraint (similar to phantom). This would allow us to define native functions with the same encapsulation protection as bytecode operations, which gives adapters a lot more power.

For example, there's been a lot of debate about whether the bytecode language should include move_to<T>(address, T). I think it's very reasonable for folks to want this, but they shouldn't need a language change to have it. It would be great to be able to write something like

public native fun move_to<private T>(a: address, t: T)

, where private T means that T can only be bound to StructDefinitionIndex's (i.e., types declared in the current module, just as we do for bytecodes).

Similarly, Sui has some native operations like transfer_object and emit_event that we would like the option to constrain in this way: MystenLabs/sui#19

If the type is private, technically the module should be able to tightly control any usage of the type/values of the type. But, you might want the constraint to ensure safety properties aren't ignored by mistake. In other words, this seems more like a feature for preventing mistakes rather than expressiveness

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

No branches or pull requests

3 participants