Skip to content

Fix/1184 1185 1186 - #1229

Merged
Maki-Zeninn merged 3 commits into
Maki-Zeninn:mainfrom
Barbieple-Devstem:fix/1184-1185-1186
Aug 30, 2026
Merged

Fix/1184 1185 1186#1229
Maki-Zeninn merged 3 commits into
Maki-Zeninn:mainfrom
Barbieple-Devstem:fix/1184-1185-1186

Conversation

@Barbieple-Devstem

Copy link
Copy Markdown
Contributor

Multi-Issue Fix: Integration Tests, Fee Tier Limits, and Route Fee Tiers API

Closes #1184
Closes #1185
Closes #1186

Summary

This PR addresses three related bugs and improvements across the stellar-router suite:

  1. Integration test compatibility - Fixes stale function signatures in test call sites
  2. Storage safety - Adds tier count validation to prevent unbounded storage growth
  3. API consistency - Converts fee tier getter to return Result type with proper error handling

Changes

Issue #1184: Fix RouterRegistry::deprecate() Integration Test Calls

Problem: Integration test call sites were invoking RouterRegistry::deprecate() with 3 arguments, but the current function signature requires 4
parameters including a reason: Option<String> field.

Impact: Prevented compilation of test_core_route_updated_after_registry_version_bump and test_registry_double_deprecate_fails tests.

Solution:

  • Updated integration-tests/tests/cross_contract_tests.rs (line 97)
  • Updated integration-tests/tests/failure_scenarios.rs (lines 161 and 163)
  • All call sites now include the missing reason argument: &None::<String>
  • Aligned with established pattern from router-registry's own test suite

Files Modified:

  • integration-tests/tests/cross_contract_tests.rs
  • integration-tests/tests/failure_scenarios.rs

Issue #1185: Add MAX_FEE_TIERS_PER_ROUTE Validation

Problem: The set_route_fee_tiers function in router-quote accepted arbitrarily large tier vectors without validation. While route tracking is
capped at MAX_TRACKED_ROUTES (500) to prevent unbounded storage growth, individual route tier lists had no equivalent limit.

Security/Performance Impact:

  • Admins could exploit this to cause unbounded storage growth
  • O(n²) insertion-sort complexity with oversized tier lists
  • Linear scan overhead in resolve_route_fee_bps degraded quote performance

Solution:

  • Added new error type: QuoteError::TooManyTiers (discriminant 11)
  • Implemented MAX_FEE_TIERS_PER_ROUTE constant (set to 100 tiers per route)
  • Added early validation in set_route_fee_tiers() to check tier count
  • Returns QuoteError::TooManyTiers if limit exceeded
  • Mirrors existing guard pattern used in track_configured_route for consistency

Files Modified:

  • contracts/router-quote/src/lib.rs

Issue #1186: Standardize get_route_fee_tiers API with Result Type

Problem: The get_route_fee_tiers function lacked consistency with other public getter methods. While functions like get_route_fee,
get_default_fee, admin, and get_quote all return Result<T, QuoteError> and properly signal initialization status, get_route_fee_tiers returned
a bare Vec<FeeTier>.

Impact:

Solution:

  1. Changed function signature: Vec<FeeTier>Result<Vec<FeeTier>, QuoteError>
  2. Added initialization check: Verifies Admin data key exists (same approach as get_default_fee)
  3. Proper error signaling: Returns QuoteError::NotInitialized when contract uninitialized
  4. Updated all callers:
    • resolve_route_fee_bps() - Uses ? operator for error propagation
    • get_all_configured_routes() - Handles Result type with fallback to empty vector
  5. Updated tests:
    • test_get_route_fee_tiers_returns_empty_when_not_set - Now asserts Ok with empty vector
    • test_get_route_fee_tiers_returns_sorted_tiers - Unwraps Result and verifies content

Files Modified:

  • contracts/router-quote/src/lib.rs

Technical Details

Code Quality

  • ✅ Consistent error handling across all getters
  • ✅ Early validation prevents cascading errors
  • ✅ Clear error types for different failure modes
  • ✅ Mirrors established patterns (MAX_TRACKED_ROUTES, require_admin_simple!)
  • ✅ All tests updated to handle new signatures

Security Considerations

  • Tier count limit prevents storage DoS attacks
  • Proper initialization checks prevent access to uninitialized contracts
  • Error propagation enables proper error handling up the call stack

Breaking Changes

  • get_route_fee_tiers() return type changed to Result (breaking API change for public interface)
  • QuoteError::TooManyTiers added with discriminant 11

Testing

All changes have been validated against existing test suite:

  • Integration tests pass with updated function signatures
  • Unit tests updated to handle Result type
  • Error paths tested through try_* variants
  • Backward compatibility maintained for other getters

Related Issues

Closes #1184
Closes #1185
Closes #1186


…:deprecate() calls

- Update cross_contract_tests.rs line 97 to include reason parameter
- Update failure_scenarios.rs lines 161 and 163 to include reason parameter
- Fixes stale signature calls to deprecate() which now requires 4 arguments
…nbounded storage growth

- Add new QuoteError::TooManyTiers (discriminant 11) for tier limit violations
- Add MAX_FEE_TIERS_PER_ROUTE constant set to 100 tiers per route
- Validate tiers.len() early in set_route_fee_tiers() function
- Prevents O(n²) insertion-sort complexity and storage growth issues
- Mirrors existing guard pattern used in track_configured_route for routes
…pe with initialization check

- Change function signature from Vec<FeeTier> to Result<Vec<FeeTier>, QuoteError>
- Add initialization check: verify Admin data key exists before returning
- Return QuoteError::NotInitialized when contract hasn't been initialized
- Update resolve_route_fee_bps to use ? operator for error propagation
- Update get_all_configured_routes to handle Result type
- Update test_get_route_fee_tiers_returns_empty_when_not_set test
- Update test_get_route_fee_tiers_returns_sorted_tiers test
- Provides consistency with other public getters like get_default_fee
@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@Barbieple-Devstem Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Maki-Zeninn
Maki-Zeninn merged commit b0686b7 into Maki-Zeninn:main Aug 30, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment