[MASTER] Proto Inconsistencies - Pre-Launch Fixes Required

[manojradhakrishnan]

Proto Inconsistencies Master Ticket

Overview

This ticket tracks all identified inconsistencies across the protobuf definitions for the Universal Connector Service (UCS). These issues were identified during a comprehensive proto review and need to be addressed before launch.

---

Critical Issues (P0 - Blockers)

1. Missing Field Numbers (Critical)

• File: payment_methods.proto
• Line: 1375
• Issue: BankNames enum missing value 46 (gap between 45 and 47)
• Impact: Potential serialization issues, field number conflicts

2. Inconsistent Package Naming

• Files: health_check.proto uses grpc.health.v1, others use types
• Issue: Mixed package naming conventions
• Impact: Code generation inconsistency, import confusion

3. Duplicate Payment Method Definitions

• File: payment_methods.proto
• Issue:
  • ClassicReward defined twice (lines 80, 1302-1304)
  • EVoucher/e_voucher naming inconsistency
  • PaySafeCard/pay_safe_card field naming inconsistency
• Impact: Code generation conflicts, API confusion

---

High Priority Issues (P1)

4. Inconsistent Field Number Sequences

• File: payment_methods.proto
• PaymentMethod oneof field numbers:
  • Lines 45-47: Card methods at 3,4,7 (skips 5,6)
  • Lines 93-95: bizum = 156, eft = 157 (jump from 59)
  • Comment says "GIFT CARDS (130-139)" but Givex at 137, PaySafeCard at 138
• Impact: Hard to maintain, error-prone for future additions

5. Inconsistent Request/Response Naming

• File: payment.proto
• Inconsistencies:
  • PaymentServiceAuthorizeRequest vs RefundServiceGetRequest (missing "Service" in some)
  • Some use Service prefix, others don't
  • Example: DisputeServiceSubmitEvidenceRequest vs EventServiceHandleRequest
• Impact: API discoverability, documentation clarity

6. Inconsistent Status Enums

• File: payment.proto
• Issues:
  • PaymentStatus vs OperationStatus vs AuthorizationStatus
  • Mixed naming conventions across services
  • Some have *_UNSPECIFIED = 0, naming patterns vary
• Impact: Developer confusion when handling responses

7. Inconsistent Amount Field Naming

• Files: Multiple
• Issues:
  • Some use amount (Money type)
  • Some use payment_amount (int64)
  • Some use *_amount suffix, others don't
  • Examples:
    • PaymentServiceRefundRequest: payment_amount (int64) and refund_amount (Money)
    • RecurringPaymentServiceChargeRequest: amount (Money) and original_payment_authorized_amount (Money)
• Impact: Inconsistent API usage, type confusion

---

Medium Priority Issues (P2)

8. Optional vs Required Field Inconsistency

• File: payment.proto
• Examples:
  • Line 1547: optional bool all_keys_required= 4; (no space before =)
  • Line 1602: optional bool all_keys_required= 4; (inconsistent spacing)
  • Some IDs are required, others optional inconsistently
• Impact: API contract clarity, proto style guide violations

9. Commented Out Code Blocks

• File: payment_methods.proto
• Lines: 167-275 (multiple commented sections)
• Issue: Large blocks of commented code for TODO items
• Impact: Cluttered proto files, maintenance burden

10. Inconsistent Import Structure

• Files: Various
• Issues:
  • Some files import payment.proto but don't use it
  • Import ordering inconsistent
  • Some use fully qualified names, others relative

11. Missing Documentation

• Files: All proto files
• Issues:
  • Many messages lack comprehensive comments
  • Field descriptions inconsistent
  • Some enums have no documentation

---

Low Priority Issues (P3)

12. Whitespace and Formatting

• File: payment_methods.proto
• Issues:
  • Inconsistent spacing around = (e.g., line 1563: BancontactCard bancontact_card = 56; vs line 909: optional string blik_code = 1;)
  • Inconsistent blank lines between sections
  • Some comments use //, others use /* */

13. Enum Naming Conventions

• Files: Multiple
• Issues:
  • Some enums use PascalCase, others UPPER_SNAKE_CASE
  • Some prefixes match type name, others don't
  • Example: BANK_NAMES_UNSPECIFIED vs CARD_NETWORK_UNSPECIFIED

14. SecretString Usage Inconsistency

• Files: Multiple
• Issues:
  • Some sensitive fields use SecretString, others use plain string
  • Inconsistent application across similar fields
  • Example: email sometimes SecretString, sometimes plain string

---

Tracking

Issue	Priority	Status	Assignee	PR
#1 Missing Field Numbers	P0	🔴 Open	-	-
#2 Package Naming	P0	🔴 Open	-	-
#3 Duplicate Definitions	P0	🔴 Open	-	-
#4 Field Number Sequences	P1	🔴 Open	-	-
#5 Naming Conventions	P1	🔴 Open	-	-
#6 Status Enums	P1	🔴 Open	-	-
#7 Amount Fields	P1	🔴 Open	-	-
#8 Optional Fields	P2	🔴 Open	-	-
#9 Commented Code	P2	🔴 Open	-	-
#10 Imports	P2	🔴 Open	-	-
#11 Documentation	P2	🔴 Open	-	-
#12 Formatting	P3	🔴 Open	-	-
#13 Enum Naming	P3	🔴 Open	-	-
#14 SecretString	P3	🔴 Open	-	-

---

Related Resources

• Previous proto review session: Claude Session
• Proto files location: /backend/grpc-api-types/proto/
• Main files affected:
  • services.proto
  • payment.proto
  • payment_methods.proto
  • composite_payment.proto
  • composite_service.proto
  • sdk_config.proto
  • health_check.proto

---

Next Steps

1. Review and prioritize issues with the team
2. Create sub-tickets for each P0/P1 issue
3. Define proto style guide to prevent future inconsistencies
4. Set up automated proto linting in CI

cc: Team leads for review