ERC-7786 interfaces and receiver #5255
Conversation
Co-authored-by: cairo <cairoeth@protonmail.com>
Co-authored-by: cairo <cairoeth@protonmail.com>
Co-authored-by: cairo <cairoeth@protonmail.com>
Co-authored-by: cairo <cairoeth@protonmail.com>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
Co-authored-by: cairo <cairoeth@protonmail.com>
Co-authored-by: cairo <cairoeth@protonmail.com>
|
|
| ) public payable virtual { | ||
| if (_isKnownGateway(msg.sender)) { | ||
| // Active mode | ||
| // no extra check |
There was a problem hiding this comment.
shouldn't this check that gatewayMessageKey and gateway are zero https://github.com/ethereum/ERCs/pull/673/files#diff-07848a22c16b11582927b37d76b2c54a0617b1444b30d11bca783ac98eaed01fR150? or is that checked by the gateway?
There was a problem hiding this comment.
As far as I remember the ERC says that in active mode the fields should be empty, not that must be empty. The idea is that we should not expect them to be populated (in fact we don't use these values in active mode)... but we should also not fail if they contain something.
There was a problem hiding this comment.
Yeah I specified it as "SHOULD" so this implementation is compliant. But it's worth considering if it should be "MUST". I don't see any concrete issue with allowing any values and ignoring them.
| // no extra check | ||
| } else if (_isKnownGateway(gateway)) { | ||
| // Passive mode | ||
| IERC7786GatewayDestinationPassive(gateway).validateReceivedMessage( |
There was a problem hiding this comment.
receiveMessage is payable so it can receive value, but the value received is not passed to validateReceivedMessage, how can the passive gateway verify the msg.value received?
There was a problem hiding this comment.
In passive mode, the value is not sent by the relayer. Value (if any) must be sent by the gateway.
So if you expect value with a message, and if you operate in passive mode, you should have a receive function. When you do the validate, the gateway should send you the value using that receive function.
How the receiver detect that value was received is a great question. I think it's more a question for the ERC than for the implementation.
There was a problem hiding this comment.
One answer is that any value should come with a corresponding attribute, and that is how it would be visible from the processMessage (instead of using msg.value)
There was a problem hiding this comment.
If we want to make payable compatible with passive mode we would have to add a value parameter to validateReceivedMessage.
There was a problem hiding this comment.
After discussion it feels like the thing is:
- Value should only be part of active mode calls.
- Passive mode should not support value, because value cannot be easily moved in a passive way
- In that case we have a choice of doing
require(msg.value == 0);in the passive mode, or to allow value to be passed by the relayer, without it being related to the semantics of the message
- In that case we have a choice of doing
There was a problem hiding this comment.
I would do require(msg.value == 0). In terms of the spec I think this check should be recommended but not mandatory.
Right, I misread the changelog, will go back to 0.8.24 for Bytes and all the code that depends on it directly or indirectly. |
Co-authored-by: cairo <cairoeth@protonmail.com>
…elin-contracts into feature/crosschain
Co-authored-by: cairo <cairoeth@protonmail.com> Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com>
Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com>
|
Because the discussion on the PR is still quite active, we (@frangio and I) decided to remove that from the main repo in 5.2. This will go in the community repo, where it will be more "fluid". Basically, we don't want to publish helpers that we know will be outdated in a few weeks. |
4 participants
Depends on #5252
ERC PR
NOTE
Bytes.sliceusesmcopythat was introduced in 0.8.25. This requirement extends to theCAIP2andCAIP10libraryPR Checklist
npx changeset add)