Add Solana support to ZetaChain client and browser deposits #199
Conversation
lukema95
requested review from
andresaiello,
fadeev and
a team
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several enhancements to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (6)
package.json (1)
95-95: Consider pinning the dependency version.The caret (^) in the version allows minor updates which could introduce breaking changes. Consider pinning the version for better dependency management:
- "@solana/wallet-adapter-react": "^0.15.35", + "@solana/wallet-adapter-react": "0.15.35",packages/tasks/src/solanaDeposit.ts (3)
14-16: Ensure 'idPath' is correctly provided and handledThe
idPathparameter is used to retrieve the keypair from the file system. Ensure thatargs.idPathis correctly provided or defaults to a valid path. Consider adding validation to handle cases where the keypair file may not exist oridPathis invalid.
45-45: Optimize module imports for better code clarityInstead of dynamically importing the
pathmodule within the function, consider importing it at the top of the file. This improves code readability and avoids unnecessary dynamic imports.Apply this diff to import
pathat the top:+import path from "path"; ... -export const getKeypairFromFile = async (filepath: string) => { - const path = await import("path"); +export const getKeypairFromFile = async (filepath: string) => {
64-70: Simplify error handling for JSON parsingThe current error handling checks for a specific error message when parsing JSON. Instead, consider catching all parsing errors and providing a clear error message. This simplifies the code and ensures all JSON parsing errors are appropriately handled.
Apply this diff:
try { parsedFileContents = Uint8Array.from(JSON.parse(fileContents)); } catch (thrownObject) { - const error: any = thrownObject; - if (!error.message.includes("Unexpected token")) { - throw error; - } - throw new Error(`Invalid secret key file at '${filepath}'!`); + throw new Error(`Invalid secret key file at '${filepath}': ${thrownObject.message}`); }packages/client/src/client.ts (2)
41-70: Consider simplifying theZetaChainClientParamstype definitionThe current union of interfaces with
neverandundefinedproperties can be complex and hard to maintain. Using a discriminated union with a commonwalletTypeproperty can make the code more readable and easier to extend.Here's a suggested refactor:
export type ZetaChainClientParams = ZetaChainClientParamsBase & - ( - | { - signer: Signer; - solanaAdapter?: never; - solanaWallet?: never; - wallet?: never; - } - | { - signer?: never; - solanaAdapter: WalletContextState; - solanaWallet?: never; - wallet?: never; - } - | { - signer?: never; - solanaAdapter?: never; - solanaWallet: SolanaWallet; - wallet?: never; - } - | { - signer?: never; - solanaAdapter?: never; - solanaWallet?: never; - wallet: Wallet; - } - | { - signer?: undefined; - solanaAdapter?: undefined; - solanaWallet?: undefined; - wallet?: undefined; - } - ); + ( + | { walletType: 'signer'; signer: Signer } + | { walletType: 'solanaAdapter'; solanaAdapter: WalletContextState } + | { walletType: 'solanaWallet'; solanaWallet: SolanaWallet } + | { walletType: 'wallet'; wallet: Wallet } + | { walletType?: undefined } + );This approach introduces a
walletTypediscriminator, making it clearer which wallet is being used and simplifying type checks.
160-164: Handle null values explicitly ingetSolanaPublicKeyWhen both
solanaAdapterandsolanaWalletareundefined, the method currently returnsnull, which is appropriate. However, ifpublicKeyisnullorundefinedwithinsolanaAdapterorsolanaWallet, it might be clearer to handle these cases explicitly.Consider updating the method to make the intent explicit:
public getSolanaPublicKey(): PublicKey | null { return ( - this.solanaAdapter?.publicKey || this.solanaWallet?.publicKey || null + (this.solanaAdapter && this.solanaAdapter.publicKey) || + (this.solanaWallet && this.solanaWallet.publicKey) || + null ); }This makes it clear that the method returns a
PublicKeyif available, ornullotherwise.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (27)
typechain-types/@openzeppelin/contracts/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC1155Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC20Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC721Errors.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/interfaces/index.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/ERC20.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/IERC20.tsis excluded by!typechain-types/**typechain-types/@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.tsis excluded by!typechain-types/**typechain-types/contracts/EthZetaMock.sol/ZetaEthMock.tsis excluded by!typechain-types/**typechain-types/contracts/TestZRC20.tsis excluded by!typechain-types/**typechain-types/contracts/shared/MockZRC20.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC1155Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC20Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/IERC721Errors__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/draft-IERC6093.sol/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/interfaces/index.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/ERC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/IERC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/EthZetaMock.sol/ZetaEthMock__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/TestZRC20__factory.tsis excluded by!typechain-types/**typechain-types/factories/contracts/shared/MockZRC20__factory.tsis excluded by!typechain-types/**typechain-types/hardhat.d.tsis excluded by!typechain-types/**typechain-types/index.tsis excluded by!typechain-types/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.json(1 hunks)packages/client/src/client.ts(4 hunks)packages/client/src/solanaDeposit.ts(3 hunks)packages/tasks/src/solanaDeposit.ts(4 hunks)
🔇 Additional comments (4)
package.json (1)
95-95: Verify Solana wallet adapter version compatibility and security.
Let's ensure the chosen version is secure and compatible with other Solana dependencies.
✅ Verification successful
Let me gather more information about the existing Solana dependencies in the project to verify compatibility.
Version 0.15.35 is compatible and secure
The chosen version of @solana/wallet-adapter-react (0.15.35) is:
- The latest available version from npm
- Compatible with the project's
@solana/web3.js(^1.95.3), meeting the peer dependency requirement (^1.77.3) - Already in use in the codebase (packages/client/src/client.ts)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version and security advisories for @solana/wallet-adapter-react
# Get latest version from npm
echo "Latest version from npm:"
npm view @solana/wallet-adapter-react version
# Check for known vulnerabilities
echo -e "\nChecking for vulnerabilities:"
npm audit --json @solana/[email protected] | jq '.advisories'
# Check compatibility with existing Solana dependencies
echo -e "\nChecking peer dependencies:"
npm info @solana/[email protected] peerDependencies
Length of output: 797
Script:
#!/bin/bash
# Check existing Solana dependencies and their versions
echo "Checking Solana dependencies in package.json:"
rg '"@solana/.*":|"@coral-xyz/.*":' package.json
# Check if there are any direct imports of the wallet adapter
echo -e "\nChecking usage of wallet adapter:"
rg -l "@solana/wallet-adapter-react"
Length of output: 442
packages/tasks/src/solanaDeposit.ts (3)
1-2: Imports are appropriate for Solana wallet integration
The imports of Wallet from @coral-xyz/anchor and Keypair from @solana/web3.js are necessary for the Solana wallet functionality.
17-19: Initialization of ZetaChainClient with 'solanaWallet' is appropriate
The client is correctly instantiated with the solanaWallet, aligning with the updated client requirements. This change enables Solana wallet integration within the ZetaChainClient.
33-33: Updated 'client.solanaDeposit' method invocation aligns with new signature
The call to client.solanaDeposit without the idPath parameter reflects the updated method signature in the client interface. This ensures compatibility with the modifications made to the ZetaChainClient.
| const home = process.env.HOME || null; | ||
| if (home) { | ||
| filepath = path.join(home, filepath.slice(1)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use 'os.homedir()' for cross-platform home directory resolution
Using process.env.HOME may not be reliable across all operating systems (e.g., it may not be defined on Windows). Instead, use os.homedir() from Node.js's os module to get the home directory in a cross-platform manner.
Apply this diff:
+import os from "os";
...
- const home = process.env.HOME || null;
+ const home = os.homedir();Committable suggestion skipped: line range outside the PR's diff.
| } catch (error) { | ||
| throw new Error(`Could not read keypair from file at '${filepath}'`); | ||
| } | ||
| // Parse contents of file | ||
| let parsedFileContents; | ||
| try { | ||
| parsedFileContents = Uint8Array.from(JSON.parse(fileContents)); | ||
| } catch (thrownObject) { | ||
| const error: any = thrownObject; | ||
| if (!error.message.includes("Unexpected token")) { | ||
| throw error; | ||
| } | ||
| throw new Error(`Invalid secret key file at '${filepath}'!`); | ||
| console.error("Transaction failed:", error); | ||
| } |
There was a problem hiding this comment.
Properly handle errors to prevent unexpected behavior.
Currently, the function logs the error but does not rethrow it or return a value, which could lead to inconsistent behavior for callers expecting a transaction signature or an error.
Modify the catch block to rethrow the error:
console.error("Transaction failed:", error);
+ throw error;Alternatively, ensure the function returns a consistent result or handles the error appropriately.
📝 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.
| } catch (error) { | |
| throw new Error(`Could not read keypair from file at '${filepath}'`); | |
| } | |
| // Parse contents of file | |
| let parsedFileContents; | |
| try { | |
| parsedFileContents = Uint8Array.from(JSON.parse(fileContents)); | |
| } catch (thrownObject) { | |
| const error: any = thrownObject; | |
| if (!error.message.includes("Unexpected token")) { | |
| throw error; | |
| } | |
| throw new Error(`Invalid secret key file at '${filepath}'!`); | |
| console.error("Transaction failed:", error); | |
| } | |
| } catch (error) { | |
| console.error("Transaction failed:", error); | |
| throw error; | |
| } |
| if (this.solanaAdapter) { | ||
| const { blockhash, lastValidBlockHeight } = | ||
| await connection.getLatestBlockhash(); | ||
| const messageLegacy = new TransactionMessage({ | ||
| instructions: tx.instructions, | ||
| payerKey: this.solanaAdapter.publicKey!, | ||
| recentBlockhash: blockhash, | ||
| }).compileToV0Message(); | ||
|
|
||
| console.log("Transaction signature:", txSignature); | ||
| } catch (error) { | ||
| console.error("Transaction failed:", error); | ||
| } | ||
| }; | ||
| const versionedTransaction = new VersionedTransaction(messageLegacy); | ||
|
|
||
| const getKeypairFromFile = async (filepath: string) => { | ||
| const path = await import("path"); | ||
| if (filepath[0] === "~") { | ||
| const home = process.env.HOME || null; | ||
| if (home) { | ||
| filepath = path.join(home, filepath.slice(1)); | ||
| txSignature = await this.solanaAdapter.sendTransaction( | ||
| versionedTransaction, | ||
| connection | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
Utilize lastValidBlockHeight when sending the transaction.
You retrieve lastValidBlockHeight but do not use it. Including it in the send options ensures that the transaction is valid within the expected block height range and can prevent transactions from failing due to blockhash expiration.
Apply this change to include lastValidBlockHeight:
txSignature = await this.solanaAdapter.sendTransaction(
versionedTransaction,
connection,
+ {
+ minContextSlot: lastValidBlockHeight,
+ }
);This ensures that the transaction remains valid and provides additional safeguards against blockhash-related issues.
Committable suggestion skipped: line range outside the PR's diff.
| provider = new anchor.AnchorProvider( | ||
| connection, | ||
| walletAdapter as any, | ||
| anchor.AnchorProvider.defaultOptions() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using the any type when casting walletAdapter.
Casting walletAdapter to any defeats TypeScript's type safety. It's better to define a proper interface for walletAdapter to ensure type safety and catch potential errors at compile time.
You can define an interface and use it instead of any:
+ interface WalletAdapter {
+ publicKey: anchor.web3.PublicKey;
+ signAllTransactions: (txs: Transaction[]) => Promise<Transaction[]>;
+ signTransaction: (tx: Transaction) => Promise<Transaction>;
+ }
provider = new anchor.AnchorProvider(
connection,
- walletAdapter as any,
+ walletAdapter as WalletAdapter,
anchor.AnchorProvider.defaultOptions()
);Committable suggestion skipped: line range outside the PR's diff.
| public isSolanaWalletConnected(): boolean { | ||
| return this.solanaAdapter?.connected || this.solanaWallet !== undefined; | ||
| } |
There was a problem hiding this comment.
Ensure consistent return type in isSolanaWalletConnected
The method currently returns undefined or boolean due to the use of optional chaining. This can lead to unexpected behavior when the result is used in conditional statements.
Modify the method to ensure it always returns a boolean:
public isSolanaWalletConnected(): boolean {
- return this.solanaAdapter?.connected || this.solanaWallet !== undefined;
+ return (
+ (this.solanaAdapter ? this.solanaAdapter.connected : false) ||
+ (this.solanaWallet !== undefined)
+ );
}This change guarantees that the method returns true or false, avoiding potential issues with undefined.
📝 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.
| public isSolanaWalletConnected(): boolean { | |
| return this.solanaAdapter?.connected || this.solanaWallet !== undefined; | |
| } | |
| public isSolanaWalletConnected(): boolean { | |
| return ( | |
| (this.solanaAdapter ? this.solanaAdapter.connected : false) || | |
| (this.solanaWallet !== undefined) | |
| ); | |
| } |
| } else if (params.solanaWallet) { | ||
| this.solanaWallet = params.solanaWallet; | ||
| } else if (params.solanaAdapter) { | ||
| this.solanaAdapter = params.solanaAdapter; |
There was a problem hiding this comment.
Add checks for multiple wallet parameters in the constructor
Currently, the constructor checks for both wallet and signer being provided but doesn't handle cases where multiple Solana wallet parameters are supplied together. To prevent conflicting wallet parameters, ensure that only one wallet type is provided at a time.
You can update the constructor to include these checks:
if (params.wallet && params.signer) {
throw new Error("You can only provide a wallet or a signer, not both.");
+} else if (
+ [params.wallet, params.signer, params.solanaWallet, params.solanaAdapter].filter(Boolean).length > 1
+) {
+ throw new Error("You can only provide one wallet parameter.");
} else if (params.wallet) {
this.wallet = params.wallet;
} else if (params.signer) {
this.signer = params.signer;
} else if (params.solanaWallet) {
this.solanaWallet = params.solanaWallet;
} else if (params.solanaAdapter) {
this.solanaAdapter = params.solanaAdapter;
}This ensures that if more than one wallet parameter is provided, an error is thrown.
📝 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.
| } else if (params.solanaWallet) { | |
| this.solanaWallet = params.solanaWallet; | |
| } else if (params.solanaAdapter) { | |
| this.solanaAdapter = params.solanaAdapter; | |
| if (params.wallet && params.signer) { | |
| throw new Error("You can only provide a wallet or a signer, not both."); | |
| } else if ( | |
| [params.wallet, params.signer, params.solanaWallet, params.solanaAdapter].filter(Boolean).length > 1 | |
| ) { | |
| throw new Error("You can only provide one wallet parameter."); | |
| } else if (params.wallet) { | |
| this.wallet = params.wallet; | |
| } else if (params.signer) { | |
| this.signer = params.signer; | |
| } else if (params.solanaWallet) { | |
| this.solanaWallet = params.solanaWallet; | |
| } else if (params.solanaAdapter) { | |
| this.solanaAdapter = params.solanaAdapter; |
|
For Cli: For Browser: |
|
@fadeev pls review |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/client/src/solanaDeposit.ts (1)
Line range hint
12-19: Consider splitting browser and programmatic wallet handling.The function currently handles both browser wallet adapter and programmatic wallet scenarios, which increases complexity. Consider:
- Splitting into separate functions for better separation of concerns
- Adding input validation for amount and recipient
- Using TypeScript types for params instead of
any[]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.json(1 hunks)packages/client/src/solanaDeposit.ts(3 hunks)packages/tasks/src/solanaDeposit.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (10)
packages/tasks/src/solanaDeposit.ts (4)
40-41: LGTM! Well-documented optional parameters with sensible defaults.
The task parameters are properly documented and include reasonable default values:
solanaNetworkdefaults to "devnet"idPathdefaults to the standard Solana CLI config location
48-50: Use 'os.homedir()' for cross-platform home directory resolution
Using process.env.HOME may not be reliable across all operating systems (e.g., it may not be defined on Windows).
54-72: LGTM! Excellent error handling practices.
The error handling is well-implemented:
- Specific error messages for file reading failures
- Proper JSON parsing error detection
- Clear error messages that help with troubleshooting
14-20: LGTM! API parameter has been removed as requested.
The changes look good:
- Proper wallet initialization using keypair
- Simplified parameter structure
- Removed API parameter as suggested in the previous review
Let's verify that all API parameters have been removed from the codebase:
Also applies to: 32-34
✅ Verification successful
API parameter has been successfully removed from solanaDeposit task
The verification confirms that:
- The solanaDeposit.ts task file no longer uses any API parameters
- The remaining API references found in client files (packages/client/src/solanaDeposit.ts and packages/client/src/getBalances.ts) are part of the internal client implementation and not related to the task's parameter structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining API parameters in Solana-related code
rg -i 'solana.*api|api.*solana'
Length of output: 225
Script:
#!/bin/bash
# Let's check the context of these API usages to understand if they're related to our changes
rg -B 5 -A 5 'const api = getEndpoints\("solana"' packages/client/src/solanaDeposit.ts
# Also check the other file
rg -B 5 -A 5 'API = this.getEndpoint\("solana"' packages/client/src/getBalances.ts
# Let's also verify if there are any references to these endpoints in our task file
rg -l "getEndpoints|getEndpoint" packages/tasks/src/solanaDeposit.ts
Length of output: 877
Script:
#!/bin/bash
# Let's check the actual implementation in our task file to confirm it's not using any API endpoints
cat packages/tasks/src/solanaDeposit.ts
# Also check if there are any other files in the tasks directory that might be using these endpoints
rg -l "getEndpoints|getEndpoint" packages/tasks/
Length of output: 2561
packages/client/src/solanaDeposit.ts (6)
20-22: LGTM! Good defensive programming practice.
The wallet connection check prevents undefined behavior by failing fast when prerequisites aren't met.
31-47: LGTM! Well-structured wallet adapter with proper error handling.
The implementation properly handles different wallet capabilities with clear error messages.
49-52: Avoid using any type when casting walletAdapter.
This is a type safety issue that was previously identified.
105-120: Utilize lastValidBlockHeight in transaction options.
The lastValidBlockHeight is retrieved but not used in the transaction options.
95-97: LGTM! Proper null checking on public key access.
The conditional access to public key with null checking is well implemented.
131-134: Properly handle errors to prevent unexpected behavior.
The function should not silently fail and potentially return undefined.
| const network = "solana_" + this.network; | ||
| const api = getEndpoints("solana" as any, network); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unsafe type assertion in getEndpoints call.
The as any type assertion bypasses TypeScript's type checking and could hide potential errors.
-const api = getEndpoints("solana" as any, network);
+const api = getEndpoints("solana", network);Consider updating the getEndpoints function's type definition if needed.
📝 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 network = "solana_" + this.network; | |
| const api = getEndpoints("solana" as any, network); | |
| const network = "solana_" + this.network; | |
| const api = getEndpoints("solana", network); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/client/src/client.ts (1)
39-68: Consider refactoring the type definition for better maintainability.The discriminated union type effectively prevents multiple wallet configurations, but consider extracting common properties to a base interface for better maintainability:
+interface WalletConfig { + signer?: never; + solanaAdapter?: never; + solanaWallet?: never; + wallet?: never; +} +interface SignerConfig extends WalletConfig { + signer: Signer; +} +interface SolanaAdapterConfig extends WalletConfig { + solanaAdapter: WalletContextState; +} // ... similar interfaces for other configs export type ZetaChainClientParams = ZetaChainClientParamsBase & (SignerConfig | SolanaAdapterConfig | /* other configs */);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.json(1 hunks)packages/client/src/client.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🔇 Additional comments (4)
packages/client/src/client.ts (4)
76-77: LGTM! Properties are well-typed and consistent.
The new Solana wallet properties follow the established pattern and are properly typed.
158-162: LGTM! Well-implemented public key retrieval.
The method properly handles null cases and uses appropriate nullish coalescing for fallbacks.
131-134:
Add validation for multiple wallet parameters.
The constructor should validate that only one wallet parameter is provided at a time.
As suggested in the previous review, add comprehensive validation:
if (params.wallet && params.signer) {
throw new Error("You can only provide a wallet or a signer, not both.");
+} else if (
+ [params.wallet, params.signer, params.solanaWallet, params.solanaAdapter].filter(Boolean).length > 1
+) {
+ throw new Error("You can only provide one wallet parameter.");
} else if (params.wallet) {154-156:
Ensure consistent boolean return type.
The method should always return a boolean value, not undefined.
As suggested in the previous review:
public isSolanaWalletConnected(): boolean {
- return this.solanaAdapter?.connected || this.solanaWallet !== undefined;
+ return (
+ (this.solanaAdapter ? this.solanaAdapter.connected : false) ||
+ (this.solanaWallet !== undefined)
+ );
}
solanaDepositto support browserRelated issue #182
Summary by CodeRabbit
Release Notes
New Features
solanaDepositfunction for improved transaction handling and error reporting.Bug Fixes
Chores