Enable undelegation on hardware wallets (#292)

* Enable undelegation on hardware wallets * Don't delegate change on ledgers * Add change address, to use in ledger address confirmation * Fix ledger error * Add OP_FALSE to ledger inputs * Add .js * Add missing imports * Fix owner address and p2cs spend on ledger
PIVX-Labs · Jan 31, 2024 · 3243079 · 3243079
1 parent ae28b99
commit 3243079
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 16 deletions.
diff --git a/scripts/ledger.js b/scripts/ledger.js
@@ -6,6 +6,8 @@ import { confirmPopup, createAlert } from './misc.js';
 import { getNetwork } from './network.js';
 import { Transaction } from './transaction.js';
 import { COIN, cChainParams } from './chain_params.js';
+import { hexToBytes, bytesToHex } from './utils.js';
+import { OP } from './script.js';
 
 /**
  * @type{TransportWebUSB}
@@ -156,11 +158,14 @@ export async function ledgerSignTransaction(wallet, transaction) {
 
     const associatedKeysets = [];
     const inputs = [];
+    const isColdStake = [];
     for (const input of transaction.vin) {
         const { hex } = await getNetwork().getTxInfo(input.outpoint.txid);
+        const { type } = wallet.getAddressesFromScript(input.scriptSig);
         inputs.push([cHardwareWallet.splitTransaction(hex), input.outpoint.n]);
         // ScriptSig is the script at this point, since it's not signed
         associatedKeysets.push(wallet.getPath(input.scriptSig));
+        isColdStake.push(type === 'p2cs');
     }
     const outputScriptHex = cHardwareWallet
         .serializeTransactionOutputs(ledgerTx)
@@ -174,10 +179,25 @@ export async function ledgerSignTransaction(wallet, transaction) {
             outputScriptHex,
         }),
     });
+
     const signedTx = Transaction.fromHex(hex);
     // Update vin with signatures
     transaction.vin = signedTx.vin;
-    return signedTx;
+    for (let i = 0; i < transaction.vin.length; i++) {
+        const input = transaction.vin[i];
+        // if it's a cold stake tx we need to add OP_FALSE
+        if (isColdStake[i]) {
+            const bytes = hexToBytes(input.scriptSig);
+            const sigLength = bytes[0];
+            input.scriptSig = bytesToHex([
+                bytes[0],
+                ...bytes.slice(1, sigLength + 1),
+                OP['FALSE'],
+                ...bytes.slice(sigLength + 1),
+            ]);
+        }
+    }
+    return transaction;
 }
 
 function createTxConfirmation(outputs) {

diff --git a/scripts/legacy.js b/scripts/legacy.js
@@ -30,13 +30,17 @@ export async function createAndSendTransaction({
     delegateChange = false,
     changeDelegationAddress = null,
     isProposal = false,
+    changeAddress = '',
+    delegationOwnerAddress,
 }) {
     const tx = wallet.createTransaction(address, amount, {
         isDelegation,
         useDelegatedInputs,
         delegateChange,
         changeDelegationAddress,
         isProposal,
+        changeAddress,
+        returnAddress: delegationOwnerAddress,
     });
     if (!wallet.isHardwareWallet()) await wallet.sign(tx);
     else {
@@ -54,18 +58,16 @@ export async function createAndSendTransaction({
  * @deprecated use the new wallet method instead
  */
 export async function undelegateGUI() {
-    if (wallet.isHardwareWallet()) {
-        return createAlert('warning', ALERTS.STAKING_LEDGER_NO_SUPPORT, 6000);
-    }
-
     // Ensure the wallet is unlocked
-    if (
-        wallet.isViewOnly() &&
-        !(await restoreWallet(
-            `${translation.walletUnlockUnstake} ${cChainParams.current.TICKER}!`
-        ))
-    )
-        return;
+    if (!wallet.isHardwareWallet()) {
+        if (
+            wallet.isViewOnly() &&
+            !(await restoreWallet(
+                `${translation.walletUnlockUnstake} ${cChainParams.current.TICKER}!`
+            ))
+        )
+            return;
+    }
 
     // Verify the amount
     const nAmount = Math.round(
@@ -74,16 +76,20 @@ export async function undelegateGUI() {
     if (!validateAmount(nAmount)) return;
 
     // Generate a new address to undelegate towards
-    const [address] = wallet.getNewAddress(1);
+
+    const [address] = await getNewAddress({
+        verify: wallet.isHardwareWallet(),
+    });
 
     // Perform the TX
     const cTxRes = await createAndSendTransaction({
         address,
         amount: nAmount,
         isDelegation: false,
         useDelegatedInputs: true,
-        delegateChange: true,
+        delegateChange: !wallet.isHardwareWallet(),
         changeDelegationAddress: await wallet.getColdStakingAddress(),
+        changeAddress: address,
     });
 
     if (!cTxRes.ok && cTxRes.err === 'No change addr') {
@@ -103,6 +109,9 @@ export async function undelegateGUI() {
  * @deprecated use the new wallet method instead
  */
 export async function delegateGUI() {
+    if (wallet.isHardwareWallet()) {
+        return createAlert('warning', ALERTS.STAKING_LEDGER_NO_SUPPORT);
+    }
     // Ensure the wallet is unlocked
     if (
         wallet.isViewOnly() &&

diff --git a/scripts/wallet.js b/scripts/wallet.js
@@ -666,6 +666,8 @@ export class Wallet {
             delegateChange = false,
             changeDelegationAddress = null,
             isProposal = false,
+            changeAddress = '',
+            returnAddress = '',
         } = {}
     ) {
         const balance = useDelegatedInputs
@@ -689,7 +691,7 @@ export class Wallet {
 
         // Add change output
         if (changeValue > 0) {
-            const [changeAddress] = this.getNewAddress(1);
+            if (!changeAddress) [changeAddress] = this.getNewAddress(1);
             if (delegateChange && changeValue > 1.01 * COIN) {
                 transactionBuilder.addColdStakeOutput({
                     address: changeAddress,
@@ -709,7 +711,7 @@ export class Wallet {
 
         // Add primary output
         if (isDelegation) {
-            const [returnAddress] = this.getNewAddress(1);
+            if (!returnAddress) [returnAddress] = this.getNewAddress(1);
             transactionBuilder.addColdStakeOutput({
                 address: returnAddress,
                 addressColdStake: address,

diff --git a/tests/unit/wallet.spec.js b/tests/unit/wallet.spec.js
@@ -77,6 +77,38 @@ describe('Wallet transaction tests', () => {
         );
     });
 
+    it('Creates a tx with change address', async () => {
+        const wallet = new Wallet(0, false);
+        wallet.setMasterKey(getLegacyMainnet());
+        const tx = wallet.createTransaction(
+            'DLabsktzGMnsK5K9uRTMCF6NoYNY6ET4Bb',
+            0.05 * 10 ** 8,
+            { changeAddress: 'D8Ervc3Ka6TuKgvXZH9Eo4ou24AiVwTbL6' }
+        );
+        expect(tx.version).toBe(1);
+        expect(tx.vin[0]).toStrictEqual(
+            new CTxIn({
+                outpoint: new COutpoint({
+                    txid: 'f8f968d80ac382a7b64591cc166489f66b7c4422f95fbd89f946a5041d285d7c',
+                    n: 1,
+                }),
+                scriptSig: '76a914f49b25384b79685227be5418f779b98a6be4c73888ac', // Script sig must be the UTXO script since it's not signed
+            })
+        );
+        expect(tx.vout[0]).toStrictEqual(
+            new CTxOut({
+                script: '76a91421ff8214d09d60713b89809bb413a0651ee6931488ac',
+                value: 4992400,
+            })
+        );
+        expect(tx.vout[1]).toStrictEqual(
+            new CTxOut({
+                script: '76a914a95cc6408a676232d61ec29dc56a180b5847835788ac',
+                value: 5000000,
+            })
+        );
+    });
+
     it('Creates a proposal tx correctly', async () => {
         const wallet = new Wallet(0, false);
         wallet.setMasterKey(getLegacyMainnet());