Conversation
- New types: EarnProviderType, YieldProvider, EarnType, EarnTransaction, EarnData - Add provider_type field to DelegationValidator - Add earn field to Balance - Add EarnDeposit/EarnWithdraw transaction types - Add Earn variant to TransactionInputType - Add earn_data to TransactionLoadMetadata::Evm - Update gemstone FFI remote types and conversions
Handle Earn input type in get_transaction_params and get_extra_fee_gas_limit (preload_mapper), preserve earn_data in map_transaction_load (preload), and add get_earn_data stub on GemGateway for future yielder integration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 foundational support for 'Earn' functionality across the platform, distinguishing it from existing 'Stake' operations. It involves adding new data structures and transaction types to represent earn deposits and withdrawals, updating core balance and transaction metadata, and integrating these changes into various blockchain providers. The modifications ensure that the system can properly categorize, process, and track yield-generating activities, setting the stage for future expansion of earn features. 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
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}
There was a problem hiding this comment.
Code Review
The pull request introduces the core infrastructure for 'Earn' functionality across multiple blockchain providers and primitives. The implementation is generally consistent with existing staking patterns. However, there are several critical issues to address, including the use of todo!() and unimplemented!() in production code paths which will cause panics, potential compilation errors due to missing or incorrect hex crate usage in EVM providers, and the use of unstable Rust features (let_chains) that may not be consistent with the project's toolchain. Additionally, some placeholder values in the new enums should be finalized.
| nonce, | ||
| chain_id, | ||
| stake_data: Some(StakeData { | ||
| data: if params.data.is_empty() { None } else { Some(hex::encode(¶ms.data)) }, |
There was a problem hiding this comment.
The hex crate is used here but it doesn't seem to be imported in this file, which will likely lead to a compilation error. Based on other files in this crate, you should use alloy_primitives::hex.
| data: if params.data.is_empty() { None } else { Some(hex::encode(¶ms.data)) }, | |
| data: if params.data.is_empty() { None } else { Some(alloy_primitives::hex::encode(¶ms.data)) }, |
| if let Some(approval) = &earn_data.approval { | ||
| Ok(TransactionParams::new(approval.token.clone(), encode_erc20_approve(&approval.spender)?, BigInt::from(0))) | ||
| } else { | ||
| Ok(TransactionParams::new(contract_address.clone(), hex::decode(call_data_hex)?, value)) |
There was a problem hiding this comment.
hex::decode is used here without a corresponding import. Since alloy_primitives::hex is used elsewhere in this file (e.g., lines 40 and 96), it's better to use the full path or add the import to avoid compilation errors.
| Ok(TransactionParams::new(contract_address.clone(), hex::decode(call_data_hex)?, value)) | |
| Ok(TransactionParams::new(contract_address.clone(), alloy_primitives::hex::decode(call_data_hex)?, value)) |
| Ok(Message { title, message: Some(message) }) | ||
| } | ||
| TransactionType::AssetActivation | TransactionType::PerpetualModifyPosition => todo!(), | ||
| TransactionType::AssetActivation | TransactionType::PerpetualModifyPosition | TransactionType::EarnDeposit | TransactionType::EarnWithdraw => todo!(), |
There was a problem hiding this comment.
Using todo!() in a daemon service is risky as it will cause a panic if an EarnDeposit or EarnWithdraw transaction is encountered. It's better to handle these types gracefully, for example by returning a generic message or a specific error, to ensure service stability.
crates/gem_aptos/src/rpc/client.rs
Outdated
| } | ||
| TransactionInputType::Swap(_, _, _) | TransactionInputType::Stake(_, _) | TransactionInputType::TokenApprove(_, _) | TransactionInputType::Generic(_, _, _) => Ok(1500), | ||
| TransactionInputType::Perpetual(_, _) => unimplemented!(), | ||
| TransactionInputType::Perpetual(_, _) | TransactionInputType::Earn(_, _) => unimplemented!(), |
There was a problem hiding this comment.
Using unimplemented!() in the RPC client will cause the application to panic at runtime when processing Earn transactions. Consider returning a Result::Err instead to allow for graceful error handling.
| if let TransactionLoadMetadata::Evm { earn_data: Some(ref ed), .. } = input.metadata | ||
| && ed.approval.is_some() | ||
| && let Some(ref gas_limit) = ed.gas_limit | ||
| { | ||
| return Ok(BigInt::from_str_radix(gas_limit, 10)?); | ||
| } |
There was a problem hiding this comment.
This block uses 'let chains' syntax (&& let), which is a relatively recent Rust feature. The existing code in this file (e.g., lines 191-195) uses nested if let statements. For consistency and to ensure compatibility with the project's current Rust toolchain, it's recommended to stick to nested if let blocks.
| if let TransactionLoadMetadata::Evm { earn_data: Some(ref ed), .. } = input.metadata | |
| && ed.approval.is_some() | |
| && let Some(ref gas_limit) = ed.gas_limit | |
| { | |
| return Ok(BigInt::from_str_radix(gas_limit, 10)?); | |
| } | |
| if let TransactionLoadMetadata::Evm { earn_data: Some(ref ed), .. } = input.metadata { | |
| if ed.approval.is_some() { | |
| if let Some(ref gas_limit) = ed.gas_limit { | |
| return Ok(BigInt::from_str_radix(gas_limit, 10)?); | |
| } | |
| } | |
| } |
| #[serde(rename_all = "lowercase")] | ||
| #[strum(serialize_all = "lowercase")] | ||
| pub enum YieldProvider { | ||
| Yo, |
There was a problem hiding this comment.
The variant Yo appears to be a placeholder. Please replace it with actual yield provider names or remove it if it's not yet implemented.
1 participant
No description provided.