Skip to content

Commit ad6eef7

Browse files
DinshawKothariDinshawKothari
authored
Merge pull request #6078 from BitGo/WIN-5376
fix(sdk-coin-icp): update verifyTransaction to include signableHex parameter
2 parents 9ea2ff1 + 9153969 commit ad6eef7

File tree

5 files changed

+124
-16
lines changed

5 files changed

+124
-16
lines changed

‎modules/sdk-coin-icp/src/icp.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
Signatures,
3939
SigningPayload,
4040
IcpTransactionExplanation,
41+
TransactionHexParams,
4142
} from './lib/iface';
4243
import { TransactionBuilderFactory } from './lib/transactionBuilderFactory';
4344
import utils from './lib/utils';
@@ -85,10 +86,16 @@ export class Icp extends BaseCoin {
8586
return Math.pow(10, this._staticsCoin.decimalPlaces);
8687
}
8788

88-
async explainTransaction(params: { txHex: string }): Promise<IcpTransactionExplanation> {
89+
async explainTransaction(params: TransactionHexParams): Promise<IcpTransactionExplanation> {
8990
const factory = this.getBuilderFactory();
90-
const txBuilder = await factory.from(params.txHex);
91+
const txBuilder = await factory.from(params.transactionHex);
9192
const transaction = await txBuilder.build();
93+
if (params.signableHex !== undefined) {
94+
const generatedSignableHex = txBuilder.transaction.payloadsData.payloads[0].hex_bytes;
95+
if (generatedSignableHex !== params.signableHex) {
96+
throw new Error('generated signableHex is not equal to params.signableHex');
97+
}
98+
}
9299
return transaction.explainTransaction();
93100
}
94101

@@ -98,7 +105,15 @@ export class Icp extends BaseCoin {
98105
if (!txHex) {
99106
throw new Error('txHex is required');
100107
}
101-
const explainedTx = await this.explainTransaction({ txHex });
108+
const txHexParams: TransactionHexParams = {
109+
transactionHex: txHex,
110+
};
111+
112+
if (txPrebuild.txInfo && txPrebuild.txInfo !== undefined && typeof txPrebuild.txInfo === 'string') {
113+
txHexParams.signableHex = txPrebuild.txInfo;
114+
}
115+
116+
const explainedTx = await this.explainTransaction(txHexParams);
102117

103118
if (Array.isArray(txParams.recipients) && txParams.recipients.length > 0) {
104119
if (txParams.recipients.length > 1) {

‎modules/sdk-coin-icp/src/lib/iface.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,8 @@ export interface PublicNodeSubmitResponse {
192192
export interface AccountIdentifierHash {
193193
hash: Buffer<ArrayBuffer>;
194194
}
195+
196+
export interface TransactionHexParams {
197+
transactionHex: string;
198+
signableHex?: string;
199+
}

‎modules/sdk-coin-icp/test/unit/icp.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,15 @@ describe('Internet computer', function () {
147147
});
148148

149149
describe('Verify a transaction', () => {
150-
it('should successfully verify a transaction', async () => {
150+
it('should successfully verify a transaction with signable Hex', async () => {
151151
const unsignedTxn = txBuilder.transaction.unsignedTransaction;
152152
unsignedTxn.should.be.a.String();
153153
const payloadsData = txBuilder.transaction.payloadsData;
154154
const serializedTxFormat = {
155155
serializedTxHex: payloadsData,
156156
publicKey: testData.Accounts.account1.publicKey,
157157
};
158+
const signableHex = payloadsData.payloads[0].hex_bytes;
158159
const serializedTxHex = Buffer.from(JSON.stringify(serializedTxFormat), 'utf-8').toString('hex');
159160
const txParams = {
160161
recipients: [
@@ -167,10 +168,69 @@ describe('Internet computer', function () {
167168
const response = await basecoin.verifyTransaction({
168169
txPrebuild: {
169170
txHex: serializedTxHex,
171+
txInfo: signableHex,
170172
},
171173
txParams: txParams,
172174
});
173175
assert(response);
174176
});
177+
178+
it('should successfully verify a transaction without signable Hex', async () => {
179+
const unsignedTxn = txBuilder.transaction.unsignedTransaction;
180+
unsignedTxn.should.be.a.String();
181+
const payloadsData = txBuilder.transaction.payloadsData;
182+
const serializedTxFormat = {
183+
serializedTxHex: payloadsData,
184+
publicKey: testData.Accounts.account1.publicKey,
185+
};
186+
const serializedTxHex = Buffer.from(JSON.stringify(serializedTxFormat), 'utf-8').toString('hex');
187+
const txParams = {
188+
recipients: [
189+
{
190+
address: testData.Accounts.account2.address,
191+
amount: '10',
192+
},
193+
],
194+
};
195+
const response = await basecoin.verifyTransaction({
196+
txPrebuild: {
197+
txHex: serializedTxHex,
198+
},
199+
txParams: txParams,
200+
});
201+
assert(response);
202+
});
203+
204+
it('should fail to verify a transaction with wrong signable Hex', async () => {
205+
const unsignedTxn = txBuilder.transaction.unsignedTransaction;
206+
unsignedTxn.should.be.a.String();
207+
const payloadsData = txBuilder.transaction.payloadsData;
208+
const serializedTxFormat = {
209+
serializedTxHex: payloadsData,
210+
publicKey: testData.Accounts.account1.publicKey,
211+
};
212+
const serializedTxHex = Buffer.from(JSON.stringify(serializedTxFormat), 'utf-8').toString('hex');
213+
const txParams = {
214+
recipients: [
215+
{
216+
address: testData.Accounts.account2.address,
217+
amount: '10',
218+
},
219+
],
220+
};
221+
222+
const wrongSignableHexValues =
223+
'0a69632d72657175657374523de3c7c5b4613155b74ede2e54493f6acbe8bf6d910154fbbb3a98ba3e0098';
224+
225+
await basecoin
226+
.verifyTransaction({
227+
txPrebuild: {
228+
txHex: serializedTxHex,
229+
txInfo: wrongSignableHexValues,
230+
},
231+
txParams: txParams,
232+
})
233+
.should.rejectedWith('generated signableHex is not equal to params.signableHex');
234+
});
175235
});
176236
});

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -744,12 +744,26 @@ export class EcdsaUtils extends BaseEcdsaUtils {
744744
assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
745745
const unsignedTx =
746746
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
747-
await this.baseCoin.verifyTransaction({
748-
txPrebuild: { txHex: unsignedTx.signableHex },
749-
txParams: params.txParams || { recipients: [] },
750-
wallet: this.wallet,
751-
walletType: this.wallet.multisigType(),
752-
});
747+
748+
// For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately.
749+
// Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex
750+
// to regenerate the signableHex and compare it against the provided value for verification.
751+
// In contrast, for other coin families, verification is typically done using just the signableHex.
752+
if (this.baseCoin.getConfig().family === 'icp') {
753+
await this.baseCoin.verifyTransaction({
754+
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
755+
txParams: params.txParams || { recipients: [] },
756+
wallet: this.wallet,
757+
walletType: this.wallet.multisigType(),
758+
});
759+
} else {
760+
await this.baseCoin.verifyTransaction({
761+
txPrebuild: { txHex: unsignedTx.signableHex },
762+
txParams: params.txParams || { recipients: [] },
763+
wallet: this.wallet,
764+
walletType: this.wallet.multisigType(),
765+
});
766+
}
753767
signablePayload = Buffer.from(unsignedTx.signableHex, 'hex');
754768
derivationPath = unsignedTx.derivationPath;
755769
} else if (requestType === RequestType.message) {

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -731,12 +731,26 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
731731
assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
732732
const unsignedTx =
733733
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
734-
await this.baseCoin.verifyTransaction({
735-
txPrebuild: { txHex: unsignedTx.signableHex },
736-
txParams: params.txParams || { recipients: [] },
737-
wallet: this.wallet,
738-
walletType: this.wallet.multisigType(),
739-
});
734+
735+
// For ICP transactions, the HSM signs the serializedTxHex, while the user signs the signableHex separately.
736+
// Verification cannot be performed directly on the signableHex alone. However, we can parse the serializedTxHex
737+
// to regenerate the signableHex and compare it against the provided value for verification.
738+
// In contrast, for other coin families, verification is typically done using just the signableHex.
739+
if (this.baseCoin.getConfig().family === 'icp') {
740+
await this.baseCoin.verifyTransaction({
741+
txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex },
742+
txParams: params.txParams || { recipients: [] },
743+
wallet: this.wallet,
744+
walletType: this.wallet.multisigType(),
745+
});
746+
} else {
747+
await this.baseCoin.verifyTransaction({
748+
txPrebuild: { txHex: unsignedTx.signableHex },
749+
txParams: params.txParams || { recipients: [] },
750+
wallet: this.wallet,
751+
walletType: this.wallet.multisigType(),
752+
});
753+
}
740754
txOrMessageToSign = unsignedTx.signableHex;
741755
derivationPath = unsignedTx.derivationPath;
742756
bufferContent = Buffer.from(txOrMessageToSign, 'hex');

0 commit comments

Comments
 (0)