feat: enable Safe recovery for HBAR

modules/sdk-coin-algo/src/algo.ts

@@ -170,6 +170,10 @@ export interface RecoveryOptions {
   externalBackupSigner?: ExternalEd25519Signer;
 }
 
+/**
+ * Signed recovery result. Optional key/account fields are populated for Safe recovery when
+ * `externalBackupSigner` is supplied. They expose only public material.
+ */
 interface RecoveryInfo {
   id: string;
   tx: string;
@@ -180,6 +184,14 @@ interface RecoveryInfo {
   genesisId: string;
   genesisHash: string;
   note?: string;
+  /** Derived user public key encoded as an ALGO address, never the encrypted `userKey` input. */
+  userKey?: string;
+  /** Derived backup public key encoded as an ALGO address, not a private key or seed. */
+  backupKey?: string;
+  /** BitGo public key encoded as an ALGO address. */
+  bitgoKey?: string;
+  /** Wallet address requiring two of the three keys to authorize a transfer; not an individual key. */
+  rootAddress?: string;
 }
 
 export interface OfflineVaultTxInfo {
@@ -1073,6 +1085,14 @@ export class Algo extends BaseCoin {
       genesisId: genesisId,
       genesisHash: genesisHash,
       note: txJson.note ? Buffer.from(txJson.note.buffer).toString('utf-8') : undefined,
+      ...(externalBackupSigner
+        ? {
+            userKey: userKeyAddress,
+            backupKey: backupKeyAddress,
+            bitgoKey: this.stellarAddressToAlgoAddress(params.bitgoKey),
+            rootAddress: params.rootAddress,
+          }
+        : {}),
     };
   }
 

modules/sdk-coin-algo/test/unit/algo.ts

@@ -820,6 +820,10 @@ describe('ALGO:', function () {
         recovery.should.have.property('coin', 'talgo');
         recovery.should.have.property('firstRound');
         recovery.should.have.property('lastRound');
+        assert.deepStrictEqual(
+          Object.keys(recovery).sort(),
+          ['tx', 'id', 'coin', 'fee', 'firstRound', 'lastRound', 'genesisId', 'genesisHash', 'note'].sort()
+        );
         getBalanceStub.callCount.should.equal(1);
         const factory = new AlgoLib.TransactionBuilderFactory(coins.get('algo'));
         const txBuilder = factory.from(recovery.tx);

modules/sdk-coin-algo/test/unit/algoSafeRecovery.ts

@@ -1,3 +1,4 @@
+import assert from 'assert';
 import should from 'should';
 import { Talgo } from '../../src';
 import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test';
@@ -10,6 +11,7 @@ import {
   deriveSafeWalletKeys,
   encodeDerivableEd25519Pub,
   encodeEd25519ChainCode,
+  RecoveryKeyMismatchError,
   type DecryptedSafeRoots,
 } from '@bitgo/sdk-core';
 import algosdk from 'algosdk';
@@ -25,13 +27,18 @@ import algosdk from 'algosdk';
  */
 describe('Safe ALGO recovery end-to-end', function () {
   let bitgo: TestBitGoAPI;
-  let basecoin;
+  let basecoin: Algo;
+  const sandbox = Sinon.createSandbox();
+
+  afterEach(function () {
+    sandbox.restore();
+  });
 
   before(function () {
     bitgo = TestBitGo.decorate(BitGoAPI, { env: 'mock' });
     bitgo.safeRegister('talgo', Talgo.createInstance);
     bitgo.initializeTestVars();
-    basecoin = bitgo.coin('talgo');
+    basecoin = bitgo.coin('talgo') as Algo;
   });
 
   // The ed25519Multisig root fixture the sdk-core unit tests pin (with a distinct BitGo root, so the
@@ -72,7 +79,6 @@ describe('Safe ALGO recovery end-to-end', function () {
   const recoveryDestination = 'GB3YETD5TSTBAIYGYHVWU3O3I7XGOB44HOZA5MOEF5M23CLLZKRQLEVAOA';
 
   it('recovers a Safe ALGO wallet whose rootAddress equals multisigAddress(1,2,[user,backup,bitgo])', async function () {
-    const sandbox = Sinon.createSandbox();
     sandbox.stub(Algo.prototype, 'getAccountBalance').resolves(nativeBalance);
 
     const roots = ed25519Roots();
@@ -106,7 +112,8 @@ describe('Safe ALGO recovery end-to-end', function () {
         algosdk.encodeAddress(Buffer.from(derived.keys.bitgo.pub, 'hex')),
       ],
     });
-    keyParams.rootAddress.should.equal(expectedRoot);
+    assert(keyParams.rootAddress);
+    assert.strictEqual(keyParams.rootAddress, expectedRoot);
 
     const recovery = await basecoin.recover({
       userKey: keyParams.userKey,
@@ -120,19 +127,64 @@ describe('Safe ALGO recovery end-to-end', function () {
       externalBackupSigner: keyParams.externalBackupSigner,
     });
 
+    assert('tx' in recovery);
     recovery.should.have.property('tx');
+    assert.strictEqual(recovery.userKey, algosdk.encodeAddress(Buffer.from(derived.keys.user.pub, 'hex')));
+    assert.strictEqual(recovery.backupKey, algosdk.encodeAddress(Buffer.from(derived.keys.backup.pub, 'hex')));
+    assert.strictEqual(recovery.bitgoKey, algosdk.encodeAddress(Buffer.from(derived.keys.bitgo.pub, 'hex')));
+    assert.strictEqual(recovery.rootAddress, expectedRoot);
+    assert.notStrictEqual(recovery.userKey, keyParams.userKey);
+    recovery.should.not.have.property('walletPassphrase');
+    recovery.should.not.have.property('externalBackupSigner');
     const decoded = algosdk.decodeSignedTransaction(Buffer.from(recovery.tx as string, 'base64'));
     should(decoded.msig).not.be.undefined();
 
     const msigPk = algosdk.decodeAddress(expectedRoot).publicKey;
     algosdk.verifyMultisig(decoded.txn.bytesToSign(), decoded.msig!, msigPk).should.equal(true);
+  });
 
-    sandbox.restore();
+  it('returns the derived public keys for an unsigned Safe recovery without encrypting or decrypting', async function () {
+    sandbox.stub(Algo.prototype, 'getAccountBalance').resolves(nativeBalance);
+    const encryptSpy = sandbox.spy(bitgo, 'encrypt');
+    const decryptSpy = sandbox.spy(bitgo, 'decrypt');
+    const derived = await deriveSafeWalletKeys({
+      coin: 'talgo',
+      slot: 'ed25519Multisig',
+      account: 0,
+      roots: ed25519Roots(),
+    });
+    assert(derived.slot === 'ed25519Multisig');
+    const keyParams = await buildSafeRecoverKeyParams({
+      bitgo,
+      coin: 'talgo',
+      slot: 'ed25519Multisig',
+      keys: derived.keys,
+      mode: 'unsigned',
+    });
+    assert(keyParams.mode === 'unsigned');
+    assert(keyParams.rootAddress);
+
+    const recovery = await basecoin.recover({
+      ...keyParams,
+      rootAddress: keyParams.rootAddress,
+      fee,
+      recoveryDestination,
+      firstRound: 5002596,
+      nodeParams,
+    });
+    assert('txHex' in recovery);
+    const expectedKeys = [derived.keys.user.pub, derived.keys.backup.pub, derived.keys.bitgo.pub].map((pub) =>
+      algosdk.encodeAddress(Buffer.from(pub, 'hex'))
+    );
+    assert.deepStrictEqual([recovery.userKey, recovery.backupKey, recovery.bitgoKey], expectedKeys);
+    assert.deepStrictEqual(recovery.keys, expectedKeys);
+    assert.strictEqual(recovery.address, keyParams.rootAddress);
+    Sinon.assert.notCalled(encryptSpy);
+    Sinon.assert.notCalled(decryptSpy);
   });
 
   it('fails closed with RecoveryKeyMismatchError when the user key does not belong to the root', async function () {
-    const sandbox = Sinon.createSandbox();
-    sandbox.stub(Algo.prototype, 'getAccountBalance').resolves(nativeBalance);
+    const getBalanceStub = sandbox.stub(Algo.prototype, 'getAccountBalance').resolves(nativeBalance);
 
     const roots = ed25519Roots();
     const derived = await deriveSafeWalletKeys({ coin: 'algo', slot: 'ed25519Multisig', account: 0, roots });
@@ -151,20 +203,21 @@ describe('Safe ALGO recovery end-to-end', function () {
     }
 
     // A rootAddress the derived keys do not control: the fail-closed check fires before any node call.
-    await basecoin
-      .recover({
-        userKey: keyParams.userKey,
-        rootAddress: algosdk.encodeAddress(Buffer.alloc(32, 0xee)),
-        bitgoKey: keyParams.bitgoKey,
-        walletPassphrase: keyParams.walletPassphrase,
-        fee,
-        recoveryDestination,
-        firstRound: 5002596,
-        nodeParams,
-        externalBackupSigner: keyParams.externalBackupSigner,
-      })
-      .should.be.rejectedWith(/does not match the given rootAddress/);
-
-    sandbox.restore();
+    await assert.rejects(
+      () =>
+        basecoin.recover({
+          userKey: keyParams.userKey,
+          rootAddress: algosdk.encodeAddress(Buffer.alloc(32, 0xee)),
+          bitgoKey: keyParams.bitgoKey,
+          walletPassphrase: keyParams.walletPassphrase,
+          fee,
+          recoveryDestination,
+          firstRound: 5002596,
+          nodeParams,
+          externalBackupSigner: keyParams.externalBackupSigner,
+        }),
+      RecoveryKeyMismatchError
+    );
+    Sinon.assert.notCalled(getBalanceStub);
   });
 });

modules/sdk-coin-hbar/src/hbar.ts

@@ -6,8 +6,9 @@ import {
   BaseCoin,
   BitGoBase,
   ErrorNoInputToRecover,
-  assertValidExternalEd25519Signature,
-  ExternalEd25519Signature,
+  assertValidExternalEd25519Signer,
+  ExternalEd25519Signer,
+  signWithExternalEd25519Signer,
   KeyPair,
   ParsedTransaction,
   ParseTransactionOptions,
@@ -114,7 +115,7 @@ interface VerifyAddressOptions extends BaseVerifyAddressOptions {
 }
 
 export interface RecoveryOptions {
-  backupKey: string;
+  backupKey?: string;
   userKey: string;
   rootAddress: string;
   recoveryDestination: string;
@@ -124,16 +125,28 @@ export interface RecoveryOptions {
   nodeId?: string;
   startTime?: string;
   tokenId?: string;
-  /** Optional pre-made signature for the backup child, whose signature must be over the transaction body bytes (`bodyBytes`). */
-  externalBackupSignature?: ExternalEd25519Signature;
+  /** Lazy signer for the seedless backup child; the signed body bytes only exist after recover() builds the transaction. */
+  externalBackupSigner?: ExternalEd25519Signer;
 }
 
+/**
+ * Signed recovery result. Optional key/account fields are populated for Safe recovery when
+ * `externalBackupSigner` is supplied. They expose only public material.
+ */
 export interface RecoveryInfo {
   id: string;
   tx: string;
   coin: string;
   startTime: string;
   nodeId: string;
+  /** Derived user public key with the Hedera public-key prefix, never the encrypted `userKey` input. */
+  userKey?: string;
+  /** Derived backup public key with the Hedera public-key prefix, not a private key or seed. */
+  backupKey?: string;
+  /** BitGo public key with the Hedera public-key prefix, when supplied by the caller. */
+  bitgoKey?: string;
+  /** Caller-supplied Hedera account ID (for example, `0.0.12345`), not a public or private key. */
+  rootAddress?: string;
 }
 
 export interface OfflineVaultTxInfo {
@@ -566,9 +579,11 @@ export class Hbar extends BaseCoin {
    * @param params
    */
   public async recover(params: RecoveryOptions): Promise<RecoveryInfo | OfflineVaultTxInfo> {
+    const backupKey = params.backupKey;
     const isUnsignedSweep =
-      (params.backupKey.startsWith(PUBLIC_KEY_PREFIX) && params.userKey.startsWith(PUBLIC_KEY_PREFIX)) ||
-      (Utils.isValidPublicKey(params.userKey) && Utils.isValidPublicKey(params.backupKey));
+      backupKey !== undefined &&
+      ((backupKey.startsWith(PUBLIC_KEY_PREFIX) && params.userKey.startsWith(PUBLIC_KEY_PREFIX)) ||
+        (Utils.isValidPublicKey(params.userKey) && Utils.isValidPublicKey(backupKey)));
 
     // Validate the root address
     if (!this.isValidAddress(params.rootAddress)) {
@@ -600,8 +615,19 @@ export class Hbar extends BaseCoin {
       throw new Error('start time is required for unsigned sweep');
     }
 
-    if (params.externalBackupSignature) {
-      assertValidExternalEd25519Signature(params.externalBackupSignature);
+    if (params.externalBackupSigner) {
+      assertValidExternalEd25519Signer(params.externalBackupSigner);
+    }
+
+    // Normalize the caller-supplied BitGo public key before any node query or signing: a malformed
+    // key must fail fast rather than surface after the backup signer has already run.
+    let bitgoPub: string | undefined;
+    if (params.externalBackupSigner && params.bitgoKey) {
+      try {
+        bitgoPub = new HbarKeyPair({ pub: params.bitgoKey }).getKeys().pub;
+      } catch (e) {
+        throw new Error('invalid bitgoKey, got: ' + params.bitgoKey);
+      }
     }
 
     if (!isUnsignedSweep && !params.walletPassphrase) {
@@ -612,13 +638,18 @@ export class Hbar extends BaseCoin {
     let backUp: string | undefined;
     let userPub: string | undefined;
     let backupPub: string | undefined;
+    const externalBackupSigner = params.externalBackupSigner;
+    const externalBackupPub = externalBackupSigner?.publicKey;
     if (!isUnsignedSweep) {
       try {
         userPrv = await this.bitgo.decrypt({ input: params.userKey, password: params.walletPassphrase });
-        if (params.externalBackupSignature) {
-          backupPub = new HbarKeyPair({ pub: params.externalBackupSignature.publicKey }).getKeys().pub;
+        if (externalBackupPub) {
+          backupPub = new HbarKeyPair({ pub: externalBackupPub }).getKeys().pub;
         } else {
-          backUp = await this.bitgo.decrypt({ input: params.backupKey, password: params.walletPassphrase });
+          if (!backupKey) {
+            throw new Error('backupKey is required for non-bitgo recovery when no external backup signer is provided');
+          }
+          backUp = await this.bitgo.decrypt({ input: backupKey, password: params.walletPassphrase });
           backupPub = new HbarKeyPair({ prv: backUp }).getKeys().pub;
         }
       } catch (e) {
@@ -635,8 +666,8 @@ export class Hbar extends BaseCoin {
     }
 
     // validate backupKey for unsigned sweep
-    if (isUnsignedSweep && !Utils.isValidPublicKey(params.backupKey)) {
-      throw new Error('invalid backupKey, got: ' + params.backupKey);
+    if (isUnsignedSweep && backupKey !== undefined && !Utils.isValidPublicKey(backupKey)) {
+      throw new Error('invalid backupKey, got: ' + backupKey);
     }
 
     const { address: destinationAddress, memoId } = Utils.getAddressDetails(params.recoveryDestination);
@@ -646,13 +677,13 @@ export class Hbar extends BaseCoin {
     const userRaw = PublicKey.fromString(userPub ?? params.userKey)
       .toStringRaw()
       .toLowerCase();
-    const backupRaw = PublicKey.fromString(backupPub ?? params.backupKey)
+    const backupRaw = PublicKey.fromString(backupPub ?? backupKey!)
       .toStringRaw()
       .toLowerCase();
     if (!onChainSigners.has(userRaw) || !onChainSigners.has(backupRaw)) {
       throw new RecoveryKeyMismatchError(
         `user key ${userPub ?? params.userKey} or backup key ${
-          backupPub ?? params.backupKey
+          backupPub ?? backupKey
         } is not in the key list of account ${params.rootAddress}`
       );
     }
@@ -711,7 +742,7 @@ export class Hbar extends BaseCoin {
         nodeId: txJson.node,
         memo: txJson.memo,
         userKey: params.userKey,
-        backupKey: params.backupKey,
+        backupKey,
         bitgoKey: params.bitgoKey,
         maxFee: fee,
         address: params.rootAddress,
@@ -723,24 +754,34 @@ export class Hbar extends BaseCoin {
 
     txBuilder.sign({ key: userPrv });
 
-    const externalBackupSignature = params.externalBackupSignature;
-    if (externalBackupSignature) {
-      txBuilder.signature(
-        externalBackupSignature.signature,
-        new HbarKeyPair({ pub: externalBackupSignature.publicKey })
-      );
-    } else {
+    if (!externalBackupPub) {
       txBuilder.sign({ key: backUp });
     }
 
-    const tx = await txBuilder.build();
+    const tx = (await txBuilder.build()) as Transaction;
+
+    if (externalBackupSigner) {
+      // Slot-2 Safe recovery: the seedless backup child signs lazily, over the exact transaction the
+      // user key just signed. A pre-made signature cannot cover the body bytes, since the amount and
+      // start time are only known once the transaction is built.
+      const signature = await signWithExternalEd25519Signer(externalBackupSigner, tx.hederaTx.bodyBytes);
+      tx.addSignature(signature, new HbarKeyPair({ pub: externalBackupSigner.publicKey }));
+    }
 
     return {
       tx: tx.toBroadcastFormat(),
       id: tx.toJson().id,
       coin: this.getChain(),
       startTime: tx.toJson().startTime,
       nodeId: tx.toJson().node,
+      ...(externalBackupSigner
+        ? {
+            userKey: userPub,
+            backupKey: backupPub,
+            bitgoKey: bitgoPub,
+            rootAddress: params.rootAddress,
+          }
+        : {}),
     };
   }