Skip to content

feat: accept Safe keycard params-EVM TSS recovery #10106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pranishnepal wants to merge 2 commits into
base: master
Choose a base branch
from
+1,554 −125
Open

feat: accept Safe keycard params-EVM TSS recovery #10106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2832ac
feat: accept Safe keycard params-EVM TSS recovery
pranishnepal Oct 30, 2026
1fdd01c
fix(abstract-eth): address review feedback on Safe keycard recovery
pranishnepal Oct 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions modules/abstract-cosmos/src/cosmosCoin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
multisigTypes,
ParsedTransaction,
ParseTransactionOptions,
RecoveryKeyMismatchError,
RecoveryProviderError,
SignedTransaction,
SigningError,
Expand Down Expand Up @@ -48,7 +49,7 @@ import {
TransactionBuildParams,
BalanceResult,
} from './lib';
import { ROOT_PATH } from './lib/constants';
import { COMPRESSED_PUBLIC_KEY_LENGTH, ROOT_PATH } from './lib/constants';
import utils from './lib/utils';
import { auditEcdsaPrivateKey } from '@bitgo/sdk-lib-mpc';

Expand Down Expand Up @@ -212,21 +213,44 @@ export class CosmosCoin<CustomMessage = never> extends BaseCoin {
}> {
const MPC = new Ecdsa();

let publicKey: string;
let keyShares: KeyShares | undefined;

if (isUnsignedSweep) {
return {
senderAddress: params.rootAddress as string,
publicKey: MPC.deriveUnhardened(params.bitgoKey || '', ROOT_PATH).slice(0, 66),
};
const bitgoKey = (params.bitgoKey || '').replace(/\s/g, '');
if (!bitgoKey) {
throw new Error('missing bitgoKey');
}
publicKey = this.derivePublicKey(MPC, bitgoKey);
} else {
keyShares = await this.getKeyShares(params);
publicKey = this.derivePublicKey(MPC, keyShares.commonKeyChain);
}

const keyShares = await this.getKeyShares(params);
const publicKey = MPC.deriveUnhardened(keyShares.commonKeyChain, ROOT_PATH).slice(0, 66);
const senderAddress = this.getAddressFromPublicKey(publicKey);
if (params.rootAddress && !this.isSameBaseAddress(senderAddress, params.rootAddress)) {
throw new RecoveryKeyMismatchError(
`derived address ${senderAddress} does not match rootAddress ${params.rootAddress}`
);
}

return {
senderAddress: this.getAddressFromPublicKey(publicKey),
publicKey,
keyShares,
};
return { senderAddress, publicKey, keyShares };
}

/**
* Compares two cosmos addresses ignoring an optional `?memoId=` suffix and bech32 casing,
* both of which `isValidAddress` accepts on a rootAddress.
*/
private isSameBaseAddress(a: string, b: string): boolean {
const base = (address: string) => address.split('?memoId=')[0].toLowerCase();
return base(a) === base(b);
}

/**
* Derives the compressed public key of the wallet's root address from a common keychain
*/
private derivePublicKey(MPC: Ecdsa, commonKeyChain: string): string {
return MPC.deriveUnhardened(commonKeyChain, ROOT_PATH).slice(0, COMPRESSED_PUBLIC_KEY_LENGTH);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions modules/abstract-cosmos/src/lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,6 @@ export const groupProposalMsgTypeUrl = '/cosmos.group.v1.MsgSubmitProposal';
export const groupVoteMsgTypeUrl = '/cosmos.group.v1.MsgVote';
export const UNAVAILABLE_TEXT = 'UNAVAILABLE';
export const ROOT_PATH = 'm/0';
/** length in hex characters of a compressed secp256k1 public key */
export const COMPRESSED_PUBLIC_KEY_LENGTH = 66;
export const sendMsgType = '/types.MsgSend'; // thorchain uses this custom message type
135 changes: 97 additions & 38 deletions modules/abstract-eth/src/abstractEthLikeNewCoins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* @prettier
*/
import {
classifyRecoveryKeys,
AddressCoinSpecific,
BitGoBase,
BuildNftTransferDataOptions,
Expand Down Expand Up @@ -284,8 +285,12 @@ export type NonTSSRecoverOptions = {
export type TSSRecoverOptions = RecoverOptionsWithBytes | NonTSSRecoverOptions;

export type RecoverOptions = {
userKey: string;
backupKey: string;
/** [encrypted] xprv, xpub or TSS key share. Omitted by unsigned TSS sweeps which pass `bitgoKey` instead. */
userKey?: string;
/** [encrypted] xprv, xpub or TSS key share. Omitted by unsigned TSS sweeps which pass `bitgoKey` instead. */
backupKey?: string;
/** TSS common keychain, the only key an unsigned Safe sweep supplies */
bitgoKey?: string;
walletPassphrase?: string;
walletContractAddress: string; // use this as walletBaseAddress for TSS
recoveryDestination: string;
Expand Down Expand Up @@ -1343,21 +1348,26 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
* @returns {void}
*/
validateRecoveryParams(params: RecoverOptions): void {
if (params.userKey === undefined) {
throw new Error('missing userKey');
}
// Unsigned TSS sweeps (Safe recovery) supply the common keychain as `bitgoKey` alone
const isBitgoKeyOnlyTss = params.isTss === true && !params.userKey && !params.backupKey && !!params.bitgoKey;

if (params.backupKey === undefined) {
throw new Error('missing backupKey');
}
if (!isBitgoKeyOnlyTss) {
if (params.userKey === undefined) {
throw new Error('missing userKey');
}

if (
!params.isUnsignedSweep &&
params.walletPassphrase === undefined &&
!params.userKey.startsWith('xpub') &&
!params.isTss
) {
throw new Error('missing wallet passphrase');
if (params.backupKey === undefined) {
throw new Error('missing backupKey');
}

if (
!params.isUnsignedSweep &&
params.walletPassphrase === undefined &&
!params.userKey.startsWith('xpub') &&
!params.isTss
) {
throw new Error('missing wallet passphrase');
}
}

if (params.walletContractAddress === undefined || !this.isValidAddress(params.walletContractAddress)) {
Expand Down Expand Up @@ -1506,6 +1516,9 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
}
}

// NOTE: deliberately keyed on params.userKey only, not getTssCommonKeychain. Resolving the
// TSS fallback chain here would scan an extra derived address whenever a caller supplies
// backupKey without userKey, changing which addresses a consolidation sweeps.
if (params.userKey) {
try {
const derivedAddress = this.deriveAddressFromPublicKey(params.userKey, index);
Expand Down Expand Up @@ -1545,6 +1558,15 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
const consolidatedTransactions: any[] = [];
let lastScanIndex = startIdx;

// An unsigned TSS sweep builds from the m/0 address derived from the common keychain and
// never reads walletContractAddress, so the forwarder address was never the address being
// swept here. Pass the address actually swept, which is what the key mismatch check compares.
// Loop-invariant, and only for the unsigned shape: in the signed shape userKey/backupKey hold
// encrypted key shares, and deriving from that ciphertext would throw before the loop starts.
const isTssUnsignedSweep = params.isTss === true && classifyRecoveryKeys(params).isUnsignedSweep;
const tssCommonKeychain = isTssUnsignedSweep ? this.getTssCommonKeychain(params) : '';
const tssBaseAddress = tssCommonKeychain ? this.deriveAddressFromPublicKey(tssCommonKeychain, 0) : undefined;

for (let i = startIdx; i < endIdx; i++) {
const consolidationAddress = this.getConsolidationAddress(params, i);
for (const address of consolidationAddress) {
Expand All @@ -1554,7 +1576,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
gasLimit: params.gasLimit,
recoveryDestination: params.recoveryDestination || '',
userKey: params.userKey || '',
walletContractAddress: address,
walletContractAddress: tssBaseAddress ?? address,
derivationSeed: '',
isTss: params.isTss,
eip1559: {
Expand All @@ -1565,7 +1587,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
chain: params.replayProtectionOptions?.chain || 0,
hardfork: params.replayProtectionOptions?.hardfork || 'london',
},
bitgoKey: '',
bitgoKey: params.bitgoKey || '',
ignoreAddressTypes: [],
};
let recoveryTransaction;
Expand Down Expand Up @@ -1620,11 +1642,9 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
}

this.validateRecoveryParams(params);
const isUnsignedSweep = params.isUnsignedSweep ?? getIsUnsignedSweep(params);

// Clean up whitespace from entered values
let userKey = params.userKey.replace(/\s/g, '');
const backupKey = params.backupKey.replace(/\s/g, '');
const recoveryKeys = classifyRecoveryKeys(params);
const { backupKey, isUnsignedSweep } = recoveryKeys;
let userKey = recoveryKeys.userKey;
const gasLimit = new optionalDeps.ethUtil.BN(this.setGasLimit(params.gasLimit));
const gasPrice = params.eip1559
? new optionalDeps.ethUtil.BN(params.eip1559.maxFeePerGas)
Expand Down Expand Up @@ -1898,7 +1918,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
this.validateEvmBasedRecoveryParams(params);

// Clean up whitespace from entered values
const userKey = params.userKey.replace(/\s/g, '');
const userKey = (params.userKey ?? '').replace(/\s/g, '');
const bitgoFeeAddress = params.bitgoFeeAddress?.replace(/\s/g, '').toLowerCase() as string;
const bitgoDestinationAddress = params.bitgoDestinationAddress?.replace(/\s/g, '').toLowerCase() as string;
const recoveryDestination = params.recoveryDestination?.replace(/\s/g, '').toLowerCase() as string;
Expand Down Expand Up @@ -2396,8 +2416,8 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
): Promise<RecoveryInfo | OfflineVaultTxInfo | UnsignedSweepTxMPCv2> {
this.validateRecoveryParams(params);
// Clean up whitespace from entered values
const userPublicOrPrivateKeyShare = params.userKey.replace(/\s/g, '');
const backupPrivateOrPublicKeyShare = params.backupKey.replace(/\s/g, '');
const userPublicOrPrivateKeyShare = (params.userKey ?? '').replace(/\s/g, '');
const backupPrivateOrPublicKeyShare = (params.backupKey ?? '').replace(/\s/g, '');

if (
getIsUnsignedSweep({
Expand All @@ -2421,11 +2441,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
const derivedCommonKeyChain = MPC.deriveUnhardened(commonKeyChain, 'm/0');
const backupKeyPair = new KeyPairLib({ pub: derivedCommonKeyChain.slice(0, 66) });
const baseAddress = backupKeyPair.getAddress();
if (baseAddress.toLowerCase() !== params.walletContractAddress.toLowerCase()) {
throw new RecoveryKeyMismatchError(
`derived wallet base address ${baseAddress} does not match the given wallet contract address ${params.walletContractAddress}`
);
}
this.assertTssBaseAddressMatchesWallet(baseAddress, params);
const unsignedTx = (await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params)).tx;
const messageHash = unsignedTx.getMessageToSign(true);
const signature = await ECDSAUtils.signRecoveryMpcV2(messageHash, userKeyShare, backupKeyShare, commonKeyChain);
Expand All @@ -2447,20 +2463,56 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
return { gasLimit, gasPrice };
}

/**
* Resolves the TSS common keychain for a recovery. Legacy callers put it in `backupKey` (and
* `userKey`); Safe unsigned sweeps supply it as `bitgoKey` alone.
* @param {RecoverOptions} params
* @returns {string} the common keychain, stripped of whitespace
*/
protected getTssCommonKeychain(params: { userKey?: string; backupKey?: string; bitgoKey?: string }): string {
return (params.backupKey || params.userKey || params.bitgoKey || '').replace(/\s/g, '');
}

/**
* Fails closed when the address derived from the TSS common keychain is not the wallet being
* recovered, before any balance or fee query is made.
* @param {string} baseAddress - address derived from the common keychain
* @param {RecoverOptions} params
*/
protected assertTssBaseAddressMatchesWallet(baseAddress: string, params: RecoverOptions): void {
// This guard is the check itself: without an address to compare against there is nothing
// stopping a wrong keychain, so refuse rather than skip.
if (!params.walletContractAddress) {
throw new Error('invalid walletContractAddress');
}
if (baseAddress.toLowerCase() !== params.walletContractAddress.toLowerCase()) {
throw new RecoveryKeyMismatchError(
`derived wallet base address ${baseAddress} does not match the given wallet contract address ${params.walletContractAddress}`
);
}
}

protected async buildUnsignedSweepTxnTSS(params: RecoverOptions): Promise<OfflineVaultTxInfo | UnsignedSweepTxMPCv2> {
const userPublicOrPrivateKeyShare = params.userKey.replace(/\s/g, '');
const backupPrivateOrPublicKeyShare = params.backupKey.replace(/\s/g, '');
// MPCv2 coins derive the base address from the common keychain. Not every MPCv2 coin overrides
// this method, so dispatch on the coin feature here rather than relying on each subclass.
if (this.staticsCoin?.features.includes(CoinFeature.MPCV2)) {
return this.buildUnsignedSweepTxnMPCv2(params);
}

const commonKeyChain = this.getTssCommonKeychain(params);
const userPublicOrPrivateKeyShare = (params.userKey ?? '').replace(/\s/g, '') || commonKeyChain;

const { gasLimit, gasPrice } = await this.getGasValues(params);

const backupKeyPair = new KeyPairLib({ pub: backupPrivateOrPublicKeyShare });
const backupKeyPair = new KeyPairLib({ pub: commonKeyChain });
const baseAddress = backupKeyPair.getAddress();
this.assertTssBaseAddressMatchesWallet(baseAddress, params);
const { txInfo, tx, nonce } = await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params);
return this.formatForOfflineVaultTSS(
txInfo,
tx,
userPublicOrPrivateKeyShare,
backupPrivateOrPublicKeyShare,
commonKeyChain,
gasPrice,
gasLimit,
nonce,
Expand All @@ -2473,13 +2525,20 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
const { gasLimit, gasPrice } = await this.getGasValues(params);

const recoverParams = params as RecoverOptions;
this.validateUnsignedSweepTSSParams(recoverParams);
await this.validateUnsignedSweepTSSParams(recoverParams);

const commonKeyChain = this.getTssCommonKeychain(recoverParams);
const derivationPath = recoverParams.derivationSeed ? getDerivationPath(recoverParams.derivationSeed) : 'm/0';
const MPC = new Ecdsa();
const derivedCommonKeyChain = MPC.deriveUnhardened(recoverParams.backupKey as string, derivationPath);
const derivedCommonKeyChain = MPC.deriveUnhardened(commonKeyChain, derivationPath);
const backupKeyPair = new KeyPairLib({ pub: derivedCommonKeyChain.slice(0, 66) });
const baseAddress = backupKeyPair.getAddress();
// Only the m/0 base address is one the caller can know in advance and pass as
// walletContractAddress. A derivationSeed deliberately sweeps a pseudorandom child
// (m/999999/x/y), so there is nothing meaningful to compare it against.
if (!recoverParams.derivationSeed) {
this.assertTssBaseAddressMatchesWallet(baseAddress, params);
}
const { txInfo, tx, nonce } = await this.buildTssRecoveryTxn(baseAddress, gasPrice, gasLimit, params);
return this.buildTxRequestForOfflineVaultMPCv2(
txInfo,
Expand All @@ -2490,7 +2549,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
gasLimit,
params.eip1559,
params.replayProtectionOptions,
recoverParams.backupKey as string
commonKeyChain
);
}

Expand Down Expand Up @@ -2562,7 +2621,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem
* @returns {void}
*/
private async validateUnsignedSweepTSSParams(params: RecoverOptions): Promise<void> {
if (_.isUndefined(params.backupKey) && params.backupKey === '') {
if (this.getTssCommonKeychain(params) === '') {
throw new Error('missing commonKeyChain');
}
if (!_.isUndefined(params.derivationSeed) && typeof params.derivationSeed !== 'string') {
Expand Down
10 changes: 4 additions & 6 deletions modules/abstract-eth/src/ethLikeToken.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import { bip32 } from '@bitgo/secp256k1';
import { BigNumber } from 'bignumber.js';

import {
classifyRecoveryKeys,
BitGoBase,
CoinConstructor,
NamedCoinConstructor,
ErrorNoInputToRecover,
RecoveryProviderError,
getIsUnsignedSweep,
Util,
} from '@bitgo/sdk-core';
import {
Expand Down Expand Up @@ -213,11 +213,9 @@ export class EthLikeToken extends AbstractEthLikeNewCoins {
throw new Error('invalid recoveryDestination');
}

const isUnsignedSweep = params.isUnsignedSweep ?? getIsUnsignedSweep(params);

// Clean up whitespace from entered values
let userKey = params.userKey.replace(/\s/g, '');
const backupKey = params.backupKey.replace(/\s/g, '');
const recoveryKeys = classifyRecoveryKeys(params);
const { backupKey, isUnsignedSweep } = recoveryKeys;
let userKey = recoveryKeys.userKey;

const gasLimit = new optionalDeps.ethUtil.BN(this.setGasLimit(params.gasLimit));
const gasPrice = params.eip1559
Expand Down
Loading
Loading