From 6c8ace1353aa898fa2da76aca539eabc11619b01 Mon Sep 17 00:00:00 2001 From: rodolfopietro97 Date: Fri, 27 Oct 2023 12:33:35 +0200 Subject: [PATCH] fix: error handling for transaction --- .../core/src/transaction/handlers/decode.ts | 7 +- .../core/src/transaction/handlers/sign.ts | 45 +++++++++---- packages/core/src/transaction/transaction.ts | 48 ++++++++++---- packages/core/src/utils/errors.ts | 15 ----- .../transaction/transaction-handler.test.ts | 64 +++++++++---------- .../tests/transaction/transaction.test.ts | 28 ++++---- packages/errors/src/model/index.ts | 1 + packages/errors/src/types/errorTypes.ts | 27 ++++++-- 8 files changed, 147 insertions(+), 88 deletions(-) diff --git a/packages/core/src/transaction/handlers/decode.ts b/packages/core/src/transaction/handlers/decode.ts index cf098a30a..6666717f2 100644 --- a/packages/core/src/transaction/handlers/decode.ts +++ b/packages/core/src/transaction/handlers/decode.ts @@ -1,12 +1,12 @@ import { type RLPValidObject } from '../../encoding'; import { - ERRORS, SIGNED_TRANSACTION_RLP, TRANSACTION_FEATURES_KIND, UNSIGNED_TRANSACTION_RLP } from '../../utils'; import { Transaction } from '../transaction'; import { type TransactionBody } from '../types'; +import { buildError, TRANSACTION } from '@vechain-sdk/errors'; /** * Decode a raw transaction. @@ -75,7 +75,10 @@ function _decodeReservedField(reserved: Buffer[]): { } { // Not trimmed reserved field if (reserved[reserved.length - 1].length === 0) - throw new Error(ERRORS.TRANSACTION.INVALID_RESERVED_NOT_TRIMMED_FIELDS); + throw buildError( + TRANSACTION.INVALID_TRANSACTION_BODY, + 'Invalid reserved field. Fields must be trimmed' + ); // Get features field const featuresField = TRANSACTION_FEATURES_KIND.kind diff --git a/packages/core/src/transaction/handlers/sign.ts b/packages/core/src/transaction/handlers/sign.ts index 46846d7a6..6ace15d1b 100644 --- a/packages/core/src/transaction/handlers/sign.ts +++ b/packages/core/src/transaction/handlers/sign.ts @@ -1,7 +1,7 @@ import { address } from '../../address'; import { secp256k1 } from '../../secp256k1'; -import { ERRORS } from '../../utils'; import { Transaction } from '../transaction'; +import { buildError, SECP256K1, TRANSACTION } from '@vechain-sdk/errors'; /** * Sign a transaction with a given private key @@ -16,15 +16,24 @@ function sign( ): Transaction { // Invalid private key if (!secp256k1.isValidPrivateKey(signerPrivateKey)) - throw new Error(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + throw buildError( + SECP256K1.INVALID_SECP256k1_PRIVATE_KEY, + 'Invalid private key used to sign the transaction.' + ); // Transaction is already signed if (transactionToSign.isSigned) - throw new Error(ERRORS.TRANSACTION.ALREADY_SIGN); + throw buildError( + TRANSACTION.ALREADY_SIGNED, + 'Cannot sign Transaction. It is already signed.' + ); // Transaction is delegated if (transactionToSign.isDelegated) - throw new Error(ERRORS.TRANSACTION.DELEGATED); + throw buildError( + TRANSACTION.INVALID_DELEGATION, + 'Transaction is already delegated. Use signWithDelegator method instead.' + ); // Sign transaction const signature = secp256k1.sign( @@ -49,21 +58,31 @@ function signWithDelegator( signerPrivateKey: Buffer, delegatorPrivateKey: Buffer ): Transaction { - // @note we will improve error messages in the future! - // Invalid private key - if ( - !secp256k1.isValidPrivateKey(signerPrivateKey) || - !secp256k1.isValidPrivateKey(delegatorPrivateKey) - ) - throw new Error(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + // Invalid private keys + if (!secp256k1.isValidPrivateKey(signerPrivateKey)) + throw buildError( + SECP256K1.INVALID_SECP256k1_PRIVATE_KEY, + 'Invalid signer private key used to sign the transaction.' + ); + if (!secp256k1.isValidPrivateKey(delegatorPrivateKey)) + throw buildError( + SECP256K1.INVALID_SECP256k1_PRIVATE_KEY, + 'Invalid delegator private key used to sign the transaction.' + ); // Transaction is already signed if (transactionToSign.isSigned) - throw new Error(ERRORS.TRANSACTION.ALREADY_SIGN); + throw buildError( + TRANSACTION.ALREADY_SIGNED, + 'Transaction is already signed.' + ); // Transaction is not delegated if (!transactionToSign.isDelegated) - throw new Error(ERRORS.TRANSACTION.NOT_DELEGATED); + throw buildError( + TRANSACTION.INVALID_DELEGATION, + 'Transaction is not delegated. You are using the wrong method. Use sign method instead.' + ); const transactionHash = transactionToSign.getSignatureHash(); const delegatedHash = transactionToSign.getSignatureHash( diff --git a/packages/core/src/transaction/transaction.ts b/packages/core/src/transaction/transaction.ts index b0d7e5cb1..97b91319b 100644 --- a/packages/core/src/transaction/transaction.ts +++ b/packages/core/src/transaction/transaction.ts @@ -5,15 +5,19 @@ import { secp256k1 } from '../secp256k1'; import { BLOCKREF_LENGTH, dataUtils, - ERRORS, SIGNATURE_LENGTH, SIGNED_TRANSACTION_RLP, TRANSACTION_FEATURES_KIND, - UNSIGNED_TRANSACTION_RLP, - TransactionUtils + TransactionUtils, + UNSIGNED_TRANSACTION_RLP } from '../utils'; import { type TransactionBody } from './types'; -import { ADDRESS, buildError } from '@vechain-sdk/errors'; +import { + ADDRESS, + buildError, + SECP256K1, + TRANSACTION +} from '@vechain-sdk/errors'; /** * Represents an immutable transaction entity. @@ -49,12 +53,20 @@ class Transaction { constructor(body: TransactionBody, signature?: Buffer) { // Body if (this._isValidBody(body)) this.body = body; - else throw new Error(ERRORS.TRANSACTION.INVALID_TRANSACTION_BODY); + else + throw buildError( + TRANSACTION.INVALID_TRANSACTION_BODY, + 'Transaction body is not valid' + ); // User passed a signature if (signature !== undefined) { if (this._isSignatureValid(signature)) this.signature = signature; - else throw new Error(ERRORS.TRANSACTION.INVALID_SIGNATURE); + else + throw buildError( + SECP256K1.INVALID_SECP256k1_SIGNATURE, + 'Invalid transaction signature' + ); } } @@ -86,10 +98,17 @@ class Transaction { public get delegator(): string { // Undelegated transaction if (!this.isDelegated) - throw new Error(ERRORS.TRANSACTION.NOT_DELEGATED); + throw buildError( + TRANSACTION.INVALID_DELEGATION, + 'Transaction is not delegated' + ); // Unsigned transaction (@note we don't check if signature is valid or not, because we have checked it into constructor at creation time) - if (!this.isSigned) throw new Error(ERRORS.TRANSACTION.NOT_SIGNED); + if (!this.isSigned) + throw buildError( + TRANSACTION.NOT_SIGNED, + 'Cannot get delegator from unsigned transaction.' + ); // Slice signature needed to recover public key // Obtains the recovery param from the signature @@ -204,7 +223,11 @@ class Transaction { */ public get origin(): string { // Unsigned transaction (@note we don't check if signature is valid or not, because we have checked it into constructor at creation time) - if (!this.isSigned) throw new Error(ERRORS.TRANSACTION.NOT_SIGNED); + if (!this.isSigned) + throw buildError( + TRANSACTION.NOT_SIGNED, + 'Cannot get origin from unsigned transaction.' + ); // Slice signature // Obtains the concatenated signature (r, s) of ECDSA digital signature @@ -227,8 +250,11 @@ class Transaction { */ get id(): string { // Unsigned transaction (@note we don't check if signature is valid or not, because we have checked it into constructor at creation time) - if (!this.isSigned) throw new Error(ERRORS.TRANSACTION.NOT_SIGNED); - + if (!this.isSigned) + throw buildError( + TRANSACTION.NOT_SIGNED, + 'Cannot get transaction id from unsigned transaction.' + ); // Return transaction ID return blake2b256( Buffer.concat([ diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index e8ccf64b3..a97026c67 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -15,21 +15,6 @@ const ERRORS = { INVALID_DATA_TYPE: function (format: string): string { return `Invalid data type. Data should be ${format}.`; } - }, - - /** - * Error messages related to transactions. - */ - TRANSACTION: { - ALREADY_SIGN: 'Transaction is already signed', - NOT_SIGNED: 'Transaction is not signed', - INVALID_SIGNATURE: 'Invalid signature', - NOT_DELEGATED: 'Transaction is not delegated', - INVALID_SIGNATURE_PRIVATE_KEY: 'Invalid signature private key', - INVALID_RESERVED_NOT_TRIMMED_FIELDS: - 'Invalid reserved field. Fields must be trimmed', - INVALID_TRANSACTION_BODY: 'Invalid transaction body', - DELEGATED: 'Transaction is delegated' } }; diff --git a/packages/core/tests/transaction/transaction-handler.test.ts b/packages/core/tests/transaction/transaction-handler.test.ts index 9e78bf755..90b540565 100644 --- a/packages/core/tests/transaction/transaction-handler.test.ts +++ b/packages/core/tests/transaction/transaction-handler.test.ts @@ -1,16 +1,18 @@ import { describe, expect, test } from '@jest/globals'; -import { - ERRORS, - SIGNATURE_LENGTH, - Transaction, - TransactionHandler -} from '../../src'; +import { SIGNATURE_LENGTH, Transaction, TransactionHandler } from '../../src'; import { transactions, invalidDecodedNotTrimmedReserved, signer, delegator } from './fixture'; +import { + InvalidSecp256k1PrivateKeyError, + TransactionAlreadySignedError, + TransactionBodyError, + TransactionDelegationError, + TransactionNotSignedError +} from '@vechain-sdk/errors'; /** * Transaction handler tests @@ -37,7 +39,7 @@ describe('Transaction handler', () => { signedTransaction, signer.privateKey ) - ).toThrowError(ERRORS.TRANSACTION.ALREADY_SIGN); + ).toThrowError(TransactionAlreadySignedError); // Sign a non-delegated transaction with delegator expect(() => @@ -46,7 +48,7 @@ describe('Transaction handler', () => { signer.privateKey, delegator.privateKey ) - ).toThrowError(ERRORS.TRANSACTION.NOT_DELEGATED); + ).toThrowError(TransactionDelegationError); // Checks on signature expect(signedTransaction.isSigned).toBe(true); @@ -59,8 +61,8 @@ describe('Transaction handler', () => { expect(signedTransaction.origin).toBeDefined(); expect(signedTransaction.origin).toBe(signer.address); - expect(() => signedTransaction.delegator).toThrow( - ERRORS.TRANSACTION.NOT_DELEGATED + expect(() => signedTransaction.delegator).toThrowError( + TransactionDelegationError ); expect(signedTransaction.isDelegated).toBe(false); expect(signedTransaction.id).toBeDefined(); @@ -85,7 +87,7 @@ describe('Transaction handler', () => { signer.privateKey, delegator.privateKey ) - ).toThrowError(ERRORS.TRANSACTION.ALREADY_SIGN); + ).toThrowError(TransactionAlreadySignedError); // Sign normally a delegated transaction expect(() => @@ -93,14 +95,14 @@ describe('Transaction handler', () => { new Transaction(transaction.body), signer.privateKey ) - ).toThrowError(ERRORS.TRANSACTION.DELEGATED); + ).toThrowError(TransactionDelegationError); expect(() => TransactionHandler.sign( new Transaction(transaction.body), delegator.privateKey ) - ).toThrowError(ERRORS.TRANSACTION.DELEGATED); + ).toThrowError(TransactionDelegationError); // Checks on signature expect(signedTransaction.isSigned).toBe(true); @@ -129,7 +131,7 @@ describe('Transaction handler', () => { new Transaction(transactions.undelegated[0].body), Buffer.from('INVALID', 'hex') ); - }).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + }).toThrowError(InvalidSecp256k1PrivateKeyError); // Invalid private keys - delegated expect(() => { @@ -138,7 +140,7 @@ describe('Transaction handler', () => { Buffer.from('INVALID', 'hex'), delegator.privateKey ); - }).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + }).toThrowError(InvalidSecp256k1PrivateKeyError); expect(() => { TransactionHandler.signWithDelegator( @@ -146,7 +148,7 @@ describe('Transaction handler', () => { signer.privateKey, Buffer.from('INVALID', 'hex') ); - }).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + }).toThrowError(InvalidSecp256k1PrivateKeyError); expect(() => { TransactionHandler.signWithDelegator( @@ -154,7 +156,7 @@ describe('Transaction handler', () => { Buffer.from('INVALID', 'hex'), Buffer.from('INVALID', 'hex') ); - }).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY); + }).toThrowError(InvalidSecp256k1PrivateKeyError); }); }); @@ -178,15 +180,15 @@ describe('Transaction handler', () => { transactions.undelegated[0].body ); expect(decodedUnsigned.signature).toBeUndefined(); - expect(() => decodedUnsigned.origin).toThrow( - ERRORS.TRANSACTION.NOT_SIGNED + expect(() => decodedUnsigned.origin).toThrowError( + TransactionNotSignedError ); - expect(() => decodedUnsigned.delegator).toThrow( - ERRORS.TRANSACTION.NOT_DELEGATED + expect(() => decodedUnsigned.delegator).toThrowError( + TransactionDelegationError ); expect(decodedUnsigned.isDelegated).toBe(false); - expect(() => decodedUnsigned.id).toThrow( - ERRORS.TRANSACTION.NOT_SIGNED + expect(() => decodedUnsigned.id).toThrowError( + TransactionNotSignedError ); expect(decodedUnsigned.isSigned).toBe(false); expect(decodedUnsigned.getSignatureHash()).toBeDefined(); @@ -240,15 +242,15 @@ describe('Transaction handler', () => { // Checks on decoded unsigned transaction expect(decodedUnsigned.body).toEqual(transaction.body); expect(decodedUnsigned.signature).toBeUndefined(); - expect(() => decodedUnsigned.origin).toThrow( - ERRORS.TRANSACTION.NOT_SIGNED + expect(() => decodedUnsigned.origin).toThrowError( + TransactionNotSignedError ); - expect(() => decodedUnsigned.delegator).toThrow( - ERRORS.TRANSACTION.NOT_SIGNED + expect(() => decodedUnsigned.delegator).toThrowError( + TransactionNotSignedError ); expect(decodedUnsigned.isDelegated).toBe(true); - expect(() => decodedUnsigned.id).toThrow( - ERRORS.TRANSACTION.NOT_SIGNED + expect(() => decodedUnsigned.id).toThrowError( + TransactionNotSignedError ); expect(decodedUnsigned.isSigned).toBe(false); expect(decodedUnsigned.getSignatureHash()).toBeDefined(); @@ -303,9 +305,7 @@ describe('Transaction handler', () => { invalidDecodedNotTrimmedReserved, false ) - ).toThrowError( - ERRORS.TRANSACTION.INVALID_RESERVED_NOT_TRIMMED_FIELDS - ); + ).toThrowError(TransactionBodyError); }); }); }); diff --git a/packages/core/tests/transaction/transaction.test.ts b/packages/core/tests/transaction/transaction.test.ts index 3ebc40a94..7c8d745bf 100644 --- a/packages/core/tests/transaction/transaction.test.ts +++ b/packages/core/tests/transaction/transaction.test.ts @@ -1,7 +1,13 @@ import { describe, expect, test } from '@jest/globals'; import { signer, delegator, transactions } from './fixture'; -import { ERRORS, Transaction, TransactionHandler } from '../../src'; -import { InvalidAddressError } from '@vechain-sdk/errors'; +import { Transaction, TransactionHandler } from '../../src'; +import { + InvalidAddressError, + InvalidSecp256k1SignatureError, + TransactionBodyError, + TransactionDelegationError, + TransactionNotSignedError +} from '@vechain-sdk/errors'; /** * Test transaction module @@ -30,17 +36,17 @@ describe('Transaction', () => { // Get id from unsigned transaction (should throw error) expect(() => unsignedTransaction.id).toThrowError( - ERRORS.TRANSACTION.NOT_SIGNED + TransactionNotSignedError ); // Get origin form unsigned transaction (should throw error) expect(() => unsignedTransaction.origin).toThrowError( - ERRORS.TRANSACTION.NOT_SIGNED + TransactionNotSignedError ); // Get delegator form unsigned and undelegated transaction (should throw error) expect(() => unsignedTransaction.delegator).toThrowError( - ERRORS.TRANSACTION.NOT_DELEGATED + TransactionDelegationError ); // Encoding @@ -85,7 +91,7 @@ describe('Transaction', () => { // Get delegator form undelegeted signed transaction (should throw error) expect(() => signedTransaction.delegator).toThrowError( - ERRORS.TRANSACTION.NOT_DELEGATED + TransactionDelegationError ); // Encoding @@ -118,17 +124,17 @@ describe('Transaction', () => { // Get id from unsigned transaction (should throw error) expect(() => unsignedTransaction.id).toThrowError( - ERRORS.TRANSACTION.NOT_SIGNED + TransactionNotSignedError ); // Get origin form unsigned transaction (should throw error) expect(() => unsignedTransaction.origin).toThrowError( - ERRORS.TRANSACTION.NOT_SIGNED + TransactionNotSignedError ); // Get delegator form unsigned transaction (should throw error) expect(() => unsignedTransaction.delegator).toThrowError( - ERRORS.TRANSACTION.NOT_SIGNED + TransactionNotSignedError ); // Encoding @@ -186,7 +192,7 @@ describe('Transaction', () => { transactions.delegated[0].body, Buffer.from('INVALID_SIGNATURE') ) - ).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE); + ).toThrowError(InvalidSecp256k1SignatureError); // Invalid transaction body (should throw error) expect( @@ -195,6 +201,6 @@ describe('Transaction', () => { ...transactions.delegated[0].body, blockRef: 'INVALID_BLOCK_REF' }) - ).toThrowError(ERRORS.TRANSACTION.INVALID_TRANSACTION_BODY); + ).toThrowError(TransactionBodyError); }); }); diff --git a/packages/errors/src/model/index.ts b/packages/errors/src/model/index.ts index 29707deb6..2ec3097c0 100644 --- a/packages/errors/src/model/index.ts +++ b/packages/errors/src/model/index.ts @@ -7,3 +7,4 @@ export * from './hdnode'; export * from './keystore'; export * from './rlp'; export * from './secp256k1'; +export * from './transaction'; diff --git a/packages/errors/src/types/errorTypes.ts b/packages/errors/src/types/errorTypes.ts index 78b5764d3..6fd2536fb 100644 --- a/packages/errors/src/types/errorTypes.ts +++ b/packages/errors/src/types/errorTypes.ts @@ -20,6 +20,10 @@ import { InvalidKeystorePasswordError, InvalidRLPError, InvalidSecp256k1PrivateKeyError, + TransactionAlreadySignedError, + TransactionNotSignedError, + TransactionBodyError, + TransactionDelegationError, type InvalidRLPErrorData, type ErrorBase, ABI, @@ -29,7 +33,8 @@ import { HDNODE, BLOOM, RLP, - DATA + DATA, + TRANSACTION } from '../model'; /** @@ -56,7 +61,8 @@ type ErrorCode = | BLOOM | ABI | RLP - | DATA; + | DATA + | TRANSACTION; /** * Conditional type to get the error data type from the error code. @@ -83,7 +89,8 @@ const ERROR_CODES = { BLOOM, ABI, RLP, - DATA + DATA, + TRANSACTION }; /** @@ -139,6 +146,14 @@ type ErrorType = ? InvalidDataTypeError : ErrorCodeT extends DATA.INVALID_DATA_RETURN_TYPE ? InvalidDataReturnTypeError + : ErrorCodeT extends TRANSACTION.ALREADY_SIGNED + ? TransactionAlreadySignedError + : ErrorCodeT extends TRANSACTION.NOT_SIGNED + ? TransactionNotSignedError + : ErrorCodeT extends TRANSACTION.INVALID_TRANSACTION_BODY + ? TransactionBodyError + : ErrorCodeT extends TRANSACTION.INVALID_DELEGATION + ? TransactionDelegationError : never; /** @@ -181,7 +196,11 @@ const ErrorClassMap = new Map< [ABI.INVALID_FUNCTION, InvalidAbiFunctionError], [RLP.INVALID_RLP, InvalidRLPError], [DATA.INVALID_DATA_TYPE, InvalidDataTypeError], - [DATA.INVALID_DATA_RETURN_TYPE, InvalidDataReturnTypeError] + [DATA.INVALID_DATA_RETURN_TYPE, InvalidDataReturnTypeError], + [TRANSACTION.ALREADY_SIGNED, TransactionAlreadySignedError], + [TRANSACTION.NOT_SIGNED, TransactionNotSignedError], + [TRANSACTION.INVALID_TRANSACTION_BODY, TransactionBodyError], + [TRANSACTION.INVALID_DELEGATION, TransactionDelegationError] ]); export {