Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Check if message already delivered #23
Check if message already delivered #23
Changes from 2 commits
19bf939
b9f9f68
aaad98c
6505639
8a4d9c2
08cdef5
3526d03
03267da
63e65f8
25ed312
7079153
cf4c210
0a1a368
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should relayers be destination specific?
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.
At the code level, this particular check is ensuring that the relayer EOA that will send the tx to the destination is included in the Teleporter message's list of allowed relayers. The function above checks that the EVM client that will send the tx is sending to the destination chain ID included in the Teleporter message.
At the architectural level,
DestinationClient
is the entity responsible for sending the tx to the correct destination chain.MessageRelayer
s should be composed of exactly oneDestinationClient
, so in that sense, relayers are destination specific.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 check seems redundant given that we look up the destination client by the chain ID already. If we choose to keep it, could we move this to right after the
destinationClient
is looked up from thedestinationClients
map?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.
Agreed. I've removed it altogether, but left the corresponding method in
DestinationClient
, as it makes sense as a getter for that interface.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.
It looks like the caller should look at the err to distinguished between the message has been received and error conditions. Do the callers do that currently? How should the caller handle errors?
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 call to
ShouldSendMessage
is valid regardless of the value of the boolean, as long as the error is nil. We perform that check here: https://github.com/ava-labs/awm-relayer/blob/2060ab680cbd1a2501a87ac3ec01d6cd9da79f37/relayer/message_relayer.go#L80-L95I added a comment to the
MessageManager
interface noting that the boolean should be ignored if a error is returned.