Integrate Teleporter message id changes #128
Conversation
fix unit tests update mock files
minghinmatthewlam
force-pushed
the
integrate-teleporter-updates
branch
from
443cd48 to
2ecf959
Compare
minghinmatthewlam
changed the title
minghinmatthewlam
requested review from
michaelkaplan13,
cam-schultz,
gwen917,
geoff-vball and
bernard-avalabs
as code owners
| @@ -125,29 +124,31 @@ func (m *messageManager) ShouldSendMessage(warpMessageInfo *vmtypes.WarpMessageI | |||
| return false, fmt.Errorf("relayer not configured to deliver to destination. destinationBlockchainID=%s", destinationBlockchainID.String()) | |||
| } | |||
|
|
|||
| teleporterMessenger := m.getTeleporterMessenger(destinationClient) | |||
| teleporterMessageID, err := m.calculateMessageID(teleporterMessenger, warpMessageInfo.WarpUnsignedMessage.SourceChainID, destinationBlockchainID, teleporterMessage.MessageNonce) | |||
There was a problem hiding this comment.
It a bit sub-optimal that getting the message ID requires an additional API call here, but given the current interfaces and utilities, it's I agree that it's the easiest option.
Alternatively, the message ID is emitted as a part of the SendCrossChainMessage event and could be used directly from there (though we don't currently subscribe or otherwise get that event), or we could provide a Golang utility for the hash operation, which would be far more efficient than an API call but have a small amount of duplicated logic.
I don't think we need to change it for this PR, but wanted to call out to hear people's thoughts.
There was a problem hiding this comment.
Yeah investigated this a bit and unfortunately there is no way to call any methods from the golang ABI without an RPC call, even if you're calling a pure function. I totally agree that needing the API here is very undesired.
I think the best solution here is probably to build our own Golang utility that reproduces the functionality. Importantly, we should add a test (or include the sanity check in the e2e network setup) that calls the Teleporter contract to make sure it aligns with our utility.
We can totally defer this to a new ticket, though.
There was a problem hiding this comment.
Also, the utility should be located in the teleporter repo.
There was a problem hiding this comment.
created new issue to track: #136
There was a problem hiding this comment.
Agreed on all points. In the future when optimizing end to end relayer latency, minimizing the number of network round trips will be an obvious starting point.
Why this should be merged
updates relayer to be compatible with Teleporter message id changes, fixes #123
How this works
We use the
abi-bindingspkg fromteleporterrepo and the destination client to create a Teleporter instance. Then when relaying a Teleporter message we use the instance to calculate message id and check message received when deciding whether we should send a message.How this was tested
Updated unit/e2e tests
CI
How is this documented
Updated code comments