-
Notifications
You must be signed in to change notification settings - Fork 39
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: promise callback library #216
base: main
Are you sure you want to change the base?
Conversation
I don't think any amount of design review will be able to give a thumbs up on this design better than working with teams to build interop into their applications. I recommend that we focus on getting the entrypoint primitive into the |
|
||
# Alternatives Considered | ||
|
||
## Callback Entrypoint |
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 entrypoint solution and this proposal are not mutually exclusive though, right? You are simply proposing that the CDM emit return data that can be used by other contracts if I understand correctly. Entrypoints would still have a place but wouldn't be the preferred way to address this use case?
- Tight coupling between the source & destination chains. Callback information must be propogated between networks | ||
- Multiple callbacks cant clearly be registered. Must be enshrined in the propogated callback data. | ||
- Single interface for relayers `relayMessage`. | ||
- The proposed solution could be implemented via a generic entrypoint to avoid adding the return data to the event. However this feels like more complexity from a code & call path perspective. Propogate callback -> parse callback -> relay -> capture return and sendMessage(). Versus -- RelayedMessage() -> continue(). |
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.
Hi @tynes! Just wanted to bring your attention to this point from Hamdi - relying on entrypoints for callbacks seems like it would increase complexity for the app dev significantly, don't you think?
This proposal comes from feedback at ETH Denver around how app devs interact with the interop protocol as currently written, and I worry that a relatively minor change like this would be much harder to implement in the future when interop is live than it would be now. We'd be happy for entrypoints to be an option as well, but I'm not sure that effort should preclude full consideration of this idea. Curious to understand your thinking around this better!
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.
+1
If this involved a lot of change to the messenger, avoiding this and instead leveraging entrypoints would make a lot of sense.
However the change is very minimal and at the same time very high leverage (consuming the return data via the event itself) in the event promise based contract development becomes more interesting.
related to the callback is managed on the sending chain and works with any remote target. | ||
|
||
```solidity | ||
library Promise { |
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.
I think the API for incentivized relay / callbacks should be as minimal and flexible as possible. This makes me wary about using a Library because then it's harder to hide the underlying callback logic and harder to update. This is why I lean towards using a dedicated contract for registering callbacks that you call vs using a library
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.
yea totally agree, the functions here are marked as external meaning this library gets deployed as a dedicated contract
no point in declaring this as a lib though, i'll update this to be a contract
This document is coupling too many different things. We are talking about a modification to the cross domain messenger, a promise library and an incentivized relay. I recommend creating a design doc that specifically is focused on the changes to the cross domain messenger that you want to get in. I also am not confident in making a bunch of small changes to the messenger without understanding how a specific user's app needs the change. Historically we added features when they sounded cool, which has resulted in a lot of tech debt. I want to see changes driven by an app attempting to integrate and then unable to do so because of the lack of a feature |
Nowhere in this design doc is incentivized relays mentioned. I've explicitly made them separate since they are not dependent on each other. In the doc you'll see this is only about chaining side effects to the return value of a remote call, generally useful when writing cross chain contracts, removing the need to write handlers that ping pong back the return value for every possible remote view call I'll go ahead and pull out the changes to the messenger but you're unnecessarily commingling the two |
Description
By adding the return data to the
RelayedMessage
event, a callback library can be introduced allowing users to attach continuations dependent on the return. This makes any remote view function callable with low latency via superchain interop.