fix(sdk-coin-flrp): update fromAddresses in delegator tx

modules/sdk-coin-flrp/src/lib/permissionlessDelegatorTxBuilder.ts

@@ -7,14 +7,17 @@ import {
   pvmSerial,
   Credential,
   TransferOutput,
+  TransferableOutput,
+  TransferInput,
+  Address,
 } from '@flarenetwork/flarejs';
 import { BuildTransactionError, NotSupported, TransactionType } from '@bitgo/sdk-core';
 import { BaseCoin as CoinConfig } from '@bitgo/statics';
 import { Transaction } from './transaction';
 import { TransactionBuilder } from './transactionBuilder';
 import utils from './utils';
 import { FlrpFeeState } from '@bitgo/public-types';
-import { Tx } from './iface';
+import { Tx, DecodedUtxoObj } from './iface';
 
 /**
  * Builder for AddPermissionlessDelegator transactions on Flare P-Chain.
@@ -210,12 +213,10 @@ export class PermissionlessDelegatorTxBuilder extends TransactionBuilder {
   }
 
   /**
-   * Get the user's address (index 0) for delegation.
+   * Get the user's address (index 0) for default reward address.
    *
-   * For delegation transactions, we use only the user key because:
-   * 1. On-chain rewards go to the C-chain address derived from the delegator's public key
-   * 2. Using the user key ensures rewards go to the user's corresponding C-chain address
-   * 3. The user key is at index 0 in the fromAddresses array (BitGo convention: [user, bitgo, backup])
+   * The user key is at index 0 in the fromAddresses array (BitGo convention: [user, bitgo, backup]).
+   * This is used as the default rewardAddress parameter (though the parameter is ignored by protocol).
    *
    * @returns Buffer containing the user's address
    * @protected
@@ -237,8 +238,9 @@ export class PermissionlessDelegatorTxBuilder extends TransactionBuilder {
    * Uses pvm.e.newAddPermissionlessDelegatorTx (post-Etna API).
    *
    * Note: The rewardAddresses parameter is accepted by the API but does NOT affect
-   * where rewards are sent on-chain - rewards always go to the C-chain address
-   * derived from the delegator's public key (user key at index 0).
+   * where rewards are sent on-chain. Rewards accrue to C-chain addresses derived
+   * from the P-chain addresses in the stake outputs. The stake outputs contain the
+   * addresses from fromAddressesBytes (sorted to match UTXO owner order).
    * @protected
    */
   protected buildFlareTransaction(): void {
@@ -275,19 +277,24 @@ export class PermissionlessDelegatorTxBuilder extends TransactionBuilder {
     }
     const utxos = utils.decodedToUtxos(this.transaction._utxos, this.transaction._network.assetId);
 
-    // Use only the user key (index 0) for fromAddressesBytes
-    // This ensures the C-chain reward address is derived from the user's public key
+    // Get user address for default reward address derivation
     const userAddress = this.getUserAddress();
 
     const rewardAddresses =
       this.transaction._rewardAddresses.length > 0 ? this.transaction._rewardAddresses : [userAddress];
 
     // Use Etna (post-fork) API - pvm.e.newAddPermissionlessDelegatorTx
+    // IMPORTANT: Sort fromAddresses to match the sorted order in UTXOs
+    // The SDK sorts UTXO addresses (utils.ts:574) before passing to FlareJS,
+    // so fromAddressesBytes must also be sorted to match UTXO owner addresses
+    const fromAddressBuffers = this.transaction._fromAddresses.map((addr) => Buffer.from(addr));
+    const sortedFromAddresses = utils.sortAddressBuffersByHex(fromAddressBuffers);
+
     const delegatorTx = pvm.e.newAddPermissionlessDelegatorTx(
       {
         end: this._endTime,
         feeState: this._feeState,
-        fromAddressesBytes: [userAddress],
+        fromAddressesBytes: sortedFromAddresses,
         nodeId: this._nodeID,
         rewardAddresses: rewardAddresses,
         start: this._startTime,
@@ -298,7 +305,107 @@ export class PermissionlessDelegatorTxBuilder extends TransactionBuilder {
       this.transaction._context
     );
 
-    this.transaction.setTransaction(delegatorTx as UnsignedTx);
+    // Fix change output threshold bug (same as ExportInPTxBuilder)
+    const flareUnsignedTx = delegatorTx as UnsignedTx;
+    const innerTx = flareUnsignedTx.getTx() as pvmSerial.AddPermissionlessDelegatorTx;
+    const changeOutputs = innerTx.baseTx.outputs;
+    let correctedDelegatorTx: pvmSerial.AddPermissionlessDelegatorTx = innerTx;
+
+    if (changeOutputs.length > 0 && this.transaction._threshold > 1) {
+      // Only apply fix for multisig wallets (threshold > 1)
+      const allWalletAddresses = this.transaction._fromAddresses.map((addr) => Buffer.from(addr));
+
+      const correctedChangeOutputs = changeOutputs.map((output) => {
+        const transferOut = output.output as TransferOutput;
+
+        const assetIdStr = utils.flareIdString(Buffer.from(output.assetId.toBytes()).toString('hex')).toString();
+        return TransferableOutput.fromNative(
+          assetIdStr,
+          transferOut.amount(),
+          allWalletAddresses,
+          this.transaction._locktime,
+          this.transaction._threshold // Fix: use wallet's threshold instead of FlareJS's default (1)
+        );
+      });
+
+      correctedDelegatorTx = this.createCorrectedDelegatorTx(innerTx, correctedChangeOutputs);
+    }
+
+    // Recreate credentials and addressMaps from corrected transaction inputs
+    // This follows the same pattern as ExportInPTxBuilder to ensure proper signing
+    const utxosWithIndex = correctedDelegatorTx.baseTx.inputs.map((input) => {
+      const inputTxid = utils.cb58Encode(Buffer.from(input.utxoID.txID.toBytes()));
+      const inputOutputIdx = input.utxoID.outputIdx.value().toString();
+
+      const originalUtxo = this.transaction._utxos.find(
+        (utxo) => utxo.txid === inputTxid && utxo.outputidx === inputOutputIdx
+      );
+
+      if (!originalUtxo) {
+        throw new BuildTransactionError(`Could not find matching UTXO for input ${inputTxid}:${inputOutputIdx}`);
+      }
+
+      const transferInput = input.input as TransferInput;
+      const actualSigIndices = transferInput.sigIndicies();
+
+      return {
+        ...originalUtxo,
+        addressesIndex: originalUtxo.addressesIndex,
+        addresses: originalUtxo.addresses,
+        threshold: originalUtxo.threshold || this.transaction._threshold,
+        actualSigIndices,
+      };
+    });
+
+    this.transaction._utxos = utxosWithIndex;
+
+    const txCredentials = utxosWithIndex.map((utxo) =>
+      this.createCredentialForUtxo(utxo, utxo.threshold, utxo.actualSigIndices)
+    );
+
+    const addressMaps = utxosWithIndex.map((utxo) =>
+      this.createAddressMapForUtxo(utxo, utxo.threshold, utxo.actualSigIndices)
+    );
+
+    // Create new UnsignedTx with corrected change outputs and proper credentials
+    const fixedUnsignedTx = new UnsignedTx(
+      correctedDelegatorTx,
+      [],
+      new FlareUtils.AddressMaps(addressMaps),
+      txCredentials
+    );
+
+    this.transaction.setTransaction(fixedUnsignedTx);
+  }
+
+  /**
+   * Create a corrected AddPermissionlessDelegatorTx with the given change outputs.
+   * This is necessary because FlareJS's newAddPermissionlessDelegatorTx doesn't support setting
+   * the threshold and locktime for change outputs - it defaults to threshold=1.
+   *
+   * FlareJS declares baseTx.outputs as readonly, so we use Object.defineProperty
+   * to override the property with the corrected outputs. This is a workaround until
+   * FlareJS adds proper support for change output thresholds.
+   *
+   * @param originalTx - The original AddPermissionlessDelegatorTx
+   * @param correctedOutputs - The corrected change outputs with proper threshold
+   * @returns A new AddPermissionlessDelegatorTx with the corrected change outputs
+   */
+  private createCorrectedDelegatorTx(
+    originalTx: pvmSerial.AddPermissionlessDelegatorTx,
+    correctedOutputs: TransferableOutput[]
+  ): pvmSerial.AddPermissionlessDelegatorTx {
+    // FlareJS declares baseTx.outputs as `public readonly outputs: readonly TransferableOutput[]`
+    // We use Object.defineProperty to override the readonly property with our corrected outputs.
+    // This is necessary because FlareJS's newAddPermissionlessDelegatorTx doesn't support change output threshold/locktime.
+    Object.defineProperty(originalTx.baseTx, 'outputs', {
+      value: correctedOutputs,
+      writable: false,
+      enumerable: true,
+      configurable: true,
+    });
+
+    return originalTx;
   }
 
   /**
@@ -317,4 +424,103 @@ export class PermissionlessDelegatorTxBuilder extends TransactionBuilder {
   protected set transaction(transaction: Transaction) {
     this._transaction = transaction;
   }
+
+  /**
+   * Create credential for a UTXO following AVAX P approach.
+   * Embed user/recovery address, leave BitGo slot empty.
+   * Signing order is guaranteed: user signs first (address match), BitGo signs second (empty slot).
+   *
+   * @param utxo - The UTXO to create credential for
+   * @param threshold - Number of signatures required
+   * @param sigIndices - Optional sigIndices from FlareJS (if not provided, derived from addressesIndex)
+   * @protected
+   */
+  protected createCredentialForUtxo(utxo: DecodedUtxoObj, threshold: number, sigIndices?: number[]): Credential {
+    const sender = this.transaction._fromAddresses;
+    const addressesIndex = utxo.addressesIndex ?? [];
+    const firstIndex = 0; // User key index
+    const bitgoIndex = 1;
+
+    if (threshold === 1) {
+      if (sender && sender.length > firstIndex) {
+        return new Credential([utils.createEmptySigWithAddress(Buffer.from(sender[firstIndex]).toString('hex'))]);
+      }
+      return new Credential([utils.createNewSig('')]);
+    }
+
+    if (addressesIndex.length >= 2 && sender && sender.length >= threshold) {
+      const effectiveSigIndices =
+        sigIndices && sigIndices.length >= 2
+          ? sigIndices
+          : [addressesIndex[firstIndex], addressesIndex[bitgoIndex]].sort((a, b) => a - b);
+
+      const emptySignatures: ReturnType<typeof utils.createNewSig>[] = [];
+      for (const sigIdx of effectiveSigIndices) {
+        const senderIdx = addressesIndex.findIndex((utxoPos) => utxoPos === sigIdx);
+        if (senderIdx === firstIndex) {
+          emptySignatures.push(utils.createEmptySigWithAddress(Buffer.from(sender[firstIndex]).toString('hex')));
+        } else {
+          emptySignatures.push(utils.createNewSig(''));
+        }
+      }
+      return new Credential(emptySignatures);
+    }
+
+    const emptySignatures: ReturnType<typeof utils.createNewSig>[] = [];
+    for (let i = 0; i < threshold; i++) {
+      emptySignatures.push(utils.createNewSig(''));
+    }
+    return new Credential(emptySignatures);
+  }
+
+  /**
+   * Create AddressMap for a UTXO following AVAX P approach.
+   *
+   * @param utxo - The UTXO to create AddressMap for
+   * @param threshold - Number of signatures required
+   * @param sigIndices - Optional sigIndices from FlareJS (if not provided, derived from addressesIndex)
+   * @protected
+   */
+  protected createAddressMapForUtxo(
+    utxo: DecodedUtxoObj,
+    threshold: number,
+    sigIndices?: number[]
+  ): FlareUtils.AddressMap {
+    const addressMap = new FlareUtils.AddressMap();
+    const sender = this.transaction._fromAddresses;
+    const addressesIndex = utxo.addressesIndex ?? [];
+    const firstIndex = 0; // User key index
+    const bitgoIndex = 1;
+
+    if (threshold === 1) {
+      if (sender && sender.length > firstIndex) {
+        addressMap.set(new Address(sender[firstIndex]), 0);
+      } else if (sender && sender.length > 0) {
+        addressMap.set(new Address(sender[0]), 0);
+      }
+      return addressMap;
+    }
+
+    if (addressesIndex.length >= 2 && sender && sender.length >= threshold) {
+      const effectiveSigIndices =
+        sigIndices && sigIndices.length >= 2
+          ? sigIndices
+          : [addressesIndex[firstIndex], addressesIndex[bitgoIndex]].sort((a, b) => a - b);
+
+      effectiveSigIndices.forEach((sigIdx, slotIdx) => {
+        const senderIdx = addressesIndex.findIndex((utxoPos) => utxoPos === sigIdx);
+        if (senderIdx === bitgoIndex || senderIdx === firstIndex) {
+          addressMap.set(new Address(sender[senderIdx]), slotIdx);
+        }
+      });
+      return addressMap;
+    }
+
+    if (sender && sender.length >= threshold) {
+      sender.slice(0, threshold).forEach((addr, i) => {
+        addressMap.set(new Address(addr), i);
+      });
+    }
+    return addressMap;
+  }
 }