Draft
Conversation
Contributor
Summary of ChangesHello @gemdev111, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new 'Earn' feature to the application, enabling users to participate in yield-generating activities. This involved a significant refactoring of the existing 'Staking' module into a more generalized 'Earn' module, along with extensive updates across the UI, view models, data models, persistence layer, and transaction signing logic to support the new functionality. The changes aim to provide users with a comprehensive interface for managing their earn positions and interacting with various yield providers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Earn' feature, providing a unified interface for yield-generating activities. The implementation includes new scenes, view models, and service layers, along with necessary database migrations and signing logic for EVM chains. While the architecture is consistent with the rest of the project, there are critical safety issues in the database migrations and potential runtime crashes in the view models that should be addressed.
| } | ||
|
|
||
| migrator.registerMigration("Add earn support") { db in | ||
| try? db.alter(table: AssetRecord.databaseTableName) { |
Contributor
There was a problem hiding this comment.
Using try? in database migrations is highly discouraged. If a migration fails (e.g., due to a schema conflict or database lock), the failure is silently ignored, leaving the database in an inconsistent state. Subsequent code that relies on these schema changes will likely crash or behave unpredictably. Use try to ensure that migration errors are caught and handled by the DatabaseMigrator. This pattern should be corrected on lines 397, 404, and 411 as well.
Suggested change
| try? db.alter(table: AssetRecord.databaseTableName) { | |
| try db.alter(table: AssetRecord.databaseTableName) { |
|
|
||
| var depositDestination: AmountInput { | ||
| AmountInput( | ||
| type: .earn(.deposit(provider: providers.first!)), |
Contributor
There was a problem hiding this comment.
Force unwrapping providers.first! is unsafe. Although the UI visibility is currently controlled by showDeposit, this property is public and could be accessed by other components or tests when providers is empty, causing a crash. Consider making depositDestination optional or providing a safe fallback.
| func fetch() async { | ||
| viewState = .loading | ||
| do { | ||
| let address = (try? wallet.account(for: asset.id.chain))?.address ?? "" |
Contributor
There was a problem hiding this comment.
If the wallet does not contain an account for the asset's chain, address will be an empty string. Calling earnService.update with an empty address is likely to result in an invalid API request or incorrect data fetching. It is safer to guard against a missing account and return early.
Suggested change
| let address = (try? wallet.account(for: asset.id.chain))?.address ?? "" | |
| guard let address = (try? wallet.account(for: asset.id.chain))?.address else { return } |
Move AmountInputAction to Primitives Add providerType to store and earn DB queries
Update imports for Staking to Earn rename Add earn FFI mappings Refactor AmountType with StakeAmountType Add earn transaction types and exhaustiveness fixes Add earn scenes and navigation Add earn balance and metadata support Add earn button to asset detail screen Add earn asset handling and validator UI Introduce assetRequest and assetData to EarnSceneViewModel and observe asset updates in EarnNavigationView to surface asset metadata (e.g. earn APR). Use assetData.metadata.earnApr as a fallback when computing APR. Rename showEarn -> showEarnBalance and update related computed properties (showBalances, showEarnButton) and inline the stake header link in AssetScene while adding accessibility identifiers for stake and earn. Update Validator views: map earn providers to YieldProvider display name in StakeValidatorViewModel and make ValidatorImageView public to show provider images (earn) or validator images (stake). Also update AmountScene to display the earn provider in the amount input section. Regenerate typeshare types and fix callers Add getEarnData gateway method Move RecipientNavigationView to Gem target Add EarnData to earn transaction flow Add loading indicator to amount toolbar
Refactor StakeDelegation to DelegationViewModel Replace the old StakeDelegationViewModel/view/header with a unified DelegationViewModel and DelegationView. Key changes: - Deleted StakeDelegationViewModel, StakeDelegationView, and ValidatorHeaderViewModel. - Introduced DelegationViewModel changes (adds ExplorerService, validatorUrl) and new unit tests (DelegationViewModelTests). - Updated StakeScene to use DelegationView and model.navigationDestination(for:) instead of the old types. - StakeSceneViewModel now accepts a currencyCode, maps delegations to DelegationViewModel, and exposes navigationDestination(for:) helper. - StakeDetailSceneViewModel updated to use DelegationViewModel and DelegationHeaderViewModel. - ViewModelFactory now supplies Preferences.standard.currency when building stake view models. - Submodule 'core' reference bumped. These changes consolidate delegation presentation and navigation logic, centralize currency handling, and wire explorer URLs for validators. Introduce ValidatorViewModel and refactor validator logic Centralize validator representation by adding ValidatorViewModel and moving name/image/url/apr logic into it. Remove legacy view models and helpers (StakeValidatorViewModel, YieldProviderViewModel, DelegationHeaderViewModel, DelegationState+Staking) and update DelegationViewModel to use ValidatorViewModel (also conforming DelegationViewModel to HeaderViewModel). Update views and view models (ValidatorImageView, ValidatorSelectionView, ValidatorView, StakeValidatorsViewModel, EarnDetail/StakeDetail scenes) to construct and consume ValidatorViewModel. Rename and update tests accordingly (StakeValidatorViewModelTests -> ValidatorViewModelTests). Misc: small formatting/import adjustments and use of DelegationStateViewModel for state title.
Rename StakeDetailScene to DelegationDetailScene and introduce DelegationDetailSceneViewModel to unify stake and earn provider handling. Add DelegationDetailActionType and consolidate availableActions/action handling into the new view model (ForEach-driven action list). Remove legacy EarnDetailScene, StakeDetailSceneViewModel and EarnDetailSceneViewModel. Update DelegationStateViewModel to expose color and textStyle and refactor DelegationViewModel to use it. Wire navigation and factory: update EarnNavigationView/StakeNavigationView and ViewModelFactory to use delegationDetailScene and pass validators. Adjust tests and mocks to the new DelegationDetailSceneViewModel.
Replace request/value pairs with ObservableQuery in EarnSceneViewModel (assetQuery, positionsQuery, providersQuery) and expose computed properties (assetData, positions, providers). Update initial query values. Update UI bindings: EarnNavigationView now uses .bindQuery(...) and removes an unused Store import. Rename StakeDetailSceneViewModelTests to DelegationDetailSceneViewModelTests. Update DelegationDetailScene to call model.onSelectAction(...) instead of performAction. Add earn-related defaults to AssetData (isEarnEnabled, earnApr).
gemdev111
force-pushed
the
gem-ios-earn-facade
branch
from
7052a7f to
f4ff1f9
Compare
Add EarnService support across the transaction system: handle .earn transaction types in TransactionFactory, propagate earnService through ServicesFactory into TransactionStateService, and wire it into TransactionPostProcessingService to call earnService.update for earnDeposit/earnWithdraw. Update package manifest to include EarnService and test dependencies, and extend the TransactionStateService test kit with an EarnService.mock provider. These changes enable processing and post-processing of earn-related transactions.
Expose and propagate 'earn' balances throughout the stack: added ChainBalanceable.getEarnBalance and a default empty implementation; implemented getEarnBalance in GatewayService and GatewayChainService to forward and map gateway responses; updated ChainServiceMock to stub earn balances. BalanceFetcher can now request earn balances, and BalanceService schedules and handles earn balance updates (added updateEarnBalance and a task to fetch/update earn balances, mapping via .earnChange). These changes enable retrieval and processing of earn/yield balances alongside existing coin/stake/token balances.
Simplify and tighten Earn-related APIs: remove the chain parameter from EarnService.getEarnData and its protocol, add an assetIds parameter to GatewayService.earnPositions, and make GatewayService.earnProviders synchronous and non-throwing (using compactMap). Update EarnService to fetch providers (non-throwing) then request positions for the specific asset, and adjust callers/test mocks (AmountEarnViewModel and MockEarnService) to match the new signatures. Also updates the core submodule commit reference.
- BalanceService: guard on earnAmount > 0 in updateEarnBalance to skip chain calls when not needed - EarnService: write BalanceRecord.earn from positions to bootstrap earn balance - EarnService: extract getEarnUpdateBalance for clean separation - Update ServicesFactory and TestKit for EarnService balanceStore dependency
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.