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 #220

Merged
merged 33 commits into from
Aug 20, 2024

Conversation

azurwastaken
Copy link
Contributor

@azurwastaken azurwastaken commented Aug 7, 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

I reopened this pr because there was an issue with our old fork. This one is from a fresh one.

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
    • Parse tx fee
    • Parse transaction from event
    • Check if message has already been processed
    • Build transaction
    • [Waiting for update to accepte blockifier tx] Submit tx to mempool
    • Update Messaging DB

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

@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?

Copying back this comment from Mohiit

@azurwastaken
Copy link
Contributor Author

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

and this one from cchudant

@azurwastaken azurwastaken changed the title L1to l2 messaging feat(client): l1<>l2 messaging crate Aug 7, 2024
@EvolveArt EvolveArt marked this pull request as ready for review August 12, 2024 11:45
crates/client/db/src/messaging_db.rs Outdated Show resolved Hide resolved
Copy link
Member

@cchudant cchudant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is cool! I didn't expect you to handle the niche message cancellation feature
I dont get the nonce part, why are you storing them in the db?

crates/client/db/src/messaging_db.rs Outdated Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Outdated Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/client.rs Outdated Show resolved Hide resolved
crates/client/db/src/messaging_db.rs Outdated Show resolved Hide resolved
crates/client/db/src/l1_db.rs Show resolved Hide resolved
crates/client/db/src/lib.rs Outdated Show resolved Hide resolved
crates/client/eth/src/client.rs Outdated Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Outdated Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Outdated Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
Copy link
Member

@cchudant cchudant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 🥳

crates/client/eth/src/l1_messaging.rs Show resolved Hide resolved
@EvolveArt EvolveArt merged commit 954f64a into madara-alliance:main Aug 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

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