Add support for new envelope types #702
Conversation
WalkthroughThe pull request extends the envelope handling functionality by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant CE as ClientEnvelope
participant P as Payload
participant T as TopicKind
CE->>+CE: Invoke TopicMatchesPayload(P, targetTopic)
alt P is PayerReport
CE->>CE: Check targetTopic == TOPIC_KIND_PAYER_REPORTS_V1
else P is PayerReportAttestation
CE->>CE: Check targetTopic == TOPIC_KIND_PAYER_REPORT_ATTESTATIONS_V1
else
CE->>CE: Handle other payload types
end
CE-->>-CE: Return match result (true/false)
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/envelopes/envelopes_test.go (1)
138-144: Add positive test cases for new envelope types.While the negative test case for
PayerReportis good, consider adding:
- A positive test case for
PayerReportwith the correct topic kind- Both positive and negative test cases for
PayerReportAttestationThis would ensure comprehensive test coverage for both new payload types.
// Payer Report envelope with wrong topic clientEnvelope, err = NewClientEnvelope(&envelopesProto.ClientEnvelope{ Payload: &envelopesProto.ClientEnvelope_PayerReport{}, Aad: buildAad(topic.NewTopic(topic.TOPIC_KIND_GROUP_MESSAGES_V1, []byte{1, 2, 3})), }) require.NoError(t, err) require.False(t, clientEnvelope.TopicMatchesPayload()) + + // Payer Report envelope with correct topic + clientEnvelope, err = NewClientEnvelope(&envelopesProto.ClientEnvelope{ + Payload: &envelopesProto.ClientEnvelope_PayerReport{}, + Aad: buildAad(topic.NewTopic(topic.TOPIC_KIND_PAYER_REPORTS_V1, []byte{1, 2, 3})), + }) + require.NoError(t, err) + require.True(t, clientEnvelope.TopicMatchesPayload()) + + // Payer Report Attestation envelope with wrong topic + clientEnvelope, err = NewClientEnvelope(&envelopesProto.ClientEnvelope{ + Payload: &envelopesProto.ClientEnvelope_PayerReportAttestation{}, + Aad: buildAad(topic.NewTopic(topic.TOPIC_KIND_GROUP_MESSAGES_V1, []byte{1, 2, 3})), + }) + require.NoError(t, err) + require.False(t, clientEnvelope.TopicMatchesPayload()) + + // Payer Report Attestation envelope with correct topic + clientEnvelope, err = NewClientEnvelope(&envelopesProto.ClientEnvelope{ + Payload: &envelopesProto.ClientEnvelope_PayerReportAttestation{}, + Aad: buildAad(topic.NewTopic(topic.TOPIC_KIND_PAYER_REPORT_ATTESTATIONS_V1, []byte{1, 2, 3})), + }) + require.NoError(t, err) + require.True(t, clientEnvelope.TopicMatchesPayload())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/envelopes/client.go(1 hunks)pkg/envelopes/envelopes_test.go(1 hunks)pkg/topic/topic.go(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
pkg/envelopes/client.go (1)
pkg/topic/topic.go (2)
TOPIC_KIND_PAYER_REPORTS_V1(15-15)TOPIC_KIND_PAYER_REPORT_ATTESTATIONS_V1(16-16)
pkg/envelopes/envelopes_test.go (2)
pkg/envelopes/client.go (2)
NewClientEnvelope(17-36)ClientEnvelope(12-15)pkg/topic/topic.go (1)
NewTopic(43-48)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Code Review
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
- GitHub Check: Upgrade Tests
- GitHub Check: Test (Node)
- GitHub Check: Build pre-baked anvil-xmtpd
🔇 Additional comments (3)
pkg/envelopes/client.go (1)
84-87: LGTM: New payload type support correctly implemented.The implementation properly extends the
TopicMatchesPayloadmethod to handle the two new envelope types, maintaining consistency with the existing pattern by checking if the target topic kind matches the expected topic kind for each payload type.pkg/topic/topic.go (2)
15-16: LGTM: New topic kinds added correctly.The new constants follow the existing naming convention with the
_V1suffix and are added sequentially in the enumeration.
29-32: LGTM: String representations properly implemented.The string representations for the new topic kinds use snake_case formatting consistent with existing topic kinds and provide clear descriptive names.
neekolas
force-pushed
the
04-04-generate_report_ids
branch
from
1599a16 to
b5f8759
Compare
neekolas
force-pushed
the
04-04-add_support_for_new_envelope_types
branch
from
7d1bea7 to
79ffefb
Compare
TL;DR
Added support for payer reports and payer report attestations in the envelope system.
What changed?
TOPIC_KIND_PAYER_REPORTS_V1andTOPIC_KIND_PAYER_REPORT_ATTESTATIONS_V1to the topic packageTopicMatchesPayload()function to handle the new payload types:PayerReportandPayerReportAttestationHow to test?
Why make this change?
This change extends the envelope system to support payer reports and their attestations, which are needed for payment verification and reporting functionality. The envelope system needs to properly match these new payload types with their corresponding topics to ensure messages are routed correctly.
Summary by CodeRabbit
New Features
Tests