-
Notifications
You must be signed in to change notification settings - Fork 24
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
Use message ID utility #160
Use message ID utility #160
Conversation
748c9a9
to
3a62f05
Compare
@@ -142,6 +141,7 @@ func (m *messageManager) ShouldSendMessage(unsignedMessage *warp.UnsignedMessage | |||
} | |||
|
|||
// Check if the message has already been delivered to the destination chain | |||
teleporterMessenger := m.getTeleporterMessenger(destinationClient) |
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 is just moved down from higher up
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress) | ||
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress, 2) |
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 will be removed from this PR. Just needed to put anything here for it to compile. This will be solved in Cam's PRs.
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 this can be updated now.
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress) | ||
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress, 2) |
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 will be removed from this PR. Just needed to put anything here for it to compile. This will be solved in Cam's PRs.
@@ -186,8 +186,7 @@ func (m *messageManager) SendMessage(signedMessage *warp.Message, destinationBlo | |||
return fmt.Errorf("relayer not configured to deliver to destination. DestinationBlockchainID=%s", destinationBlockchainID) | |||
} | |||
|
|||
teleporterMessenger := m.getTeleporterMessenger(destinationClient) | |||
teleporterMessageID, err := m.calculateMessageID(teleporterMessenger, signedMessage.SourceChainID, destinationBlockchainID, teleporterMessage.MessageNonce) | |||
teleporterMessageID, err := teleporterutils.CalculateMessageID(m.protocolAddress, signedMessage.SourceChainID, destinationBlockchainID, teleporterMessage.MessageNonce) |
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 previously decided to leave the log in CalculateMessageID
, can we add logs for handling these errors with the chagned function
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've wrapped the error here so it's clear where the error is coming from. We log any errors outside of this function, so they will get logged. I'd like to talk about our error logging in this repo in standup today if there's time.
…bs/awm-relayer into gstuart/integrate-message-id-util
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.
LGTM after updating the one spot you called out.
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress) | ||
unsignedMessage, warpEnabledChainConfigC := utils.InitChainConfig(networkID, cChainInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigA := utils.InitChainConfig(networkID, subnetAInfo, newProtocolAddress, 2) | ||
_, warpEnabledChainConfigB := utils.InitChainConfig(networkID, subnetBInfo, newProtocolAddress, 2) |
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 this can be updated now.
The merge here was a little messy so I just transposed these changes into a different PR #163 |
Why this should be merged
Closes #136
How this works
Uses a utility in the Teleporter repo to calculated the Teleporter Message ID without having to make an API call to a node.
How this was tested
Tested in the Teleporter repo.