Skip to content

Commit b9a0d1d

Browse files
authored
Merge pull request #95 from BitGo/WP-5454-fix-btc-recovery-validation
fix(mbe): fix validation params for recovery
2 parents de5a850 + d316e35 commit b9a0d1d

3 files changed

Lines changed: 48 additions & 25 deletions

File tree

src/__tests__/api/master/recoveryWallet.test.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,9 @@ describe('Recovery Tests', () => {
232232
response.status.should.equal(422);
233233
response.body.should.have.property('error');
234234
response.body.should.have.property('details');
235-
response.body.details.should.containEql('Invalid parameters provided for UTXO coin recovery');
235+
response.body.details.should.containEql(
236+
'UTXO recovery options are required for UTXO coin recovery',
237+
);
236238
});
237239

238240
it('should reject incorrect Solana parameters for a UTXO coin', async () => {
@@ -267,7 +269,9 @@ describe('Recovery Tests', () => {
267269
response.status.should.equal(422);
268270
response.body.should.have.property('error');
269271
response.body.should.have.property('details');
270-
response.body.details.should.containEql('Invalid parameters provided for UTXO coin recovery');
272+
response.body.details.should.containEql(
273+
'UTXO recovery options are required for UTXO coin recovery',
274+
);
271275
});
272276

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

297-
response.status.should.equal(500);
298-
// Since we test that the incorrect format doesn't work anymore
301+
response.status.should.equal(422);
299302
response.body.should.have.property('error');
303+
response.body.should.have.property('details');
304+
response.body.details.should.containEql(
305+
'UTXO recovery options are required for UTXO coin recovery',
306+
);
300307
});
301308
});
302309

@@ -355,7 +362,7 @@ describe('Recovery Tests', () => {
355362
response.body.should.have.property('error');
356363
response.body.should.have.property('details');
357364
response.body.details.should.containEql(
358-
'Invalid parameters provided for ETH-like coin recovery',
365+
'EVM recovery options are required for ETH-like coin recovery',
359366
);
360367
});
361368

@@ -393,7 +400,7 @@ describe('Recovery Tests', () => {
393400
response.body.should.have.property('error');
394401
response.body.should.have.property('details');
395402
response.body.details.should.containEql(
396-
'Invalid parameters provided for ETH-like coin recovery',
403+
'EVM recovery options are required for ETH-like coin recovery',
397404
);
398405
});
399406
});
@@ -448,7 +455,7 @@ describe('Recovery Tests', () => {
448455
response.body.should.have.property('error');
449456
response.body.should.have.property('details');
450457
response.body.details.should.containEql(
451-
'Invalid parameters provided for Solana coin recovery',
458+
'Solana recovery options are required for EdDSA coin recovery',
452459
);
453460
});
454461

@@ -479,7 +486,7 @@ describe('Recovery Tests', () => {
479486
response.body.should.have.property('error');
480487
response.body.should.have.property('details');
481488
response.body.details.should.containEql(
482-
'Invalid parameters provided for Solana coin recovery',
489+
'Solana recovery options are required for EdDSA coin recovery',
483490
);
484491
});
485492
});

src/__tests__/api/master/recoveryWalletMpcV2.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ describe('MBE mpcv2 recovery', () => {
203203
response.status.should.equal(422);
204204
response.body.should.have.property('error');
205205
response.body.error.should.equal(
206-
'Invalid parameters provided for ETH-like MPC V2 coin recovery',
206+
'ECDSA ETH-like recovery specific parameters are required for MPC recovery',
207207
);
208208
});
209209
});

src/api/master/handlers/recoveryWallet.ts

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,41 +53,57 @@ interface EnclavedRecoveryParams {
5353
walletContractAddress: string;
5454
}
5555

56-
function validateRecoveryParams(sdkCoin: BaseCoin, params?: CoinSpecificParams) {
56+
function validateRecoveryParams(
57+
sdkCoin: BaseCoin,
58+
params?: CoinSpecificParams,
59+
isMpcRecovery = false,
60+
) {
5761
if (!params) {
5862
return;
5963
}
6064

65+
if (isUtxoCoin(sdkCoin)) {
66+
// UTXO coins need utxoRecoveryOptions for standard recovery
67+
if (!isMpcRecovery && !params.utxoRecoveryOptions) {
68+
throw new ValidationError('UTXO recovery options are required for UTXO coin recovery');
69+
}
70+
return;
71+
}
72+
6173
if (isEddsaCoin(sdkCoin)) {
62-
if (params.evmRecoveryOptions || params.utxoRecoveryOptions) {
63-
throw new ValidationError('Invalid parameters provided for Solana coin recovery');
74+
// EdDSA coins (like Solana) need solanaRecoveryOptions for standard recovery
75+
if (!params.solanaRecoveryOptions) {
76+
throw new ValidationError('Solana recovery options are required for EdDSA coin recovery');
6477
}
65-
} else if (isEcdsaCoin(sdkCoin)) {
78+
return;
79+
}
80+
81+
if (isEcdsaCoin(sdkCoin) && isMpcRecovery) {
6682
if (isEthLikeCoin(sdkCoin)) {
6783
if (!params.ecdsaEthLikeRecoverySpecificParams) {
68-
throw new ValidationError('Invalid parameters provided for ETH-like MPC V2 coin recovery');
84+
throw new ValidationError(
85+
'ECDSA ETH-like recovery specific parameters are required for MPC recovery',
86+
);
6987
}
7088
} else if (isCosmosLikeCoin(sdkCoin)) {
89+
// ECDSA Cosmos-like MPC recovery needs ecdsaCosmosLikeRecoverySpecificParams
7190
if (!params.ecdsaCosmosLikeRecoverySpecificParams) {
7291
throw new ValidationError(
73-
'Invalid parameters provided for Cosmos-like MPC V2 coin recovery',
92+
'ECDSA Cosmos-like recovery specific parameters are required for MPC recovery',
7493
);
7594
}
7695
} else {
7796
throw new NotImplementedError(
7897
`MPC V2 recovery is not supported for coin family: ${sdkCoin.getFamily()}`,
7998
);
8099
}
81-
} else {
82-
if (isUtxoCoin(sdkCoin)) {
83-
if (params.solanaRecoveryOptions || params.evmRecoveryOptions) {
84-
throw new ValidationError('Invalid parameters provided for UTXO coin recovery');
85-
}
86-
} else if (isEthLikeCoin(sdkCoin)) {
87-
if (params.solanaRecoveryOptions || params.utxoRecoveryOptions) {
88-
throw new ValidationError('Invalid parameters provided for ETH-like coin recovery');
89-
}
100+
}
101+
if (!isMpcRecovery && isEthLikeCoin(sdkCoin)) {
102+
// Non-ECDSA ETH-like coins need evmRecoveryOptions for standard recovery
103+
if (!params.evmRecoveryOptions) {
104+
throw new ValidationError('EVM recovery options are required for ETH-like coin recovery');
90105
}
106+
return;
91107
}
92108
}
93109

@@ -216,7 +232,7 @@ export async function handleRecoveryWalletOnPrem(
216232

217233
const sdkCoin = await coinFactory.getCoin(coin, bitgo);
218234
// Validate that we have correct parameters for recovery
219-
validateRecoveryParams(sdkCoin, coinSpecificParams);
235+
validateRecoveryParams(sdkCoin, coinSpecificParams, req.decoded.isTssRecovery);
220236

221237
// Handle TSS recovery
222238
if (req.decoded.isTssRecovery) {

0 commit comments

Comments
 (0)