Update hardhat account, support solana
#190
Conversation
npx hardhat account --save
🔑 EVM Private key: 49db34db2e853bda91c17828eb614173573df9ec200bc563217aeb36687f2e0a
🔑 Solana Private key: 2kP9uN3ySc25UNqhqqTWz5Vt1vRQpcjfgmcJ346NERKaTwUq99nwbVsPg61znaMTnwBHbYyAGnkKRmKgjJhhWq73
🔐 EVM Mnemonic phrase: level treat gossip actress mistake parent trouble excite wild tone offer time
😃 EVM address: 0xD7043474A099Cad2a0A3c12F746BA5A9aD74e85f
😃 Bitcoin address: tb1qg5xxzl4qzn6pgee53a7kjve8lvewk43r8kpcjc
😃 Bech32 address: zeta16uzrga9qn89d9g9rcyhhg6a94xkhf6zlne0vkz
😃 Solana address: 2bZFnp4xkrhovr1ahDiuu7D8JxdoaWPyrptF4vDuy7ru |
|
@fadeev pls check |
|
Looking good! |
lukema95
requested review from
andresaiello,
fadeev and
a team
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request introduce functionality for handling Solana wallets in 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: 2
🧹 Outside diff range and nitpick comments (1)
packages/tasks/src/account.ts (1)
126-129: Exercise caution when displaying private keys in the console.Displaying private keys in plain text may pose a security risk if the console output is logged or viewed by unauthorized persons. Consider informing users to handle the displayed keys securely.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (26)
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/**
📒 Files selected for processing (1)
- packages/tasks/src/account.ts (3 hunks)
🧰 Additional context used
🪛 Biome
packages/tasks/src/account.ts
[error] 85-86: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
packages/tasks/src/account.ts (3)
63-64: Validate the Format ofsecretKeyBefore UsageBefore creating a
Keypairfrom thesecretKey, add validation to ensure it is in the correct format. This prevents potential runtime errors if theid.jsonfile contains invalid data.Consider adding a validation step:
const secretKey = JSON.parse(fileContent); +if (!Array.isArray(secretKey) || secretKey.length !== 64) { + throw new Error("Invalid Solana secret key format in id.json."); +}
74-75: Clarify Prompt Message for Solana Private Key InputTo improve usability, specify the expected format of the Solana private key in the prompt message. This helps users provide the correct input without confusion.
Update the prompt message:
let solanaPrivateKey = await input({ - message: "Solana private key (press Enter to skip):", + message: "Solana private key (base58 encoded, press Enter to skip):", });
126-138: Standardize Formatting of Console OutputsThe console outputs have inconsistent indentation, which may affect readability. Standardizing the indentation improves the user experience.
Adjust the indentation as follows:
console.log(` - 🔑 EVM Private key: ${pk}`); +🔑 EVM Private key: ${pk}`); solanaWallet && console.log(` - 🔑 Solana Private key: ${bs58.encode(solanaWallet.secretKey)}`); +🔑 Solana Private key: ${bs58.encode(solanaWallet.secretKey)}`); mnemonic && console.log(` - 🔐 EVM Mnemonic phrase: ${mnemonic.phrase}`); +🔐 EVM Mnemonic phrase: ${mnemonic.phrase}`); console.log(` - 😃 EVM address: ${address} - 😃 Bitcoin address: ${bitcoinAddress(pk, "testnet")} - 😃 Bech32 address: ${hexToBech32Address(address, "zeta")}`); +😃 EVM address: ${address} +😃 Bitcoin address: ${bitcoinAddress(pk, "testnet")} +😃 Bech32 address: ${hexToBech32Address(address, "zeta")}`); solanaWallet && console.log(` - 😃 Solana address: ${solanaWallet.publicKey.toString()}`); +😃 Solana address: ${solanaWallet.publicKey.toString()}`);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- packages/tasks/src/account.ts (3 hunks)
🧰 Additional context used
🪛 Biome
packages/tasks/src/account.ts
[error] 85-86: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
🔇 Additional comments (1)
packages/tasks/src/account.ts (1)
99-100: Verify Using the Same Private Key for EVM and BitcoinThe code assigns the same private key to both
EVM_PRIVATE_KEYandBTC_PRIVATE_KEY. Ensure that this is intentional and that using the same key across different platforms does not introduce security vulnerabilities.You can run the following script to check for any differing implementations:
packages/tasks/src/account.ts
Outdated
| import { bs58 } from "@coral-xyz/anchor/dist/cjs/utils/bytes"; | ||
| import { input } from "@inquirer/prompts"; | ||
| import { Keypair } from "@solana/web3.js"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use Direct Import of bs58 Package for Clarity and Maintainability
Instead of importing bs58 from "@coral-xyz/anchor/dist/cjs/utils/bytes", consider importing it directly from the bs58 package. This reduces dependency on internal paths of other packages, which may change and cause unexpected issues.
Apply this diff to import bs58 directly:
-import { bs58 } from "@coral-xyz/anchor/dist/cjs/utils/bytes";
+import bs58 from "bs58";Ensure the bs58 package is added to your project's dependencies:
npm install bs58
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/tasks/src/account.ts (1)
49-87: LGTM: Solana wallet retrieval function added.The new
getSolanaWalletFromLocalFileOrInputfunction provides a robust way to retrieve Solana wallets, with good support for both automatic loading from a config file and manual input.Consider improving error handling for config file loading.
As per a previous review comment, instead of throwing an error when failing to load the Solana wallet from the config file, consider logging the error and proceeding to prompt the user for input. This would enhance user experience by allowing recovery from errors without terminating the process.
Apply this diff to improve error handling:
} catch (error) { console.error("Failed to load Solana private key:", error); - throw new Error( - `Failed to load Solana wallet from ${solanaConfigPath}` - ); + console.log("Proceeding to manual input..."); }Add a retry limit for manual Solana key input.
The current implementation could potentially lead to an infinite loop if the user repeatedly enters invalid keys. Consider adding a maximum number of retry attempts to improve user experience.
Apply this diff to add a retry limit:
+ const MAX_RETRIES = 3; + let retries = 0; while (true) { + if (retries >= MAX_RETRIES) { + console.log("Maximum retry attempts reached. Skipping Solana wallet creation."); + return null; + } let solanaPrivateKey = await input({ message: "Solana private key (press Enter to skip):", }); if (solanaPrivateKey.trim() === "") { console.log("Skipped Solana private key input."); return null; } try { return Keypair.fromSecretKey(bs58.decode(solanaPrivateKey)); } catch (e) { console.error(`❌ Invalid Solana private key: ${e}`); + retries++; } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- packages/tasks/src/account.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (7)
packages/tasks/src/account.ts (7)
2-2: LGTM: Import changes for Solana support.The addition of
Keypairimport and the direct import ofbs58are appropriate for the new Solana functionality. Thebs58import change also addresses a previous review comment.Also applies to: 5-5
28-28: LGTM: Clarified prompt for EVM wallet recovery.The updated prompt message clearly specifies that the input is for EVM wallets, which is a good distinction given the new Solana support.
89-103: LGTM: UpdatedsavePrivateKeyfunction to support Solana.The changes to the
savePrivateKeyfunction appropriately handle the new Solana private key while maintaining existing functionality for EVM and BTC keys.Security concern: Storing private keys in plaintext.
Saving private keys in plaintext within environment files poses security risks, especially if the
.envfile is not properly secured or is accidentally committed to version control.
[security]
Consider implementing one or more of the following security measures:
- Encrypt the private keys before saving them to the .env file.
- Provide clear instructions to users on how to securely store and manage their private keys.
- Implement a secure key management system or integrate with existing key management solutions.
- Add a prominent warning in the console output about the security implications of storing private keys in the .env file.
110-118: LGTM: Integration of Solana wallet functionality inmainfunction.The changes to the
mainfunction effectively integrate Solana wallet functionality alongside existing EVM wallet handling. The updated variable names (evmWalletandsolanaWallet) improve code clarity.
125-140: LGTM: Comprehensive console output for EVM and Solana wallets.The updated console output provides clear and comprehensive information for both EVM and Solana wallets. The use of conditional statements ensures that Solana information is only displayed when available.
143-146: LGTM: UpdatedsavePrivateKeycall to include Solana private key.The
savePrivateKeycall has been correctly updated to include the Solana private key when available, aligning with the new function signature.
Line range hint
1-164: Overall: Excellent implementation of Solana wallet support.The changes in this pull request successfully introduce Solana wallet functionality while maintaining existing EVM and BTC support. The code is well-structured, with clear variable names and appropriate error handling. The integration of Solana wallet generation, recovery, and key saving is comprehensive and aligns well with the existing codebase.
A few minor suggestions for improvement have been made, primarily around error handling and security considerations. Addressing these points will further enhance the robustness and security of the implementation.
Great work on this feature addition!
related issue: #187
--saveoption, store it in theSOLANA_PRIVATE_KEYenvironment variable in the .env file.--recoveroption. When~/.config/solana/id.jsonexists locally, automatically load that private key. When it doesn't exist locally, users can input the private key in the command line or skip this step.PRIVATE_KEY,EVM_PRIVATE_KEY, andBTC_PRIVATE_KEYenvironment variables in the .env file, and their values are the same.Summary by CodeRabbit
New Features
Improvements