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

Flesh out misbehavior reports #237

Merged
merged 5 commits into from
Jan 3, 2025
Merged

Conversation

richardhuaaa
Copy link
Contributor

@richardhuaaa richardhuaaa commented Dec 20, 2024

  1. Fleshes out the misbehavior report types and fields
  2. Defines endpoints for storing and retrieving misbehavior reports

xmtp/xmtpd#302

@richardhuaaa richardhuaaa requested a review from a team December 20, 2024 22:12
@richardhuaaa richardhuaaa requested a review from a team as a code owner December 20, 2024 22:12
message MisbehaviorReport {
// Server time when the report was stored. Used only for querying reports.
// This field is not signed.
uint64 server_time_ns = 1;
Copy link
Contributor Author

@richardhuaaa richardhuaaa Dec 20, 2024

Choose a reason for hiding this comment

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

We need this separately to the reporter time (which may have client clock skew), so that the query endpoint can query all payloads larger than a certain timestamp. Could have used a sequence ID or a timestamp, decided to use a timestamp so that it's simpler, while also avoiding confusion with cursors in the main replication protocol

uint64 server_time_ns = 1;
bytes unsigned_misbehavior_report = 2;
// Signed by the node hosting the report
xmtp.identity.associations.RecoverableEcdsaSignature signature = 3;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the node's signature here. Alternative approaches:

  • No signature - it makes it a bit tricky to handle cases where a node retracts or denies a report it previously made. It also seems useful to know which node client reports came in through, as opposed to them going into one big pool.
  • Add a client signature - I think in most cases clients will want to stay anonymous. Adding your public key to a set of group payloads may end up deanonymizing a group.

When we define incentives for submitting reports, some entities may want to be rewarded. When we get there, perhaps we could add a claimant_public_key field to the UnsignedMisbehaviorReport. Probably don't need an additional signature for that.

message SubmitMisbehaviorReportResponse {}

message QueryMisbehaviorReportsRequest {
uint64 after_ns = 1;
Copy link
Contributor Author

@richardhuaaa richardhuaaa Dec 20, 2024

Choose a reason for hiding this comment

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

I think this is the minimum set of parameters we should be able to query by, but in the future it may make sense to add queries by misbehaving_node_id or misbehavior type or excuding client reports

@richardhuaaa richardhuaaa force-pushed the rich/misbehavior-reports branch from f8d181a to 490aa3a Compare December 20, 2024 22:45
@richardhuaaa richardhuaaa requested a review from neekolas January 3, 2025 00:23
@richardhuaaa richardhuaaa merged commit 0bc2969 into main Jan 3, 2025
4 checks passed
@richardhuaaa richardhuaaa deleted the rich/misbehavior-reports branch January 3, 2025 00:28
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.

2 participants