Skip to content

Commit

Permalink
fix: error handling for transaction
Browse files Browse the repository at this point in the history
  • Loading branch information
@rodolfopietro97
rodolfopietro97 committed Oct 27, 2023
1 parent 3ba8fe1 commit 6c8ace1
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 88 deletions.
7 changes: 5 additions & 2 deletions packages/core/src/transaction/handlers/decode.ts
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
45 changes: 32 additions & 13 deletions packages/core/src/transaction/handlers/sign.ts
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand Down
48 changes: 37 additions & 11 deletions packages/core/src/transaction/transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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'
);
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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([
Expand Down
15 changes: 0 additions & 15 deletions packages/core/src/utils/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
};

Expand Down
64 changes: 32 additions & 32 deletions packages/core/tests/transaction/transaction-handler.test.ts
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(() =>
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -85,22 +87,22 @@ describe('Transaction handler', () => {
signer.privateKey,
delegator.privateKey
)
).toThrowError(ERRORS.TRANSACTION.ALREADY_SIGN);
).toThrowError(TransactionAlreadySignedError);

// Sign normally a delegated transaction
expect(() =>
TransactionHandler.sign(
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);
Expand Down Expand Up @@ -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(() => {
Expand All @@ -138,23 +140,23 @@ describe('Transaction handler', () => {
Buffer.from('INVALID', 'hex'),
delegator.privateKey
);
}).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY);
}).toThrowError(InvalidSecp256k1PrivateKeyError);

expect(() => {
TransactionHandler.signWithDelegator(
new Transaction(transactions.delegated[0].body),
signer.privateKey,
Buffer.from('INVALID', 'hex')
);
}).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY);
}).toThrowError(InvalidSecp256k1PrivateKeyError);

expect(() => {
TransactionHandler.signWithDelegator(
new Transaction(transactions.delegated[0].body),
Buffer.from('INVALID', 'hex'),
Buffer.from('INVALID', 'hex')
);
}).toThrowError(ERRORS.TRANSACTION.INVALID_SIGNATURE_PRIVATE_KEY);
}).toThrowError(InvalidSecp256k1PrivateKeyError);
});
});

Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -303,9 +305,7 @@ describe('Transaction handler', () => {
invalidDecodedNotTrimmedReserved,
false
)
).toThrowError(
ERRORS.TRANSACTION.INVALID_RESERVED_NOT_TRIMMED_FIELDS
);
).toThrowError(TransactionBodyError);
});
});
});
Loading

0 comments on commit 6c8ace1

Please sign in to comment.