-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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(interop): L2ToL2CDM SentMessage event, Relay Entrypoint, Better Event Topics #11592
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @hamdiallam and the rest of your teammates on Graphite |
packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
Wondering the status on this, if its something that having merged before the first devnet would be ideal |
@skeletor-spaceman said they'd take over the PR+spec changes for these changes. Do y'all want us to do this? |
@tynes we have this as a mid-priority on our tasks. it will get done before devnet :) |
Was OOO this week but will sync with the team next Monday and let you know! |
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
caaa299
to
6e0b16d
Compare
6e0b16d
to
fb4fb10
Compare
2e619c7
to
dc208c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requested some minor changes and added an interesting question :)
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
3e51c35
to
1e2d293
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far! I focused on the tests, left few small comments and questions, and also a testFuzz_relayMessage_eventPayloadNotSent_reverts
test is missing on the CDM unit tests.
packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol
Outdated
Show resolved
Hide resolved
c538d89
to
a6ca91f
Compare
SentMessage
event.CrossL2Inbox
devex #11322 to provide a public relay entrypoint to consume that event when relaying instead of CrossL2Inbox directly executingrelayMessage
via the emitted LOG0 eventInclude indexed source,nonce as topics in the sent/relay events for easy on-chain indexing & searching
Aside