Skip to content
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

Closed

Conversation

geoff-vball
Copy link
Contributor

@geoff-vball geoff-vball commented Jan 31, 2024

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.

@geoff-vball geoff-vball force-pushed the gstuart/integrate-message-id-util branch from 748c9a9 to 3a62f05 Compare February 1, 2024 13:35
@@ -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)
Copy link
Contributor Author

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

Comment on lines -61 to +63
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)
Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Comment on lines -61 to +63
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)
Copy link
Contributor Author

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)

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

Copy link
Contributor Author

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.

Copy link
Collaborator

@michaelkaplan13 michaelkaplan13 left a 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.

Comment on lines -61 to +63
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)
Copy link
Collaborator

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.

@geoff-vball
Copy link
Contributor Author

The merge here was a little messy so I just transposed these changes into a different PR #163

@geoff-vball geoff-vball closed this Feb 2, 2024
@geoff-vball geoff-vball deleted the gstuart/integrate-message-id-util branch March 7, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants