Skip to content

fix(mbe): fix validation params for recovery #95

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
pranavjain97 merged 2 commits into from
Jul 30, 2025
Merged

fix(mbe): fix validation params for recovery #95

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
a98af04
fix(mbe): fix validation params order for utxo
pranavjain97 Jul 29, 2025
d316e35
chore(mbe): check for required params instead of other coin params
pranavjain97 Jul 30, 2025
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
23 changes: 15 additions & 8 deletions src/__tests__/api/master/recoveryWallet.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ describe('Recovery Tests', () => {
response.status.should.equal(422);
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql('Invalid parameters provided for UTXO coin recovery');
response.body.details.should.containEql(
'UTXO recovery options are required for UTXO coin recovery',
);
});

it('should reject incorrect Solana parameters for a UTXO coin', async () => {
Expand Down Expand Up @@ -267,7 +269,9 @@ describe('Recovery Tests', () => {
response.status.should.equal(422);
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql('Invalid parameters provided for UTXO coin recovery');
response.body.details.should.containEql(
'UTXO recovery options are required for UTXO coin recovery',
);
});

it('should reject using legacy coinSpecificParams format', async () => {
Expand All @@ -294,9 +298,12 @@ describe('Recovery Tests', () => {
},
});

response.status.should.equal(500);
// Since we test that the incorrect format doesn't work anymore
response.status.should.equal(422);
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql(
'UTXO recovery options are required for UTXO coin recovery',
);
});
});

Expand Down Expand Up @@ -355,7 +362,7 @@ describe('Recovery Tests', () => {
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql(
'Invalid parameters provided for ETH-like coin recovery',
'EVM recovery options are required for ETH-like coin recovery',
);
});

Expand Down Expand Up @@ -393,7 +400,7 @@ describe('Recovery Tests', () => {
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql(
'Invalid parameters provided for ETH-like coin recovery',
'EVM recovery options are required for ETH-like coin recovery',
);
});
});
Expand Down Expand Up @@ -448,7 +455,7 @@ describe('Recovery Tests', () => {
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql(
'Invalid parameters provided for Solana coin recovery',
'Solana recovery options are required for EdDSA coin recovery',
);
});

Expand Down Expand Up @@ -479,7 +486,7 @@ describe('Recovery Tests', () => {
response.body.should.have.property('error');
response.body.should.have.property('details');
response.body.details.should.containEql(
'Invalid parameters provided for Solana coin recovery',
'Solana recovery options are required for EdDSA coin recovery',
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/api/master/recoveryWalletMpcV2.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ describe('MBE mpcv2 recovery', () => {
response.status.should.equal(422);
response.body.should.have.property('error');
response.body.error.should.equal(
'Invalid parameters provided for ETH-like MPC V2 coin recovery',
'ECDSA ETH-like recovery specific parameters are required for MPC recovery',
);
});
});
48 changes: 32 additions & 16 deletions src/api/master/handlers/recoveryWallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,41 +53,57 @@ interface EnclavedRecoveryParams {
walletContractAddress: string;
}

function validateRecoveryParams(sdkCoin: BaseCoin, params?: CoinSpecificParams) {
function validateRecoveryParams(
sdkCoin: BaseCoin,
params?: CoinSpecificParams,
isMpcRecovery = false,
) {
if (!params) {
return;
}

if (isUtxoCoin(sdkCoin)) {
Copy link
Copy Markdown
Contributor

@alextse-bg alextse-bg Jul 30, 2025

Choose a reason for hiding this comment

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

can utxo ever be mpc?

// UTXO coins need utxoRecoveryOptions for standard recovery
if (!isMpcRecovery && !params.utxoRecoveryOptions) {
throw new ValidationError('UTXO recovery options are required for UTXO coin recovery');
}
return;
}

if (isEddsaCoin(sdkCoin)) {
if (params.evmRecoveryOptions || params.utxoRecoveryOptions) {
throw new ValidationError('Invalid parameters provided for Solana coin recovery');
// EdDSA coins (like Solana) need solanaRecoveryOptions for standard recovery
if (!params.solanaRecoveryOptions) {
throw new ValidationError('Solana recovery options are required for EdDSA coin recovery');
}
} else if (isEcdsaCoin(sdkCoin)) {
return;
}
Comment on lines +75 to +79
Copy link
Copy Markdown
Contributor

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo Jul 30, 2025

Choose a reason for hiding this comment

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

I don't think this is correct, you can broadcast a sol recovery transaction without a durable nonce.


if (isEcdsaCoin(sdkCoin) && isMpcRecovery) {
if (isEthLikeCoin(sdkCoin)) {
if (!params.ecdsaEthLikeRecoverySpecificParams) {
throw new ValidationError('Invalid parameters provided for ETH-like MPC V2 coin recovery');
throw new ValidationError(
'ECDSA ETH-like recovery specific parameters are required for MPC recovery',
);
}
} else if (isCosmosLikeCoin(sdkCoin)) {
// ECDSA Cosmos-like MPC recovery needs ecdsaCosmosLikeRecoverySpecificParams
if (!params.ecdsaCosmosLikeRecoverySpecificParams) {
throw new ValidationError(
'Invalid parameters provided for Cosmos-like MPC V2 coin recovery',
'ECDSA Cosmos-like recovery specific parameters are required for MPC recovery',
);
}
} else {
throw new NotImplementedError(
`MPC V2 recovery is not supported for coin family: ${sdkCoin.getFamily()}`,
);
}
} else {
if (isUtxoCoin(sdkCoin)) {
if (params.solanaRecoveryOptions || params.evmRecoveryOptions) {
throw new ValidationError('Invalid parameters provided for UTXO coin recovery');
}
} else if (isEthLikeCoin(sdkCoin)) {
if (params.solanaRecoveryOptions || params.utxoRecoveryOptions) {
throw new ValidationError('Invalid parameters provided for ETH-like coin recovery');
}
}
if (!isMpcRecovery && isEthLikeCoin(sdkCoin)) {
// Non-ECDSA ETH-like coins need evmRecoveryOptions for standard recovery
if (!params.evmRecoveryOptions) {
throw new ValidationError('EVM recovery options are required for ETH-like coin recovery');
Comment on lines +101 to +104
Copy link
Copy Markdown
Contributor

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo Jul 30, 2025

Choose a reason for hiding this comment

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

evmRecoveryOptions this should be optional no?

}
return;
}
}

Expand Down Expand Up @@ -216,7 +232,7 @@ export async function handleRecoveryWalletOnPrem(

const sdkCoin = await coinFactory.getCoin(coin, bitgo);
// Validate that we have correct parameters for recovery
validateRecoveryParams(sdkCoin, coinSpecificParams);
validateRecoveryParams(sdkCoin, coinSpecificParams, req.decoded.isTssRecovery);

// Handle TSS recovery
if (req.decoded.isTssRecovery) {
Expand Down