feat(suite): solana v2 staking support and fee estimation #16800
Conversation
WalkthroughThe pull request updates dependency management and refactors several source files related to staking and transaction processing. In various package.json files, the dependency on Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx
Outdated
Show resolved
Hide resolved
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
Outdated
Show resolved
Hide resolved
| <> | ||
| ( | ||
| <Translation | ||
| id={ | ||
| isEthereumNetwork | ||
| ? 'TR_STAKE_APPROXIMATE_DAYS' | ||
| : 'TR_UP_TO_DAYS' | ||
| } | ||
| values={{ count: daysToUnstake }} | ||
| /> | ||
| ) | ||
| </> |
There was a problem hiding this comment.
react fragment no longer needed
There was a problem hiding this comment.
we need it for the brackets. The text should be wrapped in brackets Unstaking (up to 3 days)
dev-pvl
force-pushed
the
feat/solana-v2-staking-support
branch
from
42d09c4 to
4d36393
Compare
dev-pvl
force-pushed
the
feat/solana-v2-staking-support
branch
from
4d36393 to
22e4cd8
Compare
dev-pvl
requested review from
MiroslavProchazka,
marekrjpolak and
mroz22
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/suite/src/utils/suite/solanaStaking.ts (1)
39-61: Refactor 'createTransactionShimCommon' to avoid code duplicationThe function
createTransactionShimCommonincludes a TODO comment about refactoring to eliminate code duplication. Refactoring this function can improve maintainability and reduce redundancy.Do you want me to help refactor this function to avoid code duplication?
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts (1)
145-145: Simplify conditional check using optional chainingConsider simplifying the conditional check at line 145 by using optional chaining to improve readability.
Apply this diff to use optional chaining:
- if (estimatedFee && estimatedFee.payload && 'levels' in estimatedFee.payload) { + if (estimatedFee?.payload && 'levels' in estimatedFee.payload) {🧰 Tools
🪛 Biome (1.9.4)
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/blockchain-link/src/workers/utils.ts (1)
28-71: Enhance error handling in getSolanaStakingData.The function correctly handles the new epoch parameter and SDK integration. However, consider adding more specific error handling for edge cases.
export const getSolanaStakingData = async ( descriptor: string, isTestnet: boolean, epoch: number, ) => { const blockchainEnvironment = isTestnet ? 'devnet' : 'mainnet'; // Find the blockchain configuration for the specified chain and environment const blockchainConfig = config.find(c => c.blockchain.name.toLowerCase().includes(`solana ${blockchainEnvironment}`), ); + if (!blockchainConfig) { + throw new Error(`No configuration found for Solana ${blockchainEnvironment}`); + } const serverUrl = blockchainConfig?.blockchain.server[0]; + if (!serverUrl) { + throw new Error('No server URL configured'); + } const network = isTestnet ? Network.Devnet : Network.Mainnet; const solanaClient = new Solana(network, serverUrl);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
package.json(1 hunks)packages/blockchain-link-types/package.json(0 hunks)packages/blockchain-link-types/src/solana.ts(1 hunks)packages/blockchain-link/package.json(1 hunks)packages/blockchain-link/src/workers/solana/index.ts(1 hunks)packages/blockchain-link/src/workers/utils.ts(3 hunks)packages/suite/package.json(0 hunks)packages/suite/src/actions/wallet/stake/stakeFormActions.ts(3 hunks)packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts(1 hunks)packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts(8 hunks)packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx(1 hunks)packages/suite/src/support/messages.ts(1 hunks)packages/suite/src/utils/suite/solanaStaking.ts(5 hunks)packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx(2 hunks)suite-common/wallet-constants/src/solanaStakingConstants.ts(1 hunks)suite-common/wallet-utils/package.json(1 hunks)suite-common/wallet-utils/src/solanaStakingUtils.ts(5 hunks)
💤 Files with no reviewable changes (2)
- packages/blockchain-link-types/package.json
- packages/suite/package.json
✅ Files skipped from review due to trivial changes (1)
- packages/suite/src/actions/wallet/stake/stakeFormEthereumActions.ts
🧰 Additional context used
🪛 Biome (1.9.4)
suite-common/wallet-utils/src/solanaStakingUtils.ts
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (16)
packages/suite/src/components/suite/StakingProcess/StakingInfo.tsx (1)
39-42: LGTM! Translation key change improves clarity.The change from using a tilde character to a more descriptive translation key
TR_STAKE_APPROXIMATE_DAYSimproves clarity for users.packages/suite/src/views/wallet/staking/components/StakingDashboard/components/StakingCard.tsx (2)
150-150: LGTM! Network type check follows best practices.The addition of
isEthereumNetworkvariable improves code readability and follows the suggested implementation from past review comments.
228-238: LGTM! Network-specific translation keys improve clarity.The conditional selection of translation keys based on network type provides more accurate messaging for users. The parentheses formatting also improves readability.
packages/suite/src/support/messages.ts (1)
8844-8847: LGTM! New message entry follows i18n best practices.The new message
TR_STAKE_APPROXIMATE_DAYScorrectly implements plural formatting and uses a consistent pattern for approximate day values.packages/suite/src/utils/suite/solanaStaking.ts (1)
125-125:⚠️ Potential issueFix the variable name typo caused by a non-ASCII character
In line 125, the property name
сomputeUnitPriceuses a non-ASCII character for 'c' (Cyrillic\u0441) instead of the standard ASCII 'c'. This can lead to unexpected behavior and hard-to-find bugs.Apply this diff to correct the variable name:
- сomputeUnitPrice: BigInt(estimatedFee.feePerUnit), + computeUnitPrice: BigInt(estimatedFee.feePerUnit),Consider running the following script to check for other instances of non-ASCII characters in identifiers:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find occurrences of non-ASCII characters in the codebase. grep --color='auto' -nP '[^\x00-\x7F]' -r ./packages/suite/src/utils/suite/Length of output: 1154
Non-ASCII Character Detected in
solanaStaking.tsWe confirmed that the property name on line 125 in
packages/suite/src/utils/suite/solanaStaking.tsuses a non-ASCII Cyrillic character (с) instead of the standard ASCIIc. No other occurrences in relevant TypeScript files were found (the additional output is from binary fixture files).
- Action: Replace
сomputeUnitPricewithcomputeUnitPrice.suite-common/wallet-constants/src/solanaStakingConstants.ts (1)
9-10: Confirm that the hardcoded values for SOL compute units are up to datePlease verify that the constants
SOL_COMPUTE_UNIT_PRICEandSOL_COMPUTE_UNIT_LIMITreflect the current values on the Solana network. Hardcoded values may become outdated if network parameters change.packages/blockchain-link-types/src/solana.ts (1)
73-77: LGTM! Verify type compatibility with Solana staking accounts.The new
SolanaStakingAccounttype definition correctly captures the essential fields for Solana staking accounts.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that all usages of SolanaStakingAccount in the codebase align with the new type definition # Search for all usages of SolanaStakingAccount rg -A 5 "SolanaStakingAccount" --type tsLength of output: 5022
LGTM – The
SolanaStakingAccounttype is consistently used across the codebase and aligns with the required Solana staking account structure.
- The type is defined with required fields (
statusandrentExemptReserve) and an optionalstakefield.- Searches show that all references in
packages/blockchain-link-types/src/solana.ts,packages/blockchain-link-types/src/common.ts,suite-common/wallet-utils/src/solanaStakingUtils.ts, andsuite-common/wallet-types/src/account.tscorrectly import and use the new type.packages/blockchain-link/src/workers/utils.ts (1)
1-1: LGTM! Migration to Solana-specific SDK.The import change correctly switches to the Solana-specific SDK package.
suite-common/wallet-utils/src/solanaStakingUtils.ts (1)
38-45: LGTM! Clear network configuration.The network configuration is correctly updated to use the new SDK's Network enum.
packages/suite/src/actions/wallet/stake/stakeFormActions.ts (1)
109-166: LGTM! Enhanced fee estimation support.The function correctly integrates fee estimation while maintaining backward compatibility through optional parameters.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that all callers of composeStakingTransaction handle the new optional parameters # Search for all calls to composeStakingTransaction rg "composeStakingTransaction\(" --type tsLength of output: 262
Fee Estimation Integration Verified
The
composeStakingTransactionfunction now correctly integrates fee estimation with optional parameters, and our search confirms that its callers in both Solana and Ethereum actions remain intact and backward compatible.
- Verified calls in
stakeFormSolanaActions.tsandstakeFormEthereumActions.ts.- Optional parameters (
estimatedFee,customFeeLimit) are handled appropriately.packages/blockchain-link/src/workers/solana/index.ts (3)
300-315: LGTM! Well-implemented epoch caching mechanism.The
getEpochfunction implementation is robust with:
- Proper cache checking
- Deferred promise to handle parallel requests
- Reasonable cache duration (1 hour)
369-374: LGTM! Epoch information is now properly integrated with staking data.The changes correctly fetch and pass the epoch information to
getSolanaStakingData.
375-380: LGTM! Misc object construction is properly updated.The
miscobject now includes bothsolStakingAccountsandsolEpochfields.suite-common/wallet-utils/package.json (1)
15-15: Verify compatibility with the new Solana SDK version.The addition of
@everstake/wallet-sdk-solanav2.0.5 aligns with the PR objectives for Solana v2 staking support.packages/blockchain-link/package.json (1)
75-75: LGTM! Consistent dependency version across packages.The addition of
@everstake/wallet-sdk-solanav2.0.5 matches the version used insuite-common/wallet-utils/package.json.package.json (1)
103-103: LGTM! Resolution cleanup aligns with SDK transition.The removal of
@everstake/wallet-sdk/@solana/web3.jsresolution is appropriate as the project now uses@everstake/wallet-sdk-solana.
| import { address } from '@solana/web3.js'; | ||
|
|
There was a problem hiding this comment.
Verify the validity of the 'address' import from '@solana/web3.js'
The imported address may not be a valid export from @solana/web3.js. Please verify that address is correctly imported or replace it with the appropriate module. It might need to be imported from @solana/address or replaced with new PublicKey() if you're trying to handle public keys.
Apply this diff to replace the import:
- import { address } from '@solana/web3.js';
+ import { PublicKey } from '@solana/web3.js';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { address } from '@solana/web3.js'; | |
| import { PublicKey } from '@solana/web3.js'; |
There was a problem hiding this comment.
there is no PublicKey in @solana/web3.js v2 anymore
There was a problem hiding this comment.
🧩 Analysis chain
@dev-pvl I apologize for my incorrect suggestion. You are absolutely right - the address import from @solana/web3.js is the correct approach in v2, and my suggestion to use PublicKey was outdated. The current implementation should be kept as is.
(_/）
(•ㅅ•）
＿ノ ヽ ノ＼＿
／ / ⌒Ｙ⌒ Ｙ ヽ
( (三ヽ人 / |
| ﾉ⌒＼ ￣￣ヽ ノ
ヽ＿＿＿＞､＿＿／
｜( 王 ﾉ〈
/ﾐ`ー―彡\
/ ヽ_／ \
🌐 Web query:
What are the changes in @solana/web3.js v2? What replaced PublicKey?
💡 Result:
The release of `@solana/web3.js` version 2.x introduces significant architectural changes aimed at improving performance, modularity, and security while adopting modern JavaScript practices[1][4][7][10]. Here's a breakdown of key changes:
## Major Changes in Web3.js v2
- **Modular Architecture**:
The monolithic library has been split into focused modules like:
- `@solana/accounts` (account management)
- `@solana/codecs` (data serialization)
- `@solana/rpc` (RPC interactions)
- `@solana/signers` (signature handling)[4][8]
- **Functional Programming Approach**:
Replaced class-based implementations with single-purpose functions for better tree-shaking and bundle size reduction (up to 30% smaller)[4][7].
- **Performance Improvements**:
- Native Ed25519 support via `CryptoKey` for 10x faster key operations[7]
- Zero external dependencies to prevent supply-chain risks[4]
- **Modern JS Features**:
- Native BigInt support for precise u64 handling
- Async iterators for subscriptions[7]
## PublicKey Changes
While `PublicKey` remains a core concept for account identification[2], its implementation has shifted:
1. **Functional Interface**:
Instead of class methods like `PublicKey.fromBase58()`, v2 uses standalone functions:
```js
import { createPublicKey } from '@solana/keys';
const pubkey = createPublicKey(base58String);
```
2. **Enhanced Compatibility**:
Supports multiple input types natively:
```js
createPublicKey(base58Str); // From string
createPublicKey(buffer); // From Buffer
createPublicKey(publicKey); // From existing instance[2][4]
```
3. **Security Improvements**:
Key operations now use Web Crypto API instead of custom implementations[7].
## Migration Considerations
- Backwards compatibility layer available via `@solana/web3.js-legacy`[10]
- Solana Cookbook provides side-by-side v1/v2 examples[1]
- Transaction signing uses `@solana/signers` instead of Keypair class[4][7]
Developers should reference updated documentation and leverage new modular imports to optimize bundle sizes while transitioning from legacy patterns[4][8][10].
Citations:
- 1: https://solanacompass.com/learn/Changelog/solana-changelog-nov-12-web3js-v2-skip-preflights-in-the-cli-and-simd-0191
- 2: https://solana.com/docs/clients/javascript-reference
- 3: https://www.youtube.com/watch?v=EYVumQWKU-s
- 4: https://www.panewslab.com/en/articledetails/416zlt93y3b7.html
- 5: https://www.quicknode.com/guides/solana-development/transactions/how-to-send-a-transaction-on-solana-using-javascript
- 6: https://solana.com/docs/advanced/versions
- 7: https://blog.quicknode.com/solana-web3-js-2-0-a-new-chapter-in-solana-development/
- 8: https://www.npmjs.com/package/@solana/web3.js/v/2.0.0-preview.3.20240503183512.f23c878d11c98fe8564ddf477fa7ade433101a9d
- 9: https://github.com/solana-labs/solana-web3.js/issues/1111
- 10: https://www.anza.xyz/blog/solana-web3-js-2-release
🏁 Script executed:
#!/bin/bash
# Check package.json for @solana/* dependencies
find . -name "package.json" -exec grep -l "@solana/" {} \;
# Check the actual imports from @solana packages
rg "from '@solana/" --type ts
Length of output: 925
🏁 Script executed:
#!/bin/bash
# Check the version of @solana/web3.js in package.json files
find . -name "package.json" -exec grep -A 5 '"@solana/web3.js"' {} \;
Length of output: 1130
@dev-pvl I apologize for my confusion in the previous responses. You are absolutely right about v2, and I see now that the codebase is using @solana/web3.js@^2.0.0. Looking at the v2 documentation, the functionality has been modularized and we should be using the appropriate module. The address import should be replaced with the corresponding import from @solana/addresses package.
Would you like me to help identify the correct import from the new modular packages? We can check the official migration guide or the v2 documentation for the exact package and function to use.
(_/）
(°.°）
c(,_,)
✏️ Learnings added
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.325Z
Learning: In @solana/web3.js v2, 'address' is the correct import to use for handling Solana addresses, replacing the previously used PublicKey class from v1.
Learnt from: dev-pvl
PR: trezor/trezor-suite#16800
File: packages/suite/src/actions/wallet/stake/stakeFormSolanaActions.ts:1-2
Timestamp: 2025-02-06T11:19:02.325Z
Learning: @solana/web3.js v2 has moved to a modular architecture where functionality is split into separate packages like @solana/addresses, @solana/codecs, @solana/rpc, etc., replacing the monolithic imports from v1.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
|
||
| const serializedVersiondeTx = txData.tx.versionedTx.serialize(); | ||
| const signedSerializedTx = Buffer.from(serializedVersiondeTx).toString('hex'); | ||
| txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature); |
There was a problem hiding this comment.
Verify the use of the 'address' function on 'account.descriptor'
At line 264, address(account.descriptor) is used to add the signature. Since the address import may be invalid or incorrect, ensure that this function correctly processes account.descriptor. If you're aiming to convert the account descriptor into a public key string, consider using new PublicKey(account.descriptor).toBase58() instead.
Apply this diff to correct the function usage:
- txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature);
+ txData.tx.txShim.addSignature(new PublicKey(account.descriptor).toBase58(), signedTx.payload.signature);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| txData.tx.txShim.addSignature(address(account.descriptor), signedTx.payload.signature); | |
| txData.tx.txShim.addSignature(new PublicKey(account.descriptor).toBase58(), signedTx.payload.signature); |
| const balances: Record<StakeStateType, string> = balanceResults.reduce( | ||
| (acc, [key, balance]) => ({ ...acc, [key]: balance }), | ||
| (acc, [status, balance]) => ({ ...acc, [status]: balance }), | ||
| {}, | ||
| ); |
There was a problem hiding this comment.
Optimize the balances reduction to avoid O(n²) complexity.
The spread operator in the reducer accumulator causes quadratic time complexity.
const balances: Record<StakeStateType, string> = balanceResults.reduce(
- (acc, [status, balance]) => ({ ...acc, [status]: balance }),
+ (acc, [status, balance]) => {
+ acc[status] = balance;
+ return acc;
+ },
{},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const balances: Record<StakeStateType, string> = balanceResults.reduce( | |
| (acc, [key, balance]) => ({ ...acc, [key]: balance }), | |
| (acc, [status, balance]) => ({ ...acc, [status]: balance }), | |
| {}, | |
| ); | |
| const balances: Record<StakeStateType, string> = balanceResults.reduce( | |
| - (acc, [status, balance]) => ({ ...acc, [status]: balance }), | |
| + (acc, [status, balance]) => { | |
| + acc[status] = balance; | |
| + return acc; | |
| + }, | |
| {}, | |
| ); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat are unpopular packages?This package is not very popular. Unpopular packages may have less maintenance and contain other problems. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
Description
Upgraded wallet-sdk to the latest version
Added support for Solana v2
Implemented dynamic fee estimation for better accuracy
Refactored Solana staking utils to remove unnecessary functions.
Related Issue
Resolve
Screenshots: