-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat: add origin context #16150
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: add origin context #16150
Conversation
|
||
/// @dev Sets the cross domain messenger context in transient storage. | ||
/// @param _originContext Context to set. | ||
function setCrossDomainMessageOriginContext(bytes memory _originContext) external { | ||
(uint8 encodingVersion, bytes32 messagePayloadHash) = _parseOriginContext(_originContext); | ||
|
||
assembly { | ||
tstore(ORIGIN_CONTEXT_VERSION_SLOT, encodingVersion) | ||
tstore(ORIGIN_CONTEXT_MESSAGE_PAYLOAD_HASH_SLOT, messagePayloadHash) | ||
} | ||
} |
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.
The function documentation should use @notice
instead of @dev
to maintain consistency with the documentation style used throughout the codebase. Consider updating the comment to:
/// @notice Sets the cross domain messenger context in transient storage.
/// @param _originContext Context to set.
/// @dev Sets the cross domain messenger context in transient storage. | |
/// @param _originContext Context to set. | |
function setCrossDomainMessageOriginContext(bytes memory _originContext) external { | |
(uint8 encodingVersion, bytes32 messagePayloadHash) = _parseOriginContext(_originContext); | |
assembly { | |
tstore(ORIGIN_CONTEXT_VERSION_SLOT, encodingVersion) | |
tstore(ORIGIN_CONTEXT_MESSAGE_PAYLOAD_HASH_SLOT, messagePayloadHash) | |
} | |
} | |
/// @notice Sets the cross domain messenger context in transient storage. | |
/// @param _originContext Context to set. | |
function setCrossDomainMessageOriginContext(bytes memory _originContext) external { | |
(uint8 encodingVersion, bytes32 messagePayloadHash) = _parseOriginContext(_originContext); | |
assembly { | |
tstore(ORIGIN_CONTEXT_VERSION_SLOT, encodingVersion) | |
tstore(ORIGIN_CONTEXT_MESSAGE_PAYLOAD_HASH_SLOT, messagePayloadHash) | |
} | |
} | |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
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.
This is a test function that use the same style that previous test functions. Won't fix to match the current style.
keccak256(_message) | ||
); | ||
|
||
Identifier memory id = | ||
Identifier(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER, _blockNum, _logIndex, _time, _source); | ||
bytes memory sentMessage = abi.encodePacked( | ||
abi.encode(L2ToL2CrossDomainMessenger.SentMessage.selector, block.chainid, _target, _nonce), // topics | ||
abi.encode(_sender, _message) // data | ||
abi.encode(_sender, _message, originContext) // data (no context) |
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.
The comment // data (no context)
contradicts the code which clearly includes originContext
in the data. This comment should be updated to accurately reflect what the code is doing, perhaps to // data (with originContext)
or simply // data
.
abi.encode(_sender, _message, originContext) // data (no context) | |
abi.encode(_sender, _message, originContext) // data (with originContext) |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
/ci authorize 3502c38 |
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.
Pull Request Overview
Adds an origin context parameter to cross-domain messages so it can be carried through SentMessage
events and storage.
- Introduces origin context slots, parsing, and accessor functions in
L2ToL2CrossDomainMessenger
- Updates
Hashing
library to split payload hashing and full message hashing including origin context - Updates tests, ABI snapshots, and semver locks to include the new context parameter
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/contracts-bedrock/test/L2/L2ToL2CrossDomainMessenger.t.sol | Tests updated to encode and assert on originContext |
packages/contracts-bedrock/test/L2/ExecutingMessageEmitted.t.sol | Event selector and test payloads updated for context |
packages/contracts-bedrock/src/libraries/Hashing.sol | Split hashL2toL2CrossDomainMessage into payload vs full hash |
packages/contracts-bedrock/src/L2/L2ToL2CrossDomainMessenger.sol | Core messaging contract extended with context storage, parsing, and events |
packages/contracts-bedrock/snapshots/semver-lock.json | Updated init and source code hashes after changes |
packages/contracts-bedrock/snapshots/abi/L2ToL2CrossDomainMessenger.json | ABI updated to include context fields and new functions |
packages/contracts-bedrock/interfaces/L2/IL2ToL2CrossDomainMessenger.sol | Event and resendMessage signature updated, missing new accessor |
Comments suppressed due to low confidence (2)
packages/contracts-bedrock/interfaces/L2/IL2ToL2CrossDomainMessenger.sol:45
- The interface defines the updated
SentMessage
event but does not include the newcrossDomainMessageOriginContext()
accessor or theORIGIN_CONTEXT_ENCODING_VERSION
constant. Add the function signature and constant to the interface to match the implementation and ABI.
event SentMessage(
packages/contracts-bedrock/test/L2/ExecutingMessageEmitted.t.sol:18
- [nitpick] Hard-coding the
SentMessage
event selector increases maintenance burden. Consider deriving it dynamically (e.g.,SentMessage.selector
) to ensure it stays correct when the event signature changes.
bytes32 internal constant SENT_MESSAGE_EVENT_SELECTOR =
Closing in favor of #16402 |
Description
Adds additional param to
SentMessage
event that contains context that can be propagated across N cross domain messagesDesign Doc: ethereum-optimism/design-docs#266