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

No easy way of importing the StarknetMessaging Interface #603

Open
pscott opened this issue Sep 12, 2024 · 10 comments
Open

No easy way of importing the StarknetMessaging Interface #603

pscott opened this issue Sep 12, 2024 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@pscott
Copy link

pscott commented Sep 12, 2024

It's now easy to deploy a StarknetMessaging contract with .spawnVersion(). Great idea!

However, if one wants to interact with it in his own contract (example using sendMessageToL2), then one needs to import the MockStarknetMessaging.sol.
But importing this one implies import a whole bunch of other dependencies.

I'm not sure exactly what is the strict minimum of function that are use from the MockStarknetMessaging.sol contract but it might bebeneficial to have a lean interface for it :)

@FabijanC FabijanC added the enhancement New feature or request label Sep 13, 2024
@FabijanC
Copy link
Contributor

FabijanC commented Sep 13, 2024

This part is confusing me:

It's now easy to deploy a StarknetMessaging contract with .spawnVersion(). Great idea!

Are you referring to the spawnVersion method of Devnet from starknet-devnet-js? If so, that one is for spawning Devnet, not deploying a StarknetMessaging contract. Unless you are spawning a Devnet that loads a dumped instance with a previously deployed messaging contract.

@FabijanC FabijanC self-assigned this Sep 13, 2024
@FabijanC
Copy link
Contributor

FabijanC commented Sep 20, 2024

Contract dependency summary

  • MockStarknetMessaging (defined in MockStarknetMessaging.sol) is an instance of StarknetMessaging (defined in StarknetMessaging.sol).
  • StarknetMessaging is IStarknetMessaging (defined in IStarknetMessaging.sol) and uses methods of NamedStorage (defined in NamedStorage.sol).
  • IStarknetMessaging is IStarknetMessagingEvents (defined in IStarknetMessagingEvents.sol)

Simplification proposal

Copied from Discord:

What I had in mind is replicated this cairo storage values into solidity for the mock messaging contract: https://github.com/keep-starknet-strange/piltover/blob/a9c015eada5082076185a7b1413163a3da247009/src/messaging/component.cairo#L58.

And here we extracted only the required interface traits to actually work: https://github.com/keep-starknet-strange/piltover/blob/a9c015eada5082076185a7b1413163a3da247009/src/messaging/interface.cairo#L10.

So mapping this to a solidity code should make the mock way simpler that using the actual inheritance of the Starknet contract.

@FabijanC FabijanC assigned marioiordanov and unassigned FabijanC Sep 20, 2024
@marioiordanov
Copy link
Contributor

marioiordanov commented Sep 23, 2024

Hey @pscott I can't understand the following

However, if one wants to interact with it in his own contract

Do you mean that you have a solidity contract and you want to invoke methods of MockStarknetMessaging contract?

@pscott
Copy link
Author

pscott commented Sep 30, 2024

This part is confusing me:

It's now easy to deploy a StarknetMessaging contract with .spawnVersion(). Great idea!

Are you referring to the spawnVersion method of Devnet from starknet-devnet-js? If so, that one is for spawning Devnet, not deploying a StarknetMessaging contract. Unless you are spawning a Devnet that loads a dumped instance with a previously deployed messaging contract.

My bad, I wanted to write .loadL1MessagingContract(network) function, not the spawnVersion.

@pscott
Copy link
Author

pscott commented Sep 30, 2024

Contract dependency summary

  • MockStarknetMessaging (defined in MockStarknetMessaging.sol) is an instance of StarknetMessaging (defined in StarknetMessaging.sol).
  • StarknetMessaging is IStarknetMessaging (defined in IStarknetMessaging.sol) and uses methods of NamedStorage (defined in NamedStorage.sol).
  • IStarknetMessaging is IStarknetMessagingEvents (defined in IStarknetMessagingEvents.sol)

Simplification proposal

Copied from Discord:

What I had in mind is replicated this cairo storage values into solidity for the mock messaging contract: https://github.com/keep-starknet-strange/piltover/blob/a9c015eada5082076185a7b1413163a3da247009/src/messaging/component.cairo#L58.

And here we extracted only the required interface traits to actually work: https://github.com/keep-starknet-strange/piltover/blob/a9c015eada5082076185a7b1413163a3da247009/src/messaging/interface.cairo#L10.

So mapping this to a solidity code should make the mock way simpler that using the actual inheritance of the Starknet contract.

This would work, or maybe just creating a single file that takes all the external function of:

  • MockStarknetMessaging
  • StarknetMessaging
  • IStarknetMessaging
  • IStarknetMessagingEvents

By having everything in a single file, it would make interacting with the contract much easier (though I agree it would be a messy file ...)

@pscott
Copy link
Author

pscott commented Sep 30, 2024

Hey @pscott I can't understand the following

However, if one wants to interact with it in his own contract

Do you mean that you have a solidity contract and you want to invoke methods of MockStarknetMessaging contract?

Yes this is exactly what I was referring to. Interacting with the MockStarknetMessaging contract requires me to copy paste a whole bunch of other files.

@marioiordanov
Copy link
Contributor

What I can suggest is using IStarknetMessaging interface. When you want to call methods of the messaging contract you can just do IStarknetMessaging(messaging_contract_address).sendMessageToL2

@pscott
Copy link
Author

pscott commented Sep 30, 2024

What I can suggest is using IStarknetMessaging interface. When you want to call methods of the messaging contract you can just do IStarknetMessaging(messaging_contract_address).sendMessageToL2

I believe the issue is if I wish to write IStarknetMessaging(messaging_contract_address), I need to first have imported the file.
So I write import "./IStarknetMessaging.sol";.

But since this file starts with
import "./IStarknetMessagingEvents.sol";, I also need to copy the file IStarknetMessagingEvents.sol. So I still need to copy two different files. This clobbers up my repo.

What would be nice is to have a simple, lean, interface file, that one can import and that has all the functions available to interact with the messaging contract

@marioiordanov
Copy link
Contributor

Then you can omit IStarknetMessagingEvents.sol from IStarknetMessaging.sol and you will have 1 file.

@pscott
Copy link
Author

pscott commented Oct 1, 2024

In which case I also need to remove is IStarknetMessagingEvent.
I feel like to a beginner, touching the code of a copied file might no be intuitive. I think providing the interface somewhere and say "hey, just copy this and you'll be fine" would be more convenient :D

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
Status: 🆕 New
Development

No branches or pull requests

3 participants