Skip to content

feat(sdk-core): EVM keyring wallet and address creation changes #6872

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

Merged
merged 1 commit into from
Sep 3, 2025
Merged

feat(sdk-core): EVM keyring wallet and address creation changes #6872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ export interface SupplementGenerateWalletOptions {
type: 'hot' | 'cold' | 'custodial';
subType?: 'lightningCustody' | 'lightningSelfCustody' | 'onPrem';
coinSpecific?: { [coinName: string]: unknown };
referenceWalletId?: string;
}

export interface FeeEstimateOptions {
Expand Down
2 changes: 2 additions & 0 deletions modules/sdk-core/src/bitgo/wallet/iWallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,8 @@ export interface CreateAddressOptions {
derivedAddress?: string;
index?: number;
onToken?: string;
referenceCoin?: string;
referenceAddress?: string;
}

export interface UpdateAddressOptions {
Expand Down
2 changes: 2 additions & 0 deletions modules/sdk-core/src/bitgo/wallet/iWallets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export interface GenerateWalletOptions {
commonKeychain?: string;
type?: 'hot' | 'cold' | 'custodial';
subType?: 'lightningCustody' | 'lightningSelfCustody';
referenceWalletId?: string;
}

export const GenerateLightningWalletOptionsCodec = t.strict(
Expand Down Expand Up @@ -170,6 +171,7 @@ export interface AddWalletOptions {
initializationTxs?: any;
disableTransactionNotifications?: boolean;
gasPrice?: number;
referenceWalletId?: string;
}

type KeySignatures = {
Expand Down
26 changes: 26 additions & 0 deletions modules/sdk-core/src/bitgo/wallet/wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,8 @@ export class Wallet implements IWallet {
baseAddress,
allowSkipVerifyAddress = true,
onToken,
referenceCoin,
referenceAddress,
} = params;

if (!_.isUndefined(chain)) {
Expand Down Expand Up @@ -1325,6 +1327,30 @@ export class Wallet implements IWallet {
}
}

// Validate EVM keyring params, referenceAddress is required and referenceCoin is optional for EVM keyring
Copy link
Contributor

@pranavjain97 pranavjain97 Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if referenceCoin by itself isn't valid, it should't be an independent option. Let's instead have referenceAddressEvmKeyRing and have the address and coin as nested fields.

if (!_.isUndefined(referenceAddress)) {
if (!_.isString(referenceAddress)) {
throw new Error('referenceAddress has to be a string');
}
if (!this.baseCoin.isEVM()) {
throw new Error('referenceAddress is only supported for EVM chains');
}
if (!this.baseCoin.isValidAddress(referenceAddress)) {
throw new Error('referenceAddress must be a valid address');
}
addressParams.referenceAddress = referenceAddress;

if (!_.isUndefined(referenceCoin)) {
if (!_.isString(referenceCoin)) {
throw new Error('referenceCoin has to be a string');
}
addressParams.referenceCoin = referenceCoin;
}
} else if (!_.isUndefined(referenceCoin)) {
// referenceCoin cannot be used without referenceAddress
throw new Error('referenceAddress is required when using referenceCoin for EVM keyring');
}

Comment on lines +1349 to +1353
Copy link
Contributor

@pranavjain97 pranavjain97 Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you instead do a if(_.isUndefined(referenceCoin) && !_.isUndefined(referenceAddress)) { throw error }
That's more readable.

Copy link
Contributor Author

@nvjsr nvjsr Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding changes in followup PR for same

// get keychains for address verification
const keychains = await Promise.all(this._wallet.keys.map((k) => this.baseCoin.keychains().get({ id: k, reqId })));
const rootAddress = _.get(this._wallet, 'receiveAddress.address');
Expand Down
69 changes: 66 additions & 3 deletions modules/sdk-core/src/bitgo/wallet/wallets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,21 @@ export class Wallets implements IWallets {
throw new Error('missing required string parameter label');
}

// no need to pass keys for (single) custodial wallets
if (params.type !== 'custodial') {
// Validate referenceWalletId parameter
if (params.referenceWalletId) {
if (!_.isString(params.referenceWalletId)) {
throw new Error('invalid referenceWalletId argument, expecting string');
}
if (!this.baseCoin.isEVM()) {
throw new Error('referenceWalletId is only supported for EVM chains');
}
}

// For wallets with referenceWalletId, skip multisig validation as configuration is inherited
if (params.referenceWalletId) {
// Skip all multisig validation - configuration will be inherited from reference wallet
} else if (params.type !== 'custodial') {
// Standard validation for non-custodial wallets without referenceWalletId
Comment on lines +104 to +118
Copy link
Contributor

@pranavjain97 pranavjain97 Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shoudn't be having a referenceId as a top level param that is EVM based only. Can we atleast name it evmKeyRingReferenceWalletId? Coin-specific logic shoudn't be leaking in generic add wallet methods, can we create a coin-specific util in eth-lib that can take care of this validation?

if (Array.isArray(params.keys) === false || !_.isNumber(params.m) || !_.isNumber(params.n)) {
throw new Error('invalid argument');
}
Expand Down Expand Up @@ -272,9 +285,10 @@ export class Wallets implements IWallets {
throw new Error('missing required string parameter label');
}

const { type = 'hot', label, passphrase, enterprise, isDistributedCustody } = params;
const { type = 'hot', label, passphrase, enterprise, isDistributedCustody, referenceWalletId } = params;
const isTss = params.multisigType === 'tss' && this.baseCoin.supportsTss();
const canEncrypt = !!passphrase && typeof passphrase === 'string';
const isEVMWithReference = this.baseCoin.isEVM() && referenceWalletId;

const walletParams: SupplementGenerateWalletOptions = {
label: label,
Expand All @@ -284,6 +298,11 @@ export class Wallets implements IWallets {
type: !!params.userKey && params.multisigType !== 'onchain' ? 'cold' : type,
};

// Add referenceWalletId to walletParams if provided for EVM chains
if (isEVMWithReference) {
walletParams.referenceWalletId = referenceWalletId;
}

if (!_.isUndefined(params.passcodeEncryptionCode)) {
if (!_.isString(params.passcodeEncryptionCode)) {
throw new Error('passcodeEncryptionCode must be a string');
Expand All @@ -297,15 +316,59 @@ export class Wallets implements IWallets {
walletParams.enterprise = enterprise;
}

// Validate referenceWalletId for EVM keyring
if (!_.isUndefined(referenceWalletId)) {
if (!_.isString(referenceWalletId)) {
throw new Error('invalid referenceWalletId argument, expecting string');
}
if (!this.baseCoin.isEVM()) {
throw new Error('referenceWalletId is only supported for EVM chains');
}
}

// EVM TSS wallets must use wallet version 3, 5 and 6
// Skip this validation for EVM keyring wallets as they inherit version from reference wallet
if (
isTss &&
this.baseCoin.isEVM() &&
!referenceWalletId &&
!(params.walletVersion === 3 || params.walletVersion === 5 || params.walletVersion === 6)
) {
throw new Error('EVM TSS wallets are only supported for wallet version 3, 5 and 6');
}

// Handle EVM keyring wallet creation with referenceWalletId
if (isEVMWithReference) {
// For EVM keyring wallets, multisigType will be inferred from the reference wallet
// No need to explicitly validate TSS requirement here as it will be handled by bgms

// For EVM keyring wallets, we use the add method directly with referenceWalletId
// This bypasses the normal key generation process since keys are shared via keyring
const addWalletParams = {
label,
referenceWalletId,
};

const newWallet = await this.bitgo.post(this.baseCoin.url('/wallet/add')).send(addWalletParams).result();

// For EVM keyring wallets, we need to get the keychains from the reference wallet
const userKeychain = this.baseCoin.keychains().get({ id: newWallet.keys[KeyIndices.USER] });
const backupKeychain = this.baseCoin.keychains().get({ id: newWallet.keys[KeyIndices.BACKUP] });
const bitgoKeychain = this.baseCoin.keychains().get({ id: newWallet.keys[KeyIndices.BITGO] });

const [userKey, backupKey, bitgoKey] = await Promise.all([userKeychain, backupKeychain, bitgoKeychain]);

const result: WalletWithKeychains = {
wallet: new Wallet(this.bitgo, this.baseCoin, newWallet),
userKeychain: userKey,
backupKeychain: backupKey,
bitgoKeychain: bitgoKey,
responseType: 'WalletWithKeychains',
};

return result;
}
Comment on lines +341 to +370
Copy link
Contributor

@pranavjain97 pranavjain97 Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be an EVM lib util i.e something like createEvmKeyRingWallet.


if (isTss) {
if (!this.baseCoin.supportsTss()) {
throw new Error(`coin ${this.baseCoin.getFamily()} does not support TSS at this time`);
Expand Down
3 changes: 2 additions & 1 deletion modules/sdk-core/test/unit/account-lib/mpc/tss/ecdsa/ecdsa.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import { loadWebAssembly } from '@bitgo/sdk-opensslbytes';

const openSSLBytes = loadWebAssembly().buffer;

describe('ecdsa tss', function () {
describe('ecdsa tss', function (this: Mocha.Context) {
this.timeout(60000);
const ecdsa = new Ecdsa();

let signCombine1: SignCombineRT, signCombine2: SignCombineRT;
Expand Down
Loading