From 89cbdbb5719a240dc6d876d7fe3973e30f5ef88a Mon Sep 17 00:00:00 2001 From: Kashif Jamil Date: Wed, 24 Dec 2025 19:59:36 +0530 Subject: [PATCH] refactor(sdk-coin-flrp): simplify signature recovery and verification methods Ticket: WIN-8452 --- modules/sdk-coin-flr/src/flr.ts | 7 +- modules/sdk-coin-flrp/src/flrp.ts | 4 +- modules/sdk-coin-flrp/src/lib/transaction.ts | 1 - modules/sdk-coin-flrp/src/lib/utils.ts | 20 +++--- modules/sdk-coin-flrp/test/unit/flrp.ts | 10 +-- modules/sdk-coin-flrp/test/unit/lib/utils.ts | 76 ++++++++++++++++---- 6 files changed, 80 insertions(+), 38 deletions(-) diff --git a/modules/sdk-coin-flr/src/flr.ts b/modules/sdk-coin-flr/src/flr.ts index 2d0bc63944..4961932fc0 100644 --- a/modules/sdk-coin-flr/src/flr.ts +++ b/modules/sdk-coin-flr/src/flr.ts @@ -26,7 +26,7 @@ import { TransactionExplanation, Entry, } from '@bitgo/sdk-core'; -import { BaseCoin as StaticsBaseCoin, coins, FlareNetwork } from '@bitgo/statics'; +import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics'; import { AbstractEthLikeNewCoins, optionalDeps, @@ -157,10 +157,7 @@ export class Flr extends AbstractEthLikeNewCoins { const tx = await txBuilder.build(); const payload = tx.signablePayload; const signatures = tx.signature.map((s) => Buffer.from(FlrPLib.Utils.removeHexPrefix(s), 'hex')); - const network = _.get(tx, '_network'); - const recoverPubkey = signatures.map((s) => - FlrPLib.Utils.recoverySignature(network as unknown as FlareNetwork, payload, s) - ); + const recoverPubkey = signatures.map((s) => FlrPLib.Utils.recoverySignature(payload, s)); const expectedSenders = recoverPubkey.map((r) => pubToAddress(r, true)); const senders = tx.inputs.map((i) => FlrPLib.Utils.parseAddress(i.address)); return expectedSenders.every((e) => senders.some((sender) => e.equals(sender))); diff --git a/modules/sdk-coin-flrp/src/flrp.ts b/modules/sdk-coin-flrp/src/flrp.ts index 653608558e..6dcdaf713a 100644 --- a/modules/sdk-coin-flrp/src/flrp.ts +++ b/modules/sdk-coin-flrp/src/flrp.ts @@ -323,8 +323,8 @@ export class Flrp extends BaseCoin { } } - recoverySignature(message: Buffer, signature: Buffer): Buffer { - return FlrpLib.Utils.recoverySignature(this._staticsCoin.network as FlareNetwork, message, signature); + recoverySignature(messageHash: Buffer, signature: Buffer): Buffer { + return FlrpLib.Utils.recoverySignature(messageHash, signature); } async signMessage(key: KeyPair, message: string | Buffer): Promise { diff --git a/modules/sdk-coin-flrp/src/lib/transaction.ts b/modules/sdk-coin-flrp/src/lib/transaction.ts index 5c131b835b..f49fc3da8f 100644 --- a/modules/sdk-coin-flrp/src/lib/transaction.ts +++ b/modules/sdk-coin-flrp/src/lib/transaction.ts @@ -237,7 +237,6 @@ export class Transaction extends BaseTransaction { // avaxp P-chain: transaction.ts uses addChecksum() explicitly // avaxp C-chain: deprecatedTransaction.ts uses Tx.toStringHex() which internally adds checksum const rawTx = FlareUtils.bufferToHex(utils.addChecksum(signedTxBytes)); - console.log('rawTx in toBroadcastFormat:', rawTx); return rawTx; } diff --git a/modules/sdk-coin-flrp/src/lib/utils.ts b/modules/sdk-coin-flrp/src/lib/utils.ts index 83436bfa03..d5e92639c7 100644 --- a/modules/sdk-coin-flrp/src/lib/utils.ts +++ b/modules/sdk-coin-flrp/src/lib/utils.ts @@ -151,10 +151,13 @@ export class Utils implements BaseUtils { /** * Verifies a signature + * @param messageHash - The SHA256 hash of the message (e.g., signablePayload) + * @param signature - The 64-byte signature (without recovery parameter) + * @param publicKey - The public key to verify against + * @returns true if signature is valid */ - verifySignature(network: FlareNetwork, message: Buffer, signature: Buffer, publicKey: Buffer): boolean { + verifySignature(messageHash: Buffer, signature: Buffer, publicKey: Buffer): boolean { try { - const messageHash = this.sha256(message); return ecc.verify(messageHash, publicKey, signature); } catch (e) { return false; @@ -362,17 +365,13 @@ export class Utils implements BaseUtils { } /** - * FlareJS wrapper to recover signature - * @param network - * @param message - * @param signature + * Recover public key from signature + * @param messageHash - The SHA256 hash of the message (e.g., signablePayload) + * @param signature - 65-byte signature (64 bytes signature + 1 byte recovery parameter) * @return recovered public key */ - recoverySignature(network: FlareNetwork, message: Buffer, signature: Buffer): Buffer { + recoverySignature(messageHash: Buffer, signature: Buffer): Buffer { try { - // Hash the message first - must match the hash used in signing - const messageHash = createHash('sha256').update(message).digest(); - // Extract recovery parameter and signature if (signature.length !== 65) { throw new Error('Invalid signature length - expected 65 bytes (64 bytes signature + 1 byte recovery)'); @@ -382,6 +381,7 @@ export class Utils implements BaseUtils { const sigOnly = signature.slice(0, 64); // Recover public key using the provided recovery parameter + // messageHash should already be the SHA256 hash (signablePayload) const recovered = ecc.recoverPublicKey(messageHash, sigOnly, recoveryParam, true); if (!recovered) { throw new Error('Failed to recover public key'); diff --git a/modules/sdk-coin-flrp/test/unit/flrp.ts b/modules/sdk-coin-flrp/test/unit/flrp.ts index fa38e76249..85be54566c 100644 --- a/modules/sdk-coin-flrp/test/unit/flrp.ts +++ b/modules/sdk-coin-flrp/test/unit/flrp.ts @@ -186,8 +186,7 @@ describe('Flrp test cases', function () { const signature = await basecoin.signMessage(keys, messageToSign.toString('hex')); const verify = FlrpLib.Utils.verifySignature( - basecoin._staticsCoin.network, - messageToSign, + FlrpLib.Utils.sha256(messageToSign), signature.slice(0, 64), // Remove recovery byte for verification Buffer.from(pubKey, 'hex') ); @@ -551,12 +550,9 @@ describe('Flrp test cases', function () { it('should recover signature from signed message', async () => { const message = Buffer.from(SEED_ACCOUNT.message, 'utf8'); const privateKey = Buffer.from(SEED_ACCOUNT.privateKey, 'hex'); - - // Create signature const signature = FlrpLib.Utils.createSignature(basecoin._staticsCoin.network, message, privateKey); - - // Recover public key from signature - const recoveredPubKey = basecoin.recoverySignature(message, signature); + const messageHash = FlrpLib.Utils.sha256(message); + const recoveredPubKey = basecoin.recoverySignature(messageHash, signature); recoveredPubKey.should.be.instanceOf(Buffer); recoveredPubKey.length.should.equal(33); diff --git a/modules/sdk-coin-flrp/test/unit/lib/utils.ts b/modules/sdk-coin-flrp/test/unit/lib/utils.ts index b234dff6cd..c62e8c4ca0 100644 --- a/modules/sdk-coin-flrp/test/unit/lib/utils.ts +++ b/modules/sdk-coin-flrp/test/unit/lib/utils.ts @@ -15,6 +15,7 @@ import { IMPORT_IN_P } from '../../resources/transactionData/importInP'; import { EXPORT_IN_P } from '../../resources/transactionData/exportInP'; import { IMPORT_IN_C } from '../../resources/transactionData/importInC'; import { TransactionBuilderFactory, Transaction } from '../../../src/lib'; +import { secp256k1, Address } from '@flarenetwork/flarejs'; describe('Utils', function () { let utils: Utils; @@ -206,7 +207,8 @@ describe('Utils', function () { const signature = utils.createSignature(network, message, privateKey); const sigOnly = signature.slice(0, 64); - const isValid = utils.verifySignature(network, message, sigOnly, publicKey); + const messageHash = utils.sha256(message); + const isValid = utils.verifySignature(messageHash, sigOnly, publicKey); assert.strictEqual(isValid, true); }); @@ -215,7 +217,8 @@ describe('Utils', function () { const publicKey = Buffer.from(SEED_ACCOUNT.publicKey, 'hex'); const invalidSignature = Buffer.alloc(64); - const isValid = utils.verifySignature(network, message, invalidSignature, publicKey); + const messageHash = utils.sha256(message); + const isValid = utils.verifySignature(messageHash, invalidSignature, publicKey); assert.strictEqual(isValid, false); }); @@ -480,11 +483,12 @@ describe('Utils', function () { const message = Buffer.from(SEED_ACCOUNT.message, 'utf8'); const privateKey = Buffer.from(SEED_ACCOUNT.privateKey, 'hex'); - // Create signature using the same private key + // Create signature using the same private key (createSignature hashes the message internally) const signature = utils.createSignature(network, message, privateKey); - // Recover public key - const recoveredPubKey = utils.recoverySignature(network, message, signature); + // Recover public key - pass the hashed message since recoverySignature expects pre-hashed + const messageHash = utils.sha256(message); + const recoveredPubKey = utils.recoverySignature(messageHash, signature); assert.ok(recoveredPubKey instanceof Buffer); assert.strictEqual(recoveredPubKey.length, 33); // Should be compressed public key (33 bytes) @@ -495,8 +499,9 @@ describe('Utils', function () { const privateKey = Buffer.from(SEED_ACCOUNT.privateKey, 'hex'); const signature = utils.createSignature(network, message, privateKey); - const pubKey1 = utils.recoverySignature(network, message, signature); - const pubKey2 = utils.recoverySignature(network, message, signature); + const messageHash = utils.sha256(message); + const pubKey1 = utils.recoverySignature(messageHash, signature); + const pubKey2 = utils.recoverySignature(messageHash, signature); assert.deepStrictEqual(pubKey1, pubKey2); }); @@ -510,7 +515,8 @@ describe('Utils', function () { // Create signature and recover public key const signature = utils.createSignature(network, message, privateKey); - const recoveredPubKey = utils.recoverySignature(network, message, signature); + const messageHash = utils.sha256(message); + const recoveredPubKey = utils.recoverySignature(messageHash, signature); // Convert both to hex strings for comparison assert.strictEqual(recoveredPubKey.toString('hex'), originalPubKey.toString('hex')); @@ -523,23 +529,67 @@ describe('Utils', function () { const originalPubKey = Buffer.from(ecc.pointFromScalar(privateKey, true) as Uint8Array); const signature = utils.createSignature(network, message, privateKey); - const recoveredPubKey = utils.recoverySignature(network, message, signature); + const messageHash = utils.sha256(message); + const recoveredPubKey = utils.recoverySignature(messageHash, signature); assert.strictEqual(recoveredPubKey.toString('hex'), originalPubKey.toString('hex')); }); it('should throw error for invalid signature length', function () { - const message = Buffer.from(SEED_ACCOUNT.message, 'utf8'); + const messageHash = utils.sha256(Buffer.from(SEED_ACCOUNT.message, 'utf8')); const invalidSignature = Buffer.from(INVALID_SHORT_KEYPAIR_KEY, 'hex'); - assert.throws(() => utils.recoverySignature(network, message, invalidSignature), /Failed to recover signature/); + assert.throws(() => utils.recoverySignature(messageHash, invalidSignature), /Failed to recover signature/); }); it('should throw error for signature with invalid recovery parameter', function () { - const message = Buffer.from(SEED_ACCOUNT.message, 'utf8'); + const messageHash = utils.sha256(Buffer.from(SEED_ACCOUNT.message, 'utf8')); const signature = Buffer.alloc(65); // Valid length but all zeros - invalid signature - assert.throws(() => utils.recoverySignature(network, message, signature), /Failed to recover signature/); + assert.throws(() => utils.recoverySignature(messageHash, signature), /Failed to recover signature/); + }); + + it('should recover signature and verify sender address from signed C-chain Export tx', async function () { + // Transaction from actual build response - C-chain Export tx + const tx = + '0x0000000000010000007278db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000000000000000000000000000000000000000000000000000000000000000000012a96025ad506b9fbb9023fbdc1665c7f7d7c923f000000000605236658734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd00000000000000000000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000070000000006052340000000000000000000000002000000037fa8c7e0c8ad9f09f9179b42b77e94a487c3df758d4ba538f772333ca7bf3668a2fe36648438c79d9b6b77b56effb860eaa430e0e30c4e392f59cd08000000010000000900000001750076e67d9720283a71c6e7a9a88ff662608fefdd3f316f1211957ca1873eee3ee4a74b468bda66176a3e5d3ab54d43a8c0be12348f251a3093c16d9db00cd001c31e9c15'; + const expectedSenderAddress = '0x2a96025ad506b9fbb9023fbdc1665c7f7d7c923f'; + + const factory = new TransactionBuilderFactory(coins.get('tflrp')); + const txn = (await factory.from(tx).build()) as Transaction; + const signablePayload = txn.signablePayload; + const signatures = txn.signature; + const sig = Buffer.from(utils.removeHexPrefix(signatures[0]), 'hex'); + + // Recover public key from signature (signablePayload is already SHA256 hashed) + const recoveredPubKey = utils.recoverySignature(signablePayload, sig); + + // Get the sender address from the transaction inputs + const txInputs = txn.inputs; + const senderAddressFromTx = txInputs[0].address.toLowerCase(); + + // Verify sender address matches expected + assert.strictEqual( + senderAddressFromTx, + expectedSenderAddress.toLowerCase(), + 'Transaction sender address does not match expected' + ); + + // Derive address from recovered public key + const derivedEvmAddress = + '0x' + Buffer.from(new Address(secp256k1.publicKeyToEthAddress(recoveredPubKey)).toBytes()).toString('hex'); + + // Verify the recovered public key matches the sender + assert.strictEqual( + derivedEvmAddress.toLowerCase(), + senderAddressFromTx, + 'Recovered public key does not match sender address' + ); + + // Also verify signature validity + const sigOnly = sig.slice(0, 64); + const isValid = utils.verifySignature(signablePayload, sigOnly, recoveredPubKey); + assert.strictEqual(isValid, true, 'Signature verification failed'); }); });