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

[Gateway] Misc fixes to Message parsing, signing and validation #9752

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

bolekk
Copy link
Contributor

@bolekk bolekk commented Jul 10, 2023

  1. Set Message.Sender as a non-serializable field.
  2. Add Message.Receiver field. Because messageID is scoped to a user, nodes need to set receiver to that user’s address to make sure responses can be uniquely attributed to users and also to prove to the user that it is in fact a response to their message.
  3. Fix ValidateSignature(). I mistakenly assumed that previous logic was enough to fully validate a signature but we always need to validate the expected sender. Renamed the function to ExtractSigner() to avoid any confusion.
  4. Perform message validation before calling handler methods, as suggested by @pinebit. Even though there are theoretical use cases that might not need it, I don’t think they are going to be implemented in the near future.
  5. Changing HandleGatewayMessage() attribute back to full message instead of just body. This will be needed for a very real use case, where nodes want to share received messages among each other, within the OCR round. It’s also consistent with the Gateway Handler interface.
  6. Fix broken scripts.

@github-actions
Copy link
Contributor

I see that you haven't updated any README files. Would it make sense to do so?

1. Set Message.Sender as a non-serializable field.
2. Add Message.Receiver field. Because messageID is scoped to a user, nodes need to set receiver to that user’s address to make sure responses can be uniquely attributed to users and also to prove to the user that it is in fact a response to their message.
3. Fix ValidateSignature(). I mistakenly assumed that previous logic was enough to fully validate a signature but we always need to validate the expected sender. Renamed the function to ExtractSigner() to avoid any confusion.
4. Perform message validation before calling handler methods, as suggested by @pinebit. Even though there are theoretical use cases that might not need it, I don’t think they are going to be implemented in the near future.
5. Changing HandleGatewayMessage() attribute back to full message instead of just body. This will be needed for a very real use case, where nodes want to share received messages among each other, within the OCR round. It’s also consistent with the Gateway Handler interface.
6. Fix broken scripts.
@bolekk bolekk force-pushed the chore/gateway-message-fixes branch from 7801d46 to 5d79e45 Compare July 10, 2023 05:14
@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate failed

Failed condition 71.0% 71.0% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

Copy link
Contributor

@KuphJr KuphJr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bolekk bolekk added this pull request to the merge queue Jul 10, 2023
Merged via the queue into develop with commit ee5ce33 Jul 10, 2023
@bolekk bolekk deleted the chore/gateway-message-fixes branch July 10, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants