Issue-002 Error adding voting addresses: restore legacy WIF validatio…

[osiastedian]

User description

…n; scope descriptor encryption

---

PR Type

Enhancement, Bug fix

---

Description

• Improved address validation.

• Fixed legacy WIF handling.

• Enhanced descriptor wallet support.

• Added error handling for invalid addresses.

---

Diagram Walkthrough

flowchart LR
  A[AddAddress Form] --> B{Address Validation};
  B -- Valid Address --> C[Success];
  B -- Invalid Address --> D[Error];
  E[Legacy WIF] --> B;
  F[Descriptor Wallet] --> B;

Loading

File Walkthrough

Relevant files

Bug fix

encryption.js Enhanced Encryption & Error Handling                                          src/utils/encryption.js Added type check for descriptor wallets before deriving private keys. Improved error handling for cases where the voting address does not belong to the provided descriptor wallet. Modified encryption of private keys to handle both descriptor and legacy WIF formats. Enhanced the encryption process to handle different key types more robustly. +42/-41  SingleAddressForm.jsx Improved Address Validation & Error Handling                          src/components/profile/AddAddress/SingleAddressForm.jsx Improved address validation to handle both legacy WIF and descriptor wallets. Added more robust error handling for invalid addresses. Updated schema validation to correctly handle different address types. Enhanced user experience by providing more informative error messages. +11/-9

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Bug The loop to find the matching address in the descriptor wallet iterates a maximum of 100 times. If the address is not found within the first 100 indices, an error is thrown. Consider handling cases where the address might exist beyond this limit or implementing a more robust search strategy. for (let i = 0; i < maxIndex; i++) { const fullPath = basePath.replace("*", i.toString()); const node = rootNode.derive(fullPath); const pubkey = Buffer.from(node.publicKey); const { address: derivedAddress } = payments.p2wpkh({ pubkey, network, }); if (derivedAddress === address) { derivedNode = node; break; } } if (!derivedNode) { throw new Error( `Voting address ${address} does not belong to the provided descriptor wallet.` ); } Incomplete Validation The address validation for legacy WIF addresses only checks if the derived address matches the input address. It might be beneficial to add additional checks to ensure the validity of the WIF itself, beyond just the address derivation. try { const derived = deriveAddressFromWifPrivKey(parent.privateKey); return (derived || "").toLowerCase() === value.toLowerCase(); } catch (_) { return false; }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Improve legacy key encryption The code handles privateKey encryption differently based on the type. However, the else block within the if (key === "privateKey") statement lacks error handling. Add a try...catch block to handle potential errors during encryption of legacy WIF private keys. This will prevent unexpected crashes and improve the robustness of the function. src/utils/encryption.js [47-94] if (key === "privateKey") { if (type === "descriptor") { // Derive the node matching the provided votingAddress from the descriptor wallet const network = process.env.REACT_APP_CHAIN_NETWORK === "main" ? syscoinNetworks.mainnet : syscoinNetworks.testnet; const { xprv } = parseDescriptor(value); const rootNode = HDKey.fromExtendedKey(xprv, network.bip32); const basePath = "m/84h/1h/0h/0/*".replaceAll("h", "'"); const maxIndex = 100; let derivedNode = null; for (let i = 0; i < maxIndex; i++) { const fullPath = basePath.replace("*", i.toString()); const node = rootNode.derive(fullPath); const pubkey = Buffer.from(node.publicKey); const { address: derivedAddress } = payments.p2wpkh({ pubkey, network, }); if (derivedAddress === address) { derivedNode = node; break; } } if (!derivedNode) { throw new Error( `Voting address ${address} does not belong to the provided descriptor wallet.` ); } const privateKeyHex = Buffer.from(derivedNode.privateKey).toString( "hex" ); encryptedData[key] = CryptoJS.AES.encrypt( privateKeyHex, derivedKey ).toString(); } else { // Legacy: encrypt the provided WIF directly - encryptedData[key] = CryptoJS.AES.encrypt( - value, - derivedKey - ).toString(); + try { + encryptedData[key] = CryptoJS.AES.encrypt( + value, + derivedKey + ).toString(); + } catch (error) { + console.error("Error encrypting legacy WIF:", error); + // Handle the error appropriately, e.g., throw a new error, log it, or return a default value. + } } } else { encryptedData[key] = CryptoJS.AES.encrypt( value.toString(), derivedKey ).toString(); } Suggestion importance[1-10]: 7 __ Why: The suggestion adds a try...catch block to handle potential errors during the encryption of legacy WIF private keys, improving the robustness of the function. The impact is moderate because it addresses a potential error condition, but it's not a critical bug fix.	Medium
	Improve error handling in validation The catch block in the legacy address validation currently uses a generic error catch (_). This makes debugging difficult. Replace the generic catch with a specific error catch that handles the expected error from deriveAddressFromWifPrivKey. Log the error for debugging purposes, and return false to indicate validation failure. This will improve error handling and provide more informative error messages. src/components/profile/AddAddress/SingleAddressForm.jsx [36-41] if (!parent?.privateKey || !value) { return false; } if (parent.type === "legacy") { // Validate that provided legacy WIF corresponds to the provided address try { const derived = deriveAddressFromWifPrivKey(parent.privateKey); return (derived || "").toLowerCase() === value.toLowerCase(); - } catch (_) { + } catch (error) { + console.error("Error deriving address from WIF:", error); return false; } } Suggestion importance[1-10]: 5 __ Why: The suggestion improves error handling by replacing a generic catch block with a specific one that logs the error, making debugging easier. The impact is minor because it enhances logging and doesn't directly fix a bug, but it improves maintainability.	Low