Do not allow USDC as ticker in request-loan

[Kailai-Wang]

Context

As topic - a small extra check

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
heima	Ready	Preview	Comment	Nov 5, 2025 11:29am
heima-aa-demo-app	Ready	Preview	Comment	Nov 5, 2025 11:29am

[claude]

Claude finished @Kailai-Wang's task —— View job

---

PR Review: "Do not allow USDC as ticker in request-loan"

I've completed my review of this PR. Here's my comprehensive feedback:

✅ Todo List

• Read the modified test file to understand changes
• Find and analyze the actual implementation code
• Review code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

---

📋 Summary

This PR adds a simple validation to prevent USDC from being used as collateral in loan requests by adding a 4-line check in the precheck_params function at request_loan_test.rs:309-311.

✅ Code Quality & Best Practices

Positive aspects:

• Clean implementation: The validation follows existing patterns in the function
• Proper error handling: Uses the established DetailedError::parse_error pattern consistent with other validations
• Case-insensitive comparison: Using .to_uppercase() handles various input cases
• Logical placement: Added in the parameter validation section where it belongs

🔍 Technical Analysis

Business Logic Understanding:
After analyzing the code flow, I understand why USDC cannot be used as collateral:

1. Loan Process: The system sells collateral tokens in spot market to receive USDC
2. USDC Output: The process generates USDC as the loan proceeds (usdc_sold_f64 = usdc_from_spot_fill(&spot_sell_fill))
3. Circular Logic: Using USDC as collateral would mean selling USDC to get USDC, which doesn't make business sense

Code Flow Analysis:

• request_loan_test.rs:120-148: Sells collateral → receives USDC
• request_loan_test.rs:496-497: Calculates USDC received from spot fill
• The entire system is designed around non-USDC collateral being converted to USDC

🐛 Potential Issues

Minor:

• Error Message: "Expect non-USDC collateral_ticker" could be more descriptive. Consider: "USDC cannot be used as collateral"

No Critical Issues Found:

• No race conditions or security vulnerabilities
• No performance impacts
• Input validation is appropriate

🔒 Security Assessment

• Input Validation: ✅ Proper validation prevents invalid business logic
• Error Handling: ✅ Follows established security patterns
• No Injection Risks: ✅ Simple string comparison, no user data interpolation

⚡ Performance Considerations

• Minimal Impact: ✅ Single string comparison with negligible overhead
• Early Validation: ✅ Fails fast before expensive operations

🧪 Test Coverage Concerns

Missing Test Coverage:

• No unit tests verify the USDC rejection behavior
• Should add test cases for:

  #[test]
  fn test_rejects_usdc_collateral() {
      // Test "USDC", "usdc", "Usdc" all get rejected
  }

Existing Validation Tests:

• File has comprehensive validation for other parameters
• Test framework appears to be in place via #[cfg(feature = "test-endpoints")]

🎯 Recommendations

1. Improve Error Message (request_loan_test.rs:310):

   return Err(DetailedError::parse_error("USDC cannot be used as collateral").to_rpc_error());

2. Add Unit Tests: Consider adding test coverage for this validation

3. Documentation: The business logic rationale could be documented in code comments

📊 Overall Assessment

Rating: ✅ APPROVE

This is a well-implemented, necessary business logic validation that:

• ✅ Prevents invalid loan configurations
• ✅ Follows established code patterns
• ✅ Has minimal performance impact
• ✅ Maintains security best practices

The change is small, focused, and addresses a legitimate business requirement. The implementation is clean and consistent with the existing codebase.

---