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

Improve dojo contract syntax #1458

Closed
tarrencev opened this issue Jan 18, 2024 · 8 comments
Closed

Improve dojo contract syntax #1458

tarrencev opened this issue Jan 18, 2024 · 8 comments
Assignees
Labels
contributor enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@tarrencev
Copy link
Contributor

tarrencev commented Jan 18, 2024

Dojo defines its own native storage model. When using a dojo::contract the expectation is that a developer does not use the native starknet storage model. Currently, dojo::contract syntax leaks the native starknet::storage implementation in the contract interface.

This proposal is to modify the dojo::contract code expansion and to introduce a new dojo::interface macro.

[dojo::interface]

This macro expands the succinct dojo interface definition into the starknet::interface definition, removing the need to pass the ContractState parameters.

Old:

#[starknet::interface]
trait IActions<TContractState> {
    fn spawn(self: @TContractState);
    fn move(self: @TContractState, direction: dojo_examples::models::Direction);
}

New:

#[dojo::interface]
trait IActions {
    fn spawn();
    fn move(direction: dojo_examples::models::Direction);
}

[dojo::contract]

This modifies the dojo syntax to remove references to ContractState, enabling more succinct and idiomatic dojo implementations. The existing contract code expansion should be updated to expand to the existing syntax. The modifications should be made backward compatible, allowing developers to still pass a ContractState parameter manually.

Old:

fn tile_terrain(self: @ContractState, vec: Vec2) -> felt252 {
    'land'
}

New:

fn tile_terrain(vec: Vec2) -> felt252 {
   'land'
}

In addition, the syntax should be modified to enable passing a world: IWorldDispatcher argument. In the case it is passed, the macro should remove the arg from the function parameters and inline the let world = self.world_dispatcher.read(); call.

Old:

 fn spawn(self: @ContractState) {
    let world = self.world_dispatcher.read();
    let player = get_caller_address();
    let position = get!(world, player, (Position));
    ...

New:

fn spawn(world: IWorldDispatcher) {
    let player = get_caller_address();
    let position = get!(world, player, (Position));
    ...

Open questions

  1. How does this interact with the starknet component syntax? It should still support using native components / generics / etc

Approach

The implementation should modify the existing contract expansion code here: https://github.com/dojoengine/dojo/blob/main/crates/dojo-lang/src/contract.rs and introduce a similar interface expansion implementation.

The proposed syntax diff can be seen here: #1457

@tarrencev tarrencev added enhancement New feature or request help wanted Extra attention is needed labels Jan 18, 2024
@glihm
Copy link
Collaborator

glihm commented Jan 18, 2024

Amazing proposal, having a dojo syntax being consistent and even more simpler for devs is very important and good to have.

In addition, the syntax should be modified to enable passing a world: IWorldDispatcher argument. In the case it is passed, the macro should remove the arg from the function parameters and inline the let world = self.world_dispatcher.read(); call.

self is already confusing for new comers into starknet when it comes to function signature, questions like "What should I pass for self to call the contract" -> nothing.
I feel like the world arguments may cause the same questioning as it's removed by the macro. And even if the ABI will not show it... it could be confusing.

But my guess is that you aim at having something less cryptic to read the world address inside a system, right?
Can an attribute like #[with_world] could be better?

How does this interact with the starknet component syntax? It should still support using native components / generics / etc

This should not be an issue as components are usually imported outside of the system. And the system uses the storage access of the contract to interact with it. Or at least on my end I don't see something that may clash here at the first glance.

@tarrencev
Copy link
Contributor Author

self is already confusing for new comers into starknet when it comes to function signature, questions like "What should I pass for self to call the contract" -> nothing. I feel like the world arguments may cause the same questioning as it's removed by the macro. And even if the ABI will not show it... it could be confusing.

But my guess is that you aim at having something less cryptic to read the world address inside a system, right? Can an attribute like #[with_world] could be better?

It's a good concern. I think in this case it is a bit more intuitive on the contract syntax side, since you only need to define world if you are using it, instead of with self where it always needs to be defined. It can be a bit more confusing on the abi side but imo it is worth the more intuitive syntax. The inspiration is from https://bevyengine.org/ which implements a similar pattern. Generally, I would still support the world parameter approach since I think it feels a bit less "magical" than the decorator approach

@notV4l
Copy link
Collaborator

notV4l commented Jan 18, 2024

isnt it an issue to 'hide' self type, it can be ref self: ContractState or self: @ContractState ?

for world, i would prefer to use #[with_world]
its weird to get ride of self & introduce another implicit param

@tarrencev
Copy link
Contributor Author

isnt it an issue to 'hide' self type, it can be ref self: ContractState or self: @ContractState ?

dojo::contract should never use the self param. so it should always be provided as a snapshot

for world, i would prefer to use #[with_world] its weird to get ride of self & introduce another implicit param

hmm i feel like #[with_world] is more implicit, since it declares a new variable inline that can be used. to me it is not clear what #[with_world] refers to. whereas, passing world: IWorldDispatcher in the args is both explicit and provides more context on the var name + interface

@glihm glihm added ODHack OnlyDust Hack fest and removed ODHack OnlyDust Hack fest labels Feb 14, 2024
@ponderingdemocritus ponderingdemocritus moved this to 🆕 Prioritized in Dojo Feb 26, 2024
@remybar
Copy link
Contributor

remybar commented Feb 29, 2024

Hi @tarrencev !

I would really appreciate working on a task involving Cairo to put into practice what I've learnt so far :)

So, can I work on this task please ?

Edit: In fact, it's more Rust development than Cairo but anyway, I'm still motivated to work on this task ;-)

@glihm glihm added this to the 1.0.0 milestone Mar 1, 2024
@glihm
Copy link
Collaborator

glihm commented Mar 1, 2024

Hi @tarrencev !

I would really appreciate working on a task involving Cairo to put into practice what I've learnt so far :)

So, can I work on this task please ?

Edit: In fact, it's more Rust development than Cairo but anyway, I'm still motivated to work on this task ;-)

Let's go for it! You already have an idea on how to tackle this?

@remybar
Copy link
Contributor

remybar commented Mar 1, 2024

Hi !

Some questions, just to be sure about modifications to be done on [dojo::contract].
Let's take this dojo/Cairo snippet:

#[starknet::interface]
trait IPlayerActions<TContractState> {
    fn jump(self: @TContractState, distance: u8);
}

trait IMath {
    fn operation(a: felt252, b: felt252);
}

#[dojo::contract]
mod MyContract {

    impl PlayerActions of super::IPlayerActions<ContractState> {
        fn jump(self: @ContractState, distance: u8) {}
    }

    impl Math of super::IMath {
        fn operation(a: felt252, b: felt252) {}
    }

    fn free_function_with_state(self: @ContractState){}

    fn free_function(name: felt252) {
    }
}

In this snippet:

  • jump is a function from a starknet::interface so we want to write it without self and add this self parameter while expanding the code (and just ignore it if already present).
  • operation is a function coming from a trait which is not a starknet::interface so we don't have to add self.
  • free_function_with_state is a free function with a state, nothing to do here
  • free_function is a free function without state, nothing to do here.

In brief, that means that we can automatically add missing self parameter for functions coming from starknet::interface only (dojo::interface at the end).

Are you ok with that ? @tarrencev @glihm

@glihm
Copy link
Collaborator

glihm commented Mar 12, 2024

Solved by #1594.

@glihm glihm closed this as completed Mar 12, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Dojo Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor enhancement New feature or request help wanted Extra attention is needed
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants