Skip to content

Commit fa193ec

Browse files
committed
fix(sdk-core): restore MPCv2 message /send finalization
PR #10009 replaced the MPCv2 ECDSA message finalization POST /send with a plain GET, assuming wallet-platform always combines and delivers messages inside the final share submission. That assumption only holds for custodial wallets (walletHasCustodialUserKey gate in wallet-platform's share ingestion): for self-custodial wallets the request stays pendingDelivery/readyToCombineShares until a client POSTs /send, which is what triggers the share combine, the delivered-state transition and the signed-message Kafka publish. With the GET-only tail, the post-sign re-fetch returned an un-combined request without txHash and tripped the "Unable to find txHash" assert in signAndSendMessageTxRequest, leaving the message permanently un-combined and the dApp un-notified. Finalize messages by re-fetching first (custodial requests are already terminal and must not POST /send, which wallet-platform rejects), then POST /send to trigger the combine for self-custodial requests. If /send is rejected because the request became terminal concurrently, fall back to one more GET instead of surfacing the error.
1 parent c6a0313 commit fa193ec

2 files changed

Lines changed: 149 additions & 37 deletions

File tree

‎modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,15 +1146,34 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
11461146
params.reqId
11471147
);
11481148

1149-
// Messages are combined and marked delivered by wallet-platform inside
1150-
// the final share submission itself (the signed-message Kafka event is
1151-
// published there too): by the time this call returns, a message request
1152-
// is already terminal. POSTing /send afterwards — as the transaction
1153-
// flow does — would be rejected by wallet-platform's pendingDelivery
1154-
// assert on the already-delivered request. Re-fetch the request instead,
1155-
// mirroring the legacy MPCv1 ECDSA message path.
1149+
// Message finalization differs by custody model:
1150+
// - Custodial wallets: wallet-platform combines the shares and marks the
1151+
// message delivered inside the final share submission itself (the
1152+
// signed-message Kafka event is published there too), so the request is
1153+
// already terminal by the time this call returns. A follow-up POST /send
1154+
// would be rejected by wallet-platform's pendingDelivery assert on the
1155+
// already-delivered request — re-fetch and return instead.
1156+
// - Self-custodial wallets: nothing combines the shares at ingestion;
1157+
// the request stays pendingDelivery/readyToCombineShares until the
1158+
// client POSTs /send, which triggers the combine, the delivered-state
1159+
// transition and the signed-message Kafka publish.
11561160
if (requestType === RequestType.message) {
1157-
return getTxRequest(this.bitgo, txRequest.walletId, txRequest.txRequestId, params.reqId);
1161+
const fetched = await getTxRequest(this.bitgo, txRequest.walletId, txRequest.txRequestId, params.reqId);
1162+
if (fetched.state === 'delivered' || fetched.messages?.[0]?.txHash) {
1163+
return fetched;
1164+
}
1165+
try {
1166+
return await sendTxRequest(this.bitgo, txRequest.walletId, txRequest.txRequestId, requestType, params.reqId);
1167+
} catch (err) {
1168+
// The combine/delivery may have completed concurrently (e.g. a
1169+
// custodial ingestion commit landing between the GET and the POST).
1170+
// If the request is now terminal, surface it instead of the error.
1171+
const refetched = await getTxRequest(this.bitgo, txRequest.walletId, txRequest.txRequestId, params.reqId);
1172+
if (refetched.state === 'delivered' || refetched.messages?.[0]?.txHash) {
1173+
return refetched;
1174+
}
1175+
throw err;
1176+
}
11581177
}
11591178
return sendTxRequest(this.bitgo, txRequest.walletId, txRequest.txRequestId, requestType, params.reqId);
11601179
}

‎modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts‎

Lines changed: 122 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1414,8 +1414,22 @@ describe('signRequestBase message finalization', () => {
14141414
* using the signBitgoMPCv2RoundX helpers. Only the HTTP transport is faked
14151415
* (bitgo.get / bitgo.post chains), which is exactly the surface the
14161416
* finalization-tail decision acts on: GET txrequests vs POST /send.
1417+
*
1418+
* deliveredAtIngestion models the custodial flow (wallet-platform combines
1419+
* and delivers the message inside share ingestion); when false, the first
1420+
* GET still shows the un-combined pendingDelivery/readyToCombineShares
1421+
* request, as it does for self-custodial wallets. sendFails makes the
1422+
* POST /send reject; neverDelivers keeps every GET un-combined to assert
1423+
* genuine send errors still surface.
14171424
*/
1418-
const runFullSigningFlow = async (params: { requestType: 'tx' | 'message'; txRequest: TxRequest }) => {
1425+
const runFullSigningFlow = async (params: {
1426+
requestType: 'tx' | 'message';
1427+
txRequest: TxRequest;
1428+
deliveredAtIngestion?: boolean;
1429+
sendFails?: boolean;
1430+
neverDelivers?: boolean;
1431+
}) => {
1432+
const { deliveredAtIngestion = true, sendFails = false, neverDelivers = false } = params;
14191433
const postedUrls: string[] = [];
14201434
const getUrls: string[] = [];
14211435
const hashBuffer = Buffer.alloc(32, 1);
@@ -1430,6 +1444,22 @@ describe('signRequestBase message finalization', () => {
14301444
const carrier = { transactions: [{ signatureShares: [] }] } as unknown as TxRequest;
14311445
let signPostCount = 0;
14321446

1447+
const deliveredResponse = {
1448+
...params.txRequest,
1449+
state: 'delivered',
1450+
[params.requestType === 'message' ? 'messages' : 'transactions']: [
1451+
{
1452+
state: 'signed',
1453+
txHash: '0xsigned-message-hash',
1454+
signatureShares: [],
1455+
...(params.requestType === 'tx'
1456+
? { unsignedTx: { derivationPath, signableHex: 'abcd', serializedTxHex: 'abcd' } }
1457+
: {}),
1458+
},
1459+
],
1460+
} as unknown as TxRequest;
1461+
1462+
let getCallCount = 0;
14331463
const mockBg = {
14341464
getEnv: () => 'test',
14351465
setRequestTracer: () => undefined,
@@ -1443,24 +1473,31 @@ describe('signRequestBase message finalization', () => {
14431473
},
14441474
async result() {
14451475
getUrls.push(url);
1446-
return {
1447-
txRequests: [
1448-
{
1449-
...params.txRequest,
1450-
state: 'delivered',
1451-
[params.requestType === 'message' ? 'messages' : 'transactions']: [
1452-
{
1453-
state: 'signed',
1454-
txHash: '0xsigned-message-hash',
1455-
signatureShares: [],
1456-
...(params.requestType === 'tx'
1457-
? { unsignedTx: { derivationPath, signableHex: 'abcd', serializedTxHex: 'abcd' } }
1458-
: {}),
1459-
},
1460-
],
1461-
},
1462-
],
1463-
};
1476+
getCallCount += 1;
1477+
// First fetch after the final share for a self-custodial wallet:
1478+
// nothing has combined the shares yet.
1479+
if (
1480+
params.requestType === 'message' &&
1481+
(!deliveredAtIngestion || neverDelivers) &&
1482+
(getCallCount === 1 || neverDelivers)
1483+
) {
1484+
return {
1485+
txRequests: [
1486+
{
1487+
...params.txRequest,
1488+
state: 'pendingDelivery',
1489+
messages: [
1490+
{
1491+
...(params.txRequest.messages![0] as Record<string, unknown>),
1492+
state: 'readyToCombineShares',
1493+
signatureShares: [],
1494+
},
1495+
],
1496+
},
1497+
],
1498+
};
1499+
}
1500+
return { txRequests: [deliveredResponse] };
14641501
},
14651502
}),
14661503
post: (url: string) => {
@@ -1473,7 +1510,12 @@ describe('signRequestBase message finalization', () => {
14731510
},
14741511
async result() {
14751512
if (!url.endsWith('/sign')) {
1476-
return { ...params.txRequest, state: 'delivered' };
1513+
if (sendFails) {
1514+
throw new Error(
1515+
'Expected transaction request to be in state pendingDelivery but it is in state delivered.'
1516+
);
1517+
}
1518+
return deliveredResponse;
14771519
}
14781520
signPostCount += 1;
14791521
const body = this._body as { signatureShares: SignatureShareRecord[]; signerGpgPublicKey: string };
@@ -1532,24 +1574,27 @@ describe('signRequestBase message finalization', () => {
15321574
return { result, postedUrls, getUrls };
15331575
};
15341576

1535-
it('message requests finalize via GET (wallet-platform delivers at share ingestion), never POST /send', async () => {
1577+
const messageTxRequest = (txRequestId: string) =>
1578+
({
1579+
txRequestId,
1580+
walletId: walletID,
1581+
apiVersion: 'full',
1582+
state: 'pendingDelivery',
1583+
messages: [{ messageRaw: 'raw', messageEncoded: 'abcd', derivationPath: 'm/0' }],
1584+
} as unknown as TxRequest);
1585+
1586+
it('message requests delivered at share ingestion (custodial) finalize via GET without POST /send', async () => {
15361587
const txRequestId = 'msg-txreq-finalization';
15371588
const { result, postedUrls, getUrls } = await runFullSigningFlow({
15381589
requestType: 'message',
1539-
txRequest: {
1540-
txRequestId,
1541-
walletId: walletID,
1542-
apiVersion: 'full',
1543-
state: 'pendingDelivery',
1544-
messages: [{ messageRaw: 'raw', messageEncoded: 'abcd', derivationPath: 'm/0' }],
1545-
} as unknown as TxRequest,
1590+
txRequest: messageTxRequest(txRequestId),
15461591
});
15471592

15481593
assert.strictEqual(result.state, 'delivered');
15491594
assert.strictEqual(
15501595
postedUrls.filter((url) => url.endsWith('/send')).length,
15511596
0,
1552-
'message signing must not POST /send (wallet-platform already delivered at share ingestion)'
1597+
'an already-delivered message must not POST /send (wallet-platform rejects it)'
15531598
);
15541599
assert.ok(
15551600
postedUrls.every((url) => url.includes(`/txrequests/${txRequestId}/messages/0/sign`)),
@@ -1561,6 +1606,54 @@ describe('signRequestBase message finalization', () => {
15611606
assert.ok(getUrls[0].includes('/txrequests'));
15621607
});
15631608

1609+
it('message requests left un-combined at ingestion (self-custodial) finalize via POST /send', async () => {
1610+
const txRequestId = 'msg-txreq-self-custodial';
1611+
const { result, postedUrls, getUrls } = await runFullSigningFlow({
1612+
requestType: 'message',
1613+
txRequest: messageTxRequest(txRequestId),
1614+
deliveredAtIngestion: false,
1615+
});
1616+
1617+
assert.strictEqual(result.state, 'delivered');
1618+
assert.strictEqual(
1619+
postedUrls.filter((url) => url.endsWith(`/txrequests/${txRequestId}/messages/0/send`)).length,
1620+
1,
1621+
'self-custodial message signing must POST /send to trigger the combine and delivery'
1622+
);
1623+
assert.strictEqual(getUrls.length, 1, 'the pre-send GET is the only fetch when /send succeeds');
1624+
});
1625+
1626+
it('a /send rejection falls back to a re-fetch when the message became terminal concurrently', async () => {
1627+
const txRequestId = 'msg-txreq-send-race';
1628+
const { result, postedUrls, getUrls } = await runFullSigningFlow({
1629+
requestType: 'message',
1630+
txRequest: messageTxRequest(txRequestId),
1631+
deliveredAtIngestion: false,
1632+
sendFails: true,
1633+
});
1634+
1635+
assert.strictEqual(result.state, 'delivered');
1636+
assert.strictEqual(
1637+
postedUrls.filter((url) => url.endsWith(`/txrequests/${txRequestId}/messages/0/send`)).length,
1638+
1,
1639+
'the /send attempt must have been made before the fallback fetch'
1640+
);
1641+
assert.strictEqual(getUrls.length, 2, 'the tail must re-fetch once after the /send rejection');
1642+
});
1643+
1644+
it('a /send rejection for a request that is still un-combined still surfaces the error', async () => {
1645+
await assert.rejects(
1646+
runFullSigningFlow({
1647+
requestType: 'message',
1648+
txRequest: messageTxRequest('msg-txreq-send-fails'),
1649+
deliveredAtIngestion: false,
1650+
sendFails: true,
1651+
neverDelivers: true,
1652+
}),
1653+
/pendingDelivery/
1654+
);
1655+
});
1656+
15641657
it('transaction requests still finalize via POST /send', async () => {
15651658
const txRequestId = 'tx-txreq-finalization';
15661659
const { result, postedUrls } = await runFullSigningFlow({

0 commit comments

Comments
 (0)