Merge pull request #3945 from LiskHQ/3941-custom-tx-fails-fee-validation

Custom transaction fails to validate if `FEE` member is not defined - Closes #3941

elements/lisk-transactions/src/base_transaction.ts

@@ -115,7 +115,7 @@ export abstract class BaseTransaction {
 	public receivedAt?: Date;
 
 	public static TYPE: number;
-	public static FEE: string;
+	public static FEE = '0';
 
 	protected _id?: string;
 	protected _senderId?: string;
@@ -143,13 +143,22 @@ export abstract class BaseTransaction {
 		const tx = (typeof rawTransaction === 'object' && rawTransaction !== null
 			? rawTransaction
 			: {}) as Partial<TransactionJSON>;
+
 		this.amount = new BigNum(
 			isValidNumber(tx.amount) ? (tx.amount as string | number) : '0',
 		);
+
 		this.fee = new BigNum(
-			isValidNumber(tx.fee) ? (tx.fee as string | number) : '0',
+			isValidNumber(tx.fee)
+				? (tx.fee as string | number)
+				: (this.constructor as typeof BaseTransaction).FEE,
 		);
 
+		this.type =
+			typeof tx.type === 'number'
+				? tx.type
+				: (this.constructor as typeof BaseTransaction).TYPE;
+
 		this._id = tx.id;
 		this.recipientId = tx.recipientId || '';
 		this.recipientPublicKey = tx.recipientPublicKey || undefined;
@@ -166,10 +175,6 @@ export abstract class BaseTransaction {
 		this.signatures = (tx.signatures as string[]) || [];
 		this._signSignature = tx.signSignature;
 		this.timestamp = typeof tx.timestamp === 'number' ? tx.timestamp : 0;
-		this.type =
-			typeof tx.type === 'number'
-				? tx.type
-				: (this.constructor as typeof BaseTransaction).TYPE;
 
 		// Additional data not related to the protocol
 		this.confirmations = tx.confirmations;

elements/lisk-transactions/test/base_transaction.ts

@@ -25,6 +25,7 @@ import {
 	addTransactionFields,
 	MockStateStore as store,
 	TestTransaction,
+	TestTransactionBasicImpl,
 } from './helpers';
 import {
 	validAccount as defaultSenderAccount,
@@ -48,6 +49,7 @@ describe('Base transaction class', () => {
 
 	let validTestTransaction: BaseTransaction;
 	let transactionWithDefaultValues: BaseTransaction;
+	let transactionWithBasicImpl: BaseTransaction;
 	let validSecondSignatureTestTransaction: BaseTransaction;
 	let validMultisignatureTestTransaction: BaseTransaction;
 	let storeAccountGetStub: sinon.SinonStub;
@@ -56,6 +58,7 @@ describe('Base transaction class', () => {
 	beforeEach(async () => {
 		validTestTransaction = new TestTransaction(defaultTransaction);
 		transactionWithDefaultValues = new TestTransaction({});
+		transactionWithBasicImpl = new TestTransactionBasicImpl({});
 		validSecondSignatureTestTransaction = new TestTransaction(
 			defaultSecondSignatureTransaction,
 		);
@@ -79,7 +82,7 @@ describe('Base transaction class', () => {
 
 		it('should set default values', async () => {
 			expect(transactionWithDefaultValues.amount.toString()).to.be.eql('0');
-			expect(transactionWithDefaultValues.fee.toString()).to.be.eql('0');
+			expect(transactionWithDefaultValues.fee.toString()).to.be.eql('10000000');
 			expect(transactionWithDefaultValues.recipientId).to.be.eql('');
 			expect(transactionWithDefaultValues.recipientPublicKey).to.be.undefined;
 			expect(transactionWithDefaultValues.timestamp).to.be.eql(0);
@@ -116,6 +119,12 @@ describe('Base transaction class', () => {
 				.and.be.instanceof(BigNum);
 		});
 
+		it('should have default fee if fee param is invalid', async () => {
+			const transactionWithInvalidFee = new TestTransaction({ fee: 'invalid' });
+
+			expect(transactionWithInvalidFee.fee.toString()).to.be.eql('10000000');
+		});
+
 		it('should have id string', async () => {
 			expect(validTestTransaction)
 				.to.have.property('id')
@@ -501,6 +510,15 @@ describe('Base transaction class', () => {
 			expect(status).to.eql(Status.OK);
 		});
 
+		it('should return a successful transaction response with a valid transaction with basic impl', async () => {
+			transactionWithBasicImpl.sign('passphrase');
+			const { id, status, errors } = transactionWithBasicImpl.validate();
+
+			expect(id).to.be.eql(transactionWithBasicImpl.id);
+			expect(errors).to.be.empty;
+			expect(status).to.eql(Status.OK);
+		});
+
 		it('should call getBasicBytes', async () => {
 			const getBasicBytesStub = sandbox
 				.stub(validTestTransaction as any, 'getBasicBytes')

elements/lisk-transactions/test/helpers/index.ts

@@ -13,6 +13,14 @@
  *
  */
 import { addTransactionFields } from './add_transaction_fields';
-import { TestTransaction } from './test_transaction_class';
+import {
+	TestTransaction,
+	TestTransactionBasicImpl,
+} from './test_transaction_class';
 import { MockStateStore } from './state_store';
-export { addTransactionFields, MockStateStore, TestTransaction };
+export {
+	addTransactionFields,
+	MockStateStore,
+	TestTransaction,
+	TestTransactionBasicImpl,
+};

elements/lisk-transactions/test/helpers/test_transaction_class.ts

@@ -57,3 +57,31 @@ export class TestTransaction extends BaseTransaction {
 		return { data: raw };
 	}
 }
+
+export class TestTransactionBasicImpl extends BaseTransaction {
+	public static TYPE = 1;
+
+	public validateAsset() {
+		return [];
+	}
+
+	public applyAsset() {
+		return [];
+	}
+
+	public undoAsset() {
+		return [];
+	}
+
+	public verifyAgainstTransactions(
+		transactions: ReadonlyArray<TransactionJSON>,
+	): ReadonlyArray<TransactionError> {
+		transactions.forEach(() => true);
+
+		return [];
+	}
+
+	public assetFromSync(raw: any) {
+		return { data: raw };
+	}
+}