Update ERC-7786: Extend ERC 7786 to support PULL communication mode #852
Conversation
File
|
eip-review-bot
changed the title
philippecamacho
changed the title
ERCS/erc-7786.md
Outdated
| @@ -103,6 +103,12 @@ This event signals that a would-be sender has requested a message to be sent. | |||
|
|
|||
| If `outboxId` is present, post-processing MAY be required to send the message through the cross-chain channel. | |||
|
|
|||
| #### Mandatory and optional attributes | |||
|
|
|||
| The `attributes` parameter MUST contain as its first element the value `read(true)` or `read(false)`. `read(true)` means the message can be read by the application on the destination gateway. `read(false)` means the message by pushed by the destination gateway on the application via `executeMessage`(see below). | |||
There was a problem hiding this comment.
- if mandatory, why not just explicitly enforce at the function signature level? i.e.
sendMessage(dest, recv, payload, bool pullable, attributes) - personally, i think
read(true)is not intelligible. I prefer one ofpullable,supports_pull,pull_based,push_based.
wdyt? @frangio @elliedavidson
There was a problem hiding this comment.
pushBased used in 15c1f1b.
ERCS/erc-7786.md
Outdated
|
|
||
| The `attributes` parameter MUST contain as its first element the value `read(true)` or `read(false)`. `read(true)` means the message can be read by the application on the destination gateway. `read(false)` means the message by pushed by the destination gateway on the application via `executeMessage`(see below). | ||
|
|
||
| In the case `read(true)` is set as teh first element of the `attributes` parameters, the next element MUST be `messageId(val)` where val is a sequence of 32 bytes. |
There was a problem hiding this comment.
- typo "teh" -> "the"
- didn't Francisco and CJ say they would want to add
msgIdas mandatory for all?
There was a problem hiding this comment.
messageId has been defined as a default attribute in 15c1f1b.
ERCS/erc-7786.md
Outdated
| @@ -103,6 +103,12 @@ This event signals that a would-be sender has requested a message to be sent. | |||
|
|
|||
| If `outboxId` is present, post-processing MAY be required to send the message through the cross-chain channel. | |||
|
|
|||
| #### Mandatory and optional attributes | |||
There was a problem hiding this comment.
All attributes should be optional. read(false) would be the default.
There was a problem hiding this comment.
Makes sense. I use the term "default" attributes instead, in 15c1f1b. Also following @alxiong's suggestion I used the name pushBased instead of read.
Co-authored-by: Francisco Giordano <[email protected]>
There was a problem hiding this comment.
To be totally honest, I think the addition of this either push or pull operating mode has complicated the standard more than is optimal. I would actually prefer for all gateways to support both push and pull, and I think that would be cleaner. If an app doesn't want push, it can just choose to either not implement executeMessage, or implement it as a no-op. And for the storage concern (forcing all gateways to store all messages so they are queryable), gateways can still delete messages after some time if they choose to, with a mechanism to repopulate if neccessary; most messages will be consumed fairly quickly anyways, and repopulation can be triggered by retrying the relaying process from source.
Just my thoughts. I just think a big advantage of this ERC is it's simplicity, and I don't really like the added complexity of this switching behavior.
ERCS/erc-7786.md
Outdated
|
|
||
| Compared to previous ERCs in this area, this ERC offers compatibility with chains outside of the Ethereum/EVM ecosystem, and it is extensible to support the different feature sets of various protocols while offering a shared core of standard functionality. | ||
| Compared to previous ERCs in this area, this ERC offers compatibility with chains outside of the Ethereum/EVM ecosystem, and it is extensible to support the different feature sets of various protocols while offering a shared core of standard functionality. Moreover, this standard supports both PUSH and PULL communication modes, capturing a larger class of interoperability protocols. |
There was a problem hiding this comment.
I actually have concerns about using push and pull in this way, based on our conversations in the working group. I think these terms are too broad, and mean different things to different people, and so adding this here could lead to confusion.
There was a problem hiding this comment.
I agree, also because PUSH/PULL communication modes are not defined in this section. The sentence was removed in 39159f7.
Co-authored-by: CJ Cobb <[email protected]>
|
The commit b566adb (as a parent of c10aac8) contains errors. |
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: