Conversation
Introduce structured Tron stake data and unfreeze support across the stack. Add TronVote, TronUnfreeze and TronStakeData types (and export Resource) in primitives and propagate them into TransactionLoadMetadata. Update gem_tron preload logic to compute stake data (votes or unfreeze amounts) via a new get_stake_data path and adapt transaction load to carry stake_data instead of a raw votes map. Implement calculate_unfreeze_amounts in preload_mapper to derive unfreeze items from account frozen state (with unit tests), and update gemstone uniffi models and conversion mappings to use the new Tron stake/unfreeze types. These changes enable representing both vote adjustments and automatic unfreeze operations in transaction preload metadata.
|
Review changes with Changed Files
|
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 addresses and resolves a bug related to unstaking all votes on the Tron network. It introduces a more robust and explicit data model for handling staking and unstaking operations by replacing a generic HashMap with a specialized enum. This change allows the system to accurately distinguish between vote modifications and unfreeze actions, particularly when a user intends to remove all their staked assets, thereby improving the reliability of Tron-related transactions. 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}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a bug in the Tron unstaking process by introducing a TronStakeData enum. This change correctly handles the scenario where unstaking all votes should trigger an unfreeze operation, distinguishing it from partial unstakes or other staking actions. The new data structures are consistently integrated across the codebase, including the FFI layer. My review includes a suggestion to enhance the clarity and efficiency of the core logic in get_stake_data.
| let account = self.get_account(&input.sender_address).await?; | ||
| let mut current_votes: HashMap<String, u64> = account.votes.unwrap_or_default().into_iter().map(|v| (v.vote_address, v.vote_count)).collect(); | ||
|
|
||
| let vote_amount = input.value.parse::<u64>().unwrap_or(0) / 10_u64.pow(asset.decimals as u32); | ||
| let amount = input.value.parse::<u64>().unwrap_or(0) / 10_u64.pow(asset.decimals as u32); | ||
| let mut votes: HashMap<String, u64> = account | ||
| .votes | ||
| .as_ref() | ||
| .map(|v| v.iter().map(|v| (v.vote_address.clone(), v.vote_count)).collect()) | ||
| .unwrap_or_default(); | ||
|
|
||
| match stake_type { | ||
| StakeType::Stake(validator) => { | ||
| *current_votes.entry(validator.id.clone()).or_insert(0) += vote_amount; | ||
| } | ||
| StakeType::Unstake(delegation) => { | ||
| if let Some(votes) = current_votes.get_mut(&delegation.base.validator_id) { | ||
| *votes = votes.saturating_sub(vote_amount); | ||
| StakeType::Stake(v) => *votes.entry(v.id.clone()).or_default() += amount, | ||
| StakeType::Unstake(d) => { | ||
| votes.entry(d.base.validator_id.clone()).and_modify(|v| *v = v.saturating_sub(amount)); | ||
| votes.retain(|_, v| *v > 0); | ||
| if votes.is_empty() { | ||
| return Ok(TronStakeData::Unfreeze(calculate_unfreeze_amounts(account.frozen_v2.as_ref(), input.value.parse().unwrap_or(0)))); | ||
| } | ||
| } | ||
| StakeType::Redelegate(redelegate_data) => { | ||
| if let Some(votes) = current_votes.get_mut(&redelegate_data.delegation.base.validator_id) { | ||
| *votes = votes.saturating_sub(vote_amount); | ||
| } | ||
| *current_votes.entry(redelegate_data.to_validator.id.clone()).or_insert(0) += vote_amount; | ||
| StakeType::Redelegate(r) => { | ||
| votes.entry(r.delegation.base.validator_id.clone()).and_modify(|v| *v = v.saturating_sub(amount)); | ||
| *votes.entry(r.to_validator.id.clone()).or_default() += amount; | ||
| } | ||
| StakeType::Rewards(_) | StakeType::Withdraw(_) | StakeType::Freeze(_) => {} | ||
| } |
There was a problem hiding this comment.
To improve code clarity and avoid redundant operations, you can parse input.value once at the beginning of the Stake match arm.
Additionally, in the Unstake arm, you can use votes.values().all(|&count| count == 0) to check if all votes are cleared. This avoids modifying the votes map with retain just for the check, as the final filter operation will handle the removal of zero-count votes.
let account = self.get_account(&input.sender_address).await?;
let value_sun = input.value.parse::<u64>().unwrap_or(0);
let amount = value_sun / 10_u64.pow(asset.decimals as u32);
let mut votes: HashMap<String, u64> = account
.votes
.as_ref()
.map(|v| v.iter().map(|v| (v.vote_address.clone(), v.vote_count)).collect())
.unwrap_or_default();
match stake_type {
StakeType::Stake(v) => *votes.entry(v.id.clone()).or_default() += amount,
StakeType::Unstake(d) => {
votes.entry(d.base.validator_id.clone()).and_modify(|v| *v = v.saturating_sub(amount));
if votes.values().all(|&count| count == 0) {
return Ok(TronStakeData::Unfreeze(calculate_unfreeze_amounts(account.frozen_v2.as_ref(), value_sun)));
}
}
StakeType::Redelegate(r) => {
votes.entry(r.delegation.base.validator_id.clone()).and_modify(|v| *v = v.saturating_sub(amount));
*votes.entry(r.to_validator.id.clone()).or_default() += amount;
}
StakeType::Rewards(_) | StakeType::Withdraw(_) | StakeType::Freeze(_) => {}
}
| let mut current_votes: HashMap<String, u64> = account.votes.unwrap_or_default().into_iter().map(|v| (v.vote_address, v.vote_count)).collect(); | ||
|
|
||
| let vote_amount = input.value.parse::<u64>().unwrap_or(0) / 10_u64.pow(asset.decimals as u32); | ||
| let amount = input.value.parse::<u64>().unwrap_or(0) / 10_u64.pow(asset.decimals as u32); |
There was a problem hiding this comment.
avoid unwrap_or we have BigNumberFormatter for this. @copilot @gemdev111
|
@gemcoder21 I've opened a new pull request, #923, to work on those changes. Once the pull request is ready, I'll request review from you. |
| frozen | ||
| .map(|frozen| { | ||
| let mut items: Vec<_> = frozen.iter().filter(|f| f.amount > 0).collect(); | ||
| items.sort_by_key(|f| std::cmp::Reverse(f.amount)); |
There was a problem hiding this comment.
why you need to sort? simplify this code
…ng (#923) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gemcoder21 <104884878+gemcoder21@users.noreply.github.com>
Change numeric fields to UInt64 and add typeshare/serde metadata for Swift interop: TronVote.count and TronUnfreeze.amount use UInt64; add PartialEq derives and typeshare swift conformances; tag TronStakeData with serde(tag="type", content="content"). Simplify calculate_unfreeze_amounts in preload_mapper.rs by using an iterator chain with filter and scan to compute unfreeze entries, removing the temporary allocation and explicit sorting (now processes in original order).
Changed votes: HashMap<String, u64> to TronStakeData enum that handles both: