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: l2tol2cdm RelayedMessage return value #227

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hamdiallam
Copy link
Contributor

Description

Increase leverage of the L2ToL2CDM by including the return value in the RelayedMessage event

@hamdiallam hamdiallam changed the title relay message return feat: l2tol2cdm RelayedMessage return value Mar 24, 2025
@tynes
Copy link
Contributor

tynes commented Mar 24, 2025

This is missing the resource usage section. How will this impact the happy path (no returndata) vs worst case (returndata bomb) in gas costs and database size?

- Bridge Funds
- Invoke calldata on via a remotely controlled proxy account

Read-based cross chain messages are less common but will increasingly become so in an asynchronous world. Solidity has a `view` modifier specifically targeted for reading -- it would be natural to also want to also query this state onchain very quickly.
Copy link
Contributor

@tynes tynes Mar 24, 2025

Choose a reason for hiding this comment

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

For some usecases, its not safe to read remote data without knowing the timestamp of the read. You could be tricked into reading stale data. Is there an easy way to see the timestamp of the read?

Copy link
Contributor Author

@hamdiallam hamdiallam Mar 24, 2025

Choose a reason for hiding this comment

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

Identifier of the remote log plays in nicely here. The consumer on the source chain can choose to reject at whatever latency, the RelayedMessage event if the timestamp of the event is stale

Nothing needs to change on the destination.

@tynes
Copy link
Contributor

tynes commented Mar 24, 2025

I recommend adding the table to the header that was recently added to the template: https://github.com/ethereum-optimism/design-docs/blob/main/assets/design-doc-template.md#project-name-design-doc

@hamdiallam
Copy link
Contributor Author

This is missing the resource usage section. How will this impact the happy path (no returndata) vs worst case (returndata bomb) in gas costs and database size?

ah good catch, let me add this


### Single Point of Failure and Multi Client Considerations

N/A as this is a smart contract change.
Copy link
Contributor

Choose a reason for hiding this comment

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

Our smart contracts are a single point of failure. Sometimes it just isn't possible to get away from a single point of failure, but we should generally strive to be aware of the single points of failure and architect away as many as possible. It would still be good to consider this section. One consideration could be the tradeoff between adding a ton of stuff into the messenger vs not and keeping it simple. It is permissionless to build a cross domain messenger with arbitrary features that isn't part of the predeploys. This is just the one that we maintain. So backwards compatibility with any change is a big consideration with this change. The event identifier changes, so it will break backwards compatibility with the existing relayers. Which is ok, could be good to note down

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

}
```

The `messageHash` known ahead of time can be correlated with the corresponding `RelayedMessage`. Thus the return value of any read function or return value of a write call becomes immediately actionable for the sender, without requiring special integration.
Copy link
Contributor

@tremarkley tremarkley Mar 25, 2025

Choose a reason for hiding this comment

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

In order to read/validate the RelayedMessage, the identifier of the RelayedMessage is a requirement.
Wouldn't an integration be required for constructing the identifier of the RelayedMessage since you cannot calculate the identifier ahead of time?

Copy link
Contributor Author

@hamdiallam hamdiallam Mar 25, 2025

Choose a reason for hiding this comment

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

by ahead of time here, I meant with the return value of sendMessage, the message hash. You can directly chain the continuation since the caller just needs to assert the msg hash matches with the RelayedMessage event.

I'll make this a bit more clear

Copy link
Contributor

Choose a reason for hiding this comment

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

I was referring to the message identifier not being able to be calculated ahead of time and not the message hash. The identifier is needed in order to trigger the continuation, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea after relay is when the continuation would be invoked. you need the return value

the identifier isnt computed ahead of time. It can just be consumed right away after emission

Copy link
Contributor

@tremarkley tremarkley Mar 25, 2025

Choose a reason for hiding this comment

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

Yeah and so that identifier and message will need to be fetched or indexed offchain. Do you see this requiring some additional offchain infra to find and “relay” these events back to the source chain? Relay in quotes because it’s not a relay using relayMessage, but you need to have some service that fetches that event and then triggers the continuation on the source chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, we'll definitely need indexing of pending promises that relayers will have to handle separately. Especially now that the data is hashed.

This plays in nicely for protocols that have to propagate the return data backwards, which incurs extra gas cost

@tynes
Copy link
Contributor

tynes commented Mar 31, 2025

We generally have consensus on this approach, but will add the hash of the returndata to the event rather than the full returndata itself. The returndata can be unwrapped on the originating chain. The reason that the hash is used is to make the costs more constant, reducing risk for paying a relayer on the local chain for their execution on the remote chain. There is a concern that extra data needs to be posted to the calldata, which puts pressure on DA. This is a valid concern, if its abi encoded returndata, then it likely compresses pretty well, but ultimately depends on the returndata itself

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.

3 participants