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

feat(client): l1<>l2 messaging crate #216

Closed
wants to merge 64 commits into from
Closed

feat(client): l1<>l2 messaging crate #216

wants to merge 64 commits into from

Conversation

azurwastaken
Copy link
Contributor

@azurwastaken azurwastaken commented Aug 5, 2024

Pull Request type

  • Feature

What is the current behavior?

Resolves: #204

What is the new behavior?

  • Added a crate with a worker that listen for LogMessageToL2 event on main contract and handle it as the old madara did.
  • Added some entries to the db for messaging related data

Does this introduce a breaking change?

No

Usefull information

🚨Require #208 🚨
This PR include a lot of code from #208 as we started from it to build with the feature from the eth crate.

A few notes on the current design:

  • There is no guarantee that L1->L2 messages will be executed in a centralized context.
  • There is no ordering guarantee, nonces are solely used

Current Progress

Overall Workflow

  • Create a stream of LogMessageToL2 events
  • Get last synced block from Messaging DB
  • Consume the stream and log event
  • Process message

Specific case to handle

Tests

  • E2E test 1

    • Launch Anvil Node
    • Launch Worker
    • Send L1->L2 message
    • Assert that event is emitted on L1
    • Assert that even was caught by the worker with correct data
    • Assert the tx hash computed by the worker is correct
    • Assert that the tx has been included in the mempool
    • Assert that DB was correctly updated (last synced block & nonce)
    • Assert that the tx was correctly executed
  • E2E test 2

    • Should fail if we try to send multiple messages with same nonces
  • E2E test 3

    • Message Cancellation

@Mohiiit
Copy link
Contributor

Mohiiit commented Aug 5, 2024

Hey @azurwastaken, we were thinking of a different approach about the structure of the code. So instead of creating l1-messaging a different crate, we can have it inside the eth-crate, just like update_state. And as of now we are joining l1_sync with l1_messages_sync and l2_sync, so I was thinking to join l1_sync and l2_sync and inside the l1_sync we will have l1_messages_sync, state_update_sync and l1_gas_price sync, wdyt?

so structure of the code we had in mind was:

  • eth-crate
    |_____ state_update
    |_____ l1_messages
    |_____ l1_gas_price
    |_____ maybe a sync.rs with sync function which calls above 3

and the above sync function get called in the sync crate, inside the starknet_sync_worker, wdyt?

cc: @cchudant

@azurwastaken
Copy link
Contributor Author

Hey @azurwastaken, we were thinking of a different approach about the structure of the code. So instead of creating l1-messaging a different crate, we can have it inside the eth-crate, just like update_state. And as of now we are joining l1_sync with l1_messages_sync and l2_sync, so I was thinking to join l1_sync and l2_sync and inside the l1_sync we will have l1_messages_sync, state_update_sync and l1_gas_price sync, wdyt?

so structure of the code we had in mind was:

  • eth-crate
    |_____ state_update
    |_____ l1_messages
    |_____ l1_gas_price
    |_____ maybe a sync.rs with sync function which calls above 3

and the above sync function get called in the sync crate, inside the starknet_sync_worker, wdyt?

cc: @cchudant

Hey @Mohiiit , yes you are right, its porbably better if everything is together

@EvolveArt EvolveArt changed the title Adding L1 L2 messaging crate feat(client): l1<>l2 messaging crate Aug 6, 2024
@cchudant
Copy link
Member

cchudant commented Aug 6, 2024

Hey @azurwastaken, we were thinking of a different approach about the structure of the code. So instead of creating l1-messaging a different crate, we can have it inside the eth-crate, just like update_state. And as of now we are joining l1_sync with l1_messages_sync and l2_sync, so I was thinking to join l1_sync and l2_sync and inside the l1_sync we will have l1_messages_sync, state_update_sync and l1_gas_price sync, wdyt?

so structure of the code we had in mind was:

* eth-crate
  |_____ state_update
  |_____ l1_messages
  |_____ l1_gas_price
  |_____ maybe a `sync.rs` with sync function which calls above 3

and the above sync function get called in the sync crate, inside the starknet_sync_worker, wdyt?

cc: @cchudant

yes that makes sense
however a small thing here: i dont want the eth crate to be called from sync, i'd like it to be a Service so that i can just activate it even when doing block production (not sync) mode

also in the future this eth crate will expose a common interface to the rest of the app that is swappable with other chains (e.g for starknet L3s) but i'm getting ahead of myself

@azurwastaken
Copy link
Contributor Author

azurwastaken commented Aug 7, 2024

Outdated, see #220

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

Successfully merging this pull request may close these issues.

feat(l1): Implement l1<>l2 messaging
3 participants