Skip to content

Conversation

@quetool
Copy link
Member

@quetool quetool commented Dec 17, 2025

Description

This PR implements Deposit with Exchange (DwE) functionality with a new Transfer endpoint integration, enabling users to swap/bridge tokens when needed during deposits. The implementation includes fee splitting support and a refactored DWE service architecture.

Version: 1.8.0-beta01

Key Changes

1. New Transfers Service

  • New Service: TransfersService (packages/reown_appkit/lib/modal/services/transfers/transfers_service.dart)
    • Handles quote generation for transfers, swaps, and bridges
    • Supports direct transfers (same chain, same asset)
    • Supports cross-chain transfers via /appkit/v1/transfers/quote endpoint
    • Includes quote status checking and exchange assets retrieval
    • Implements ITransfersService interface with methods:
      • getQuote() - Generates quotes for transfers/swaps/bridges
      • getQuoteStatus() - Checks status of transfer quotes
      • getExchangeAssets() - Retrieves available assets from exchanges

2. Quote Models

  • New Models: Comprehensive quote system with Freezed models
    • Quote - Main quote container with origin, destination, fees, steps, and timing
    • QuoteFee - Fee breakdown with id, label, amount, and currency
    • QuoteAmount - Amount representation with value and currency
    • QuoteStep - Union type for deposit and transaction steps
    • QuoteStatus - Enum for tracking status (waiting, pending, success, submitted, failure, refund, timeout)
    • QuoteType - Enum for quote types (direct-transfer)
    • Support for direct transfers and multi-step transfers
    • Fee breakdown with currency information
    • Status tracking with helper methods (isError, isSuccess)

3. Refactored DWE Service

  • Updated: DWEService (packages/reown_appkit/lib/modal/services/dwe_service/dwe_service.dart)
    • Integrated with new TransfersService via dependency injection
    • Renamed selectedAssetdepositAsset for clarity
    • Added depositAmountInUSD and depositAmountInAsset value notifiers
    • New loopOnTransferStatusCheck method for transfer status polling
    • Updated loopOnDepositStatusCheck to use QuoteStatus enum instead of string status
    • Enhanced configuration with filterByNetwork and depositAssetButton options
    • Support for configuredRecipients map per namespace
    • Improved error handling and status management
    • Default supported assets now includes all exchange assets

4. New UI Pages

  • Exchange Assets Selector Page (packages/reown_appkit/lib/modal/pages/public/dwe/exchange_assets_selector_page.dart)

    • Allows users to select source assets from exchanges
    • Real-time quote fetching when asset selection changes
    • Displays quote details including fees, estimated time, and exchange rates
    • Handles quote errors and loading states
    • Filters out same-chain swaps (not currently supported)
    • Shows quote breakdown with origin/destination amounts and fee details
  • Payment Process Page (packages/reown_appkit/lib/modal/pages/public/dwe/payment_process_page.dart)

    • Manages the payment flow after quote selection
    • Handles direct transfers and exchange deposits
    • Status tracking with visual feedback
    • Error handling and recovery
    • Opens exchange URLs for user payment
    • Polls for deposit/transfer status updates

5. Updated Base Implementation

  • AppKit Base: Enhanced to support transfer quotes

    • Updated AppKitBaseImpl to integrate transfer service
    • Added transfer-related methods and configurations
  • Exchange Service: Updated to work with new transfer flow

    • Enhanced asset model support for transfer operations
    • Updated query models for transfer compatibility
  • Asset Models: Extended to support transfer operations

    • Added methods for native token detection
    • Enhanced network and address handling

6. Version & Dependencies

  • Version bumped to 1.8.0-beta01
  • Updated dependencies in pubspec.yaml files
  • Updated CHANGELOG with new features:
    • "Deposit From Exchange enhancement: Swapping/Bridging tokens if needed"
    • Updated terminology from "Deposit with Exchange" to "Deposit From Exchange"

Technical Details

Transfer Flow

  1. User selects deposit asset and amount
  2. System fetches available exchange assets via getExchangeAssets
  3. User selects source asset from exchange
  4. System generates quote via getQuote:
    • If same chain and same asset → Direct transfer (no swap needed)
    • If different chain or asset → Cross-chain transfer/swap via transfers API
  5. User reviews quote with fees and timing
  6. User confirms and initiates payment
  7. System opens exchange URL for user to complete payment
  8. System tracks status via loopOnTransferStatusCheck or loopOnDepositStatusCheck
  9. Status updates displayed to user with appropriate UI feedback

Fee Splitting

  • Fees are included in quote responses with breakdown by type
  • Each fee includes:
    • id - Unique identifier for fee type
    • label - Human-readable label (e.g., "Service Fee")
    • amount - Fee amount in base units
    • currency - Currency/asset for the fee
  • Supports multiple fee types (service fees, network fees, etc.)
  • Fees displayed in UI for transparency

Direct Transfer Support

  • When source and destination are same chain and asset, uses direct transfer
  • Bypasses exchange/swapping logic
  • Faster execution path with minimal steps
  • No swap fees for direct transfers
  • Uses QuoteType.directTransfer for identification

Status Management

  • Unified QuoteStatus enum for both deposit and transfer status
  • Status polling with configurable intervals (3 seconds)
  • Maximum attempt limits (30 attempts) with timeout handling
  • Graceful error handling and user feedback
  • Support for cancellation and manual status checks

Files Changed

  • 52 files changed: 5,950 insertions(+), 644 deletions(-)
  • Major additions in transfers service directory:
    • transfers_service.dart - Main service implementation
    • i_transfers_service.dart - Service interface
    • quote_models.dart - Quote data models
    • quote_params.dart - Request parameter models
    • quote_results.dart - Response result models
    • transfers_utils.dart - Utility functions
  • Refactored DWE service and UI pages
  • Updated models and utilities
  • New UI components for exchange selection and payment processing

Breaking Changes

  • configDeposit method signature updated:
    • Removed preselectedRecipient parameter
    • Added filterByNetwork and depositAssetButton parameters
    • Changed configuredRecipients from single string to map per namespace

Testing Considerations

  • Test direct transfer flow (same chain, same asset)
  • Test cross-chain transfer flow
  • Test fee calculation and display
  • Test status polling and error handling
  • Test exchange asset selection and quote generation
  • Test configured recipients per namespace
  • Test filter by network functionality
  • Test deposit asset button visibility
  • Test quote error scenarios
  • Test status timeout and cancellation

How Has This Been Tested?

Due Dilligence

  • Breaking change
  • Requires a documentation update

@quetool quetool changed the title dwe logic refactor [DWE] Logic refactor to implement fee splitting through transfer endpoints Jan 8, 2026
@quetool quetool self-assigned this Jan 8, 2026
@quetool quetool marked this pull request as ready for review January 8, 2026 17:18
@claude
Copy link

claude bot commented Jan 8, 2026

Claude finished @quetool's task in 3m 13s —— View job


Code Review Complete

Reviewing PR #311: DWE Logic Refactor with Transfer Endpoints


Review Summary

This PR introduces a significant feature enhancement for Deposit with Exchange (DwE) by adding transfer/swap/bridge capabilities through a new TransfersService. The implementation is generally well-structured with proper use of Freezed models, dependency injection, and Flutter best practices. However, there are several critical issues that need attention before merging.


Critical Issues

1. Timestamp Unit Mismatch Bug 🐛

File: packages/reown_appkit/lib/modal/services/dwe_service/dwe_service.dart:526-533

bool _tokenPriceIsOld(int timestampSeconds) {
  final now = DateTime.now();
  final timestamp = DateTime.fromMillisecondsSinceEpoch(
    timestampSeconds * 1000,  // ❌ Incorrectly multiplies by 1000
  );
  final difference = now.difference(timestamp);
  return difference.inMinutes > 5;
}

The method parameter is named timestampSeconds but at line 514, it's being stored as milliseconds:

_fetchedTokens.add((asset, DateTime.now().millisecondsSinceEpoch));  // Milliseconds!

This causes the cache expiration logic to be completely broken. The timestamp gets multiplied by 1000 again, making it ~1000x too large, so cached tokens will never expire properly.

Fix: Either:

  • Change parameter name to timestampMilliseconds and remove the * 1000, OR
  • Store timestamps as seconds instead of milliseconds

Fix this →


2. Missing HTTP Status Code Validation 🔒

File: packages/reown_appkit/lib/modal/services/transfers/transfers_service.dart

Lines 152-168 and 181-189 don't validate HTTP status codes before parsing responses:

final response = await http.post(
  url.replace(queryParameters: _requiredParams),
  headers: _requiredHeaders,
  body: body,
);
final responseBody = response.body;
core.logger.d('[$runtimeType] -- quote response: $responseBody');

final responseData = jsonDecode(response.body) as Map<String, dynamic>;  // ❌ No status check

This could cause crashes when the API returns 4xx/5xx errors with non-JSON bodies. The code checks for error keys in the response, but that only works if JSON parsing succeeds.

Recommendation: Add status code validation:

if (response.statusCode != 200) {
  throw StateError('API error: ${response.statusCode} - ${response.body}');
}

Fix this →


3. parseUnits Edge Case: Empty Integer Part ⚠️

File: packages/reown_appkit/lib/modal/services/transfers/utils/transfers_utils.dart:28-56

Line 34 has a potential issue:

final integerPart = parts[0].replaceAll(RegExp(r'^[+-]?0*'), '');

If the input is "0.5", this regex removes ALL leading zeros including the significant zero, making integerPart = "" (empty). Then at line 46:

final combined = integerPart + adjustedDecimal;
if (combined.isEmpty) return BigInt.zero;  // This check happens AFTER combining

If integerPart is empty but adjustedDecimal has content, combined won't be empty, but BigInt.parse("") would have failed if called on empty integerPart alone.

Test cases to verify:

  • parseUnits("0.5", 18)
  • parseUnits(".5", 18)
  • parseUnits("00.5", 18)

Actually, looking closer, this should work because the empty integerPart concatenates with adjustedDecimal, so "" + "500..." becomes "500...". But it's a subtle edge case worth testing.


4. Race Condition in Status Polling ⚠️

Files:

  • dwe_service.dart:163-217 (loopOnDepositStatusCheck)
  • dwe_service.dart:222-275 (loopOnTransferStatusCheck)

Both methods use instance variables _isLooping and _shouldStopLooping without proper synchronization:

bool _isLooping = false;
bool _shouldStopLooping = false;

void loopOnDepositStatusCheck(...) async {
  if (_isLooping) return;  // ❌ Race condition
  _isLooping = true;
  _shouldStopLooping = false;
  // ... async loop
}

If two calls happen simultaneously (e.g., user rapidly clicks), both could pass the if (_isLooping) check before either sets it to true, causing multiple concurrent loops.

Impact: Potentially multiple status check loops running, wasting API calls and showing incorrect UI states.

Recommendation: Use a mutex/lock pattern or make the guard check-and-set atomic. Consider using a Completer or tracking the loop Future.


High Priority Issues

5. Inconsistent Error Handling

File: packages/reown_appkit/lib/modal/pages/public/dwe/exchange_assets_selector_page.dart:101-113

Two different error types are caught, but both result in the same generic error handling:

} on StateError catch (e) {
  ModalProvider.of(context).instance.onModalError.broadcast(ModalError(e.message));
  _quoteResult = null;
  _selectedExchangeAsset.value = null;
} on ArgumentError catch (e) {
  ModalProvider.of(context).instance.onModalError.broadcast(ModalError(e.message));
  _quoteResult = null;
  _selectedExchangeAsset.value = null;
}

Issues:

  1. The catch blocks are identical - combine them
  2. No logging of the error for debugging
  3. Generic error messages aren't user-friendly
  4. Should probably show a toast or snackbar to inform the user

6. Null Safety Issue in UI

File: packages/reown_appkit/lib/modal/pages/public/dwe/exchange_assets_selector_page.dart:447

Text(
  ' ${networkInfo!.name}',  // ❌ Force unwrap with !

Line 447 force-unwraps networkInfo without a null check. While line 397-403 retrieves it, there's no guarantee it's non-null. If getNetworkInfo returns null, this will crash.

Recommendation: Add null check or use networkInfo?.name ?? 'Unknown'


7. Same-Chain Swap Filtering Logic

File: packages/reown_appkit/lib/modal/pages/public/dwe/exchange_assets_selector_page.dart:55-60

// WE DON'T CURRENTLY SUPPORT SAME-CHAIN SWAPS
// SO WE REMOVE THE TOKENS THAT WOULD TRIGGER IT
allAssets.removeWhere((asset) {
  return asset.network == selectedAsset.network &&
      asset.address != selectedAsset.address;
});

This logic removes same-chain different-asset swaps, which is fine if not supported. However:

  1. UX Issue: If all assets are filtered out, the user sees "No assets supported for this network" without understanding why
  2. Silent failure: No indication to the user that swaps ARE available but not supported yet
  3. This seems like a temporary workaround that should be properly documented or have a better UI message

Medium Priority Issues

8. Hardcoded Network Fee Label Check

File: packages/reown_appkit/lib/modal/pages/public/dwe/exchange_assets_selector_page.dart:432

Visibility(
  visible: fee.label.toLowerCase() == 'network fee',  // ❌ Magic string

Hardcoded string comparison for special UI treatment. If the API changes the label, this breaks silently.

Recommendation: Define constants for fee types or use fee IDs instead of labels for logic.


9. Missing Timeout on HTTP Requests

All HTTP calls in TransfersService and DWEService lack timeout parameters. This could cause the app to hang indefinitely on slow/dead connections.

Recommendation: Add timeout:

final response = await http.post(
  url,
  headers: headers,
  body: body,
).timeout(Duration(seconds: 30));

10. Breaking Change Documentation

The PR description mentions breaking changes to configDeposit method signature, but:

  1. No migration guide is provided
  2. The PR checklist doesn't mark "Breaking change" as checked
  3. Should document how existing integrations need to update their code

11. parseUnits Negative Number Handling

File: packages/reown_appkit/lib/modal/services/transfers/utils/transfers_utils.dart:50-53

// Handle negative
if (value.startsWith('-')) {
  return -BigInt.parse(combined);
}

This doesn't properly handle the sign in the regex at line 34. If the input is "-5.0", the regex r'^[+-]?0*' removes the sign, but then line 50 checks if the original value starts with -. This works, but it's confusing because the sign is removed and then re-checked.

Recommendation: Better handling would be to extract sign first, then process absolute value.


Minor Issues & Code Quality

12. Commented Out Code

Multiple instances of large commented-out code blocks that should be removed:

  • payment_process_page.dart:66-78 - 12 lines of commented code for wallet deposits
  • appkit_modal_dwe_page.dart:103-111 - Commented recipient configuration
  • dwe_service.dart:456-463 - Commented state clearing code

Recommendation: Remove commented code or convert to TODO comments with issue references.


13. Magic Numbers

  • loopOnDepositStatusCheck: maxAttempts = 30, Duration(seconds: 3) = 90 seconds max
  • loopOnTransferStatusCheck: maxAttempts = 60, waitingInterval = 5 = 300 seconds max

These should be constants with clear names explaining the timeout strategy.


14. Typo in Comment

File: quote_models.dart:52

// pending: cgeck on 1st, and loaders on every other one  // ❌ "cgeck" -> "check"

15. Test Mock Methods in Production Code

File: dwe_service.dart:277-419

Three @visibleForTesting mock methods (loopOnStatusHappyPathMock, loopOnStatusUnhappyPathMock1, loopOnStatusUnhappyPathMock2) are included in production code. While marked for testing, they add ~140 lines of code that shouldn't be in the main service.

Recommendation: Move these to a separate test helper class or use proper mocking in tests.


16. Missing Input Validation

File: transfers_utils.dart:1-18

scaleAmountToBaseUnits validates format with regex but doesn't validate:

  • Negative amounts (should probably reject)
  • Maximum safe integer limits
  • Null/empty input (throws ArgumentError, which is fine)

Security Considerations

✅ No External Domain URLs Detected

Scanned all changed files - no external domain URLs found besides approved domains.

✅ URL Opening

ReownCoreUtils.openURL(urlResult.url) at payment_process_page.dart:90 opens exchange URLs. This is expected behavior but ensure:

  1. The API returns trusted URLs only (server-side validation)
  2. Consider adding URL validation/allowlist on client side

Testing Recommendations

The PR description has an empty "How Has This Been Tested?" section. Please add:

  1. Unit Tests Needed:

    • parseUnits edge cases (empty integer, leading zeros, negatives)
    • scaleAmountToBaseUnits edge cases
    • QuoteStatus.fromStatus all possible status strings
    • Quote model serialization/deserialization
    • Timestamp expiration logic (especially after fixing bug docs: fix spelling issues #1)
  2. Integration Tests Needed:

    • Full flow: select asset → get quote → process payment → status check
    • Error handling: API failures, timeout scenarios
    • Race condition testing: rapid clicks on payment buttons
    • Network switching during active transfers
  3. Manual Testing Scenarios:

    • Direct transfer (same chain, same asset)
    • Cross-chain transfer
    • Same-chain swap (should show appropriate message)
    • Quote errors and recovery
    • Status check timeout (5+ minutes)
    • Cancel/back during payment processing

Architecture & Design Feedback

✅ Strengths:

  1. Good separation of concerns: TransfersService, DWEService, and UI are well-separated
  2. Proper use of Freezed: Immutable models with union types for QuoteStep
  3. Dependency injection: GetIt usage is consistent
  4. Comprehensive models: Quote system has good structure with fees, steps, and status tracking

⚠️ Areas for Improvement:

  1. Error handling strategy: Should be more consistent across the codebase
  2. Status polling: Consider using WebSocket or Server-Sent Events for real-time updates instead of polling
  3. State management: Consider using a proper state management solution (Riverpod/Bloc) for complex status flows
  4. Retry logic: No automatic retry on transient failures

PR Checklist Issues

The PR description shows:

  • Breaking change - Should be checked (configDeposit signature changed)
  • Requires a documentation update - Should be checked (new API, breaking changes)

Recommendations Summary

Must Fix Before Merge:

  1. Fix timestamp unit mismatch bug (docs: fix spelling issues #1) ⚠️
  2. Add HTTP status code validation (Initial Release #2)
  3. Fix race condition in status polling (Bump rexml from 3.2.9 to 3.3.6 in /packages/reown_appkit/example/base/ios #4)

Should Fix Before Merge:
4. Improve error handling consistency (#5)
5. Add null safety checks (#6)
6. Improve same-chain swap filtering UX (#7)
7. Add HTTP request timeouts (#9)
8. Complete PR checklist and add testing documentation

Can Address in Follow-up:

  • Remove commented code
  • Extract constants for magic numbers
  • Move test mocks to separate files
  • Add comprehensive unit tests
  • Fix typos

Final Notes

This is a substantial feature addition that's generally well-implemented. The critical bugs (#1 and #2) must be fixed before merging as they could cause production issues. The race condition (#4) should also be addressed to prevent edge case failures.

Overall code quality is good with proper use of Flutter/Dart patterns. After addressing the critical issues and improving error handling, this will be ready for merge.

Estimated effort to address critical issues: 2-3 hours


quetool and others added 2 commits January 9, 2026 11:51
* test: Add comprehensive unit and integration tests for transfers service and DWE enhancements

- Add unit tests for TransfersService covering:
  * Direct transfer quote generation (same chain/asset)
  * Cross-chain transfer quotes
  * Quote status retrieval
  * Exchange assets retrieval
  * Error handling for invalid chain IDs

- Add unit tests for transfers utility functions:
  * parseUnits() - 11 test cases covering various scenarios
  * scaleAmountToBaseUnits() - 12 test cases including edge cases
  * DEAD_ADDRESSES_BY_NAMESPACE constant validation

- Add unit tests for quote models:
  * QuoteStatus enum conversion and properties
  * QuoteFeeExtension formatting
  * QuoteExtension formatting
  * QuoteStepExtension type checking

- Add unit tests for quote params and results:
  * GetQuoteParams serialization/deserialization
  * GetTransfersQuoteParams with optional fields
  * GetQuoteStatusParams
  * GetQuoteStatusResult and GetExchangeAssetsResult

- Add integration tests for DWEService.loopOnTransferStatusCheck():
  * Success status handling
  * Waiting/pending status polling behavior
  * Failure status handling
  * Timeout after max attempts
  * Stop checking functionality
  * Concurrent loop prevention
  * API error handling
  * Correct parameter passing

All tests follow existing patterns using mockito and flutter_test.
All linter errors have been resolved.

* Tests improvements with AI
@quetool quetool merged commit dac92e6 into develop Jan 15, 2026
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants