From 10eb3fc2fa5cc5718933287765299df0dd036e82 Mon Sep 17 00:00:00 2001 From: Svetoslav Borislavov Date: Tue, 5 Nov 2024 13:20:28 +0200 Subject: [PATCH] feat: add transaction oversized on create message (#1117) Signed-off-by: Svetoslav Borislavov --- .../transactions/transactions.service.spec.ts | 40 +++++++++------- .../src/transactions/transactions.service.ts | 6 +++ .../api/test/spec/transaction.e2e-spec.ts | 46 +++++++++++++++++++ .../libs/common/src/constants/errorCodes.ts | 2 + .../libs/common/src/utils/sdk/transaction.ts | 5 ++ .../src/main/shared/constants/errorCodes.ts | 2 + 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/back-end/apps/api/src/transactions/transactions.service.spec.ts b/back-end/apps/api/src/transactions/transactions.service.spec.ts index 9cb48ec6d..98bc40045 100644 --- a/back-end/apps/api/src/transactions/transactions.service.spec.ts +++ b/back-end/apps/api/src/transactions/transactions.service.spec.ts @@ -18,7 +18,6 @@ import { TransactionId, Timestamp, Client, - FileUpdateTransaction, } from '@hashgraph/sdk'; import { NOTIFICATIONS_SERVICE, MirrorNodeService, ErrorCodes } from '@app/common'; @@ -31,6 +30,7 @@ import { notifyWaitingForSignatures, notifySyncIndicators, MirrorNetworkGRPC, + isTransactionBodyOverMaxSize, } from '@app/common/utils'; import { Transaction, @@ -383,6 +383,7 @@ describe('TransactionsService', () => { }); jest.spyOn(PublicKey.prototype, 'verify').mockReturnValueOnce(true); jest.mocked(isExpired).mockReturnValueOnce(false); + jest.mocked(isTransactionBodyOverMaxSize).mockReturnValueOnce(false); transactionsRepo.count.mockResolvedValueOnce(0); jest.spyOn(MirrorNetworkGRPC, 'fromBaseURL').mockReturnValueOnce(MirrorNetworkGRPC.TESTNET); jest.mocked(getClientFromNetwork).mockResolvedValueOnce(client); @@ -440,8 +441,8 @@ describe('TransactionsService', () => { await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.SNMP); }); - it.skip('should throw on transaction create if unsupported type', async () => { - const sdkTransaction = new FileUpdateTransaction(); + it('should throw on transaction create if expired', async () => { + const sdkTransaction = new AccountCreateTransaction(); const dto: CreateTransactionDto = { name: 'Transaction 1', @@ -456,14 +457,15 @@ describe('TransactionsService', () => { usr.keys = userKeys; }); jest.spyOn(PublicKey.prototype, 'verify').mockReturnValueOnce(true); + jest.mocked(isExpired).mockReturnValueOnce(true); - await expect(service.createTransaction(dto, user as User)).rejects.toThrow( - 'File Update/Append transactions are not currently supported', - ); + await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.TE); }); - it('should throw on transaction create if expired', async () => { - const sdkTransaction = new AccountCreateTransaction(); + it('should throw on transaction create if save fails', async () => { + const sdkTransaction = new AccountCreateTransaction().setTransactionId( + new TransactionId(AccountId.fromString('0.0.1'), Timestamp.fromDate(new Date())), + ); const dto: CreateTransactionDto = { name: 'Transaction 1', @@ -474,16 +476,25 @@ describe('TransactionsService', () => { mirrorNetwork: 'testnet', }; + const client = Client.forTestnet(); + jest.mocked(attachKeys).mockImplementationOnce(async (usr: User) => { usr.keys = userKeys; }); jest.spyOn(PublicKey.prototype, 'verify').mockReturnValueOnce(true); - jest.mocked(isExpired).mockReturnValueOnce(true); + jest.mocked(isExpired).mockReturnValueOnce(false); + jest.mocked(isTransactionBodyOverMaxSize).mockReturnValueOnce(false); + transactionsRepo.count.mockResolvedValueOnce(0); + jest.spyOn(MirrorNetworkGRPC, 'fromBaseURL').mockReturnValueOnce(MirrorNetworkGRPC.TESTNET); + jest.mocked(getClientFromNetwork).mockResolvedValueOnce(client); + transactionsRepo.save.mockRejectedValueOnce(new Error('Failed to save')); - await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.TE); + await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.FST); + + client.close(); }); - it('should throw on transaction create if save fails', async () => { + it('should throw on transaction create if transaction over max size', async () => { const sdkTransaction = new AccountCreateTransaction().setTransactionId( new TransactionId(AccountId.fromString('0.0.1'), Timestamp.fromDate(new Date())), ); @@ -504,12 +515,9 @@ describe('TransactionsService', () => { }); jest.spyOn(PublicKey.prototype, 'verify').mockReturnValueOnce(true); jest.mocked(isExpired).mockReturnValueOnce(false); - transactionsRepo.count.mockResolvedValueOnce(0); - jest.spyOn(MirrorNetworkGRPC, 'fromBaseURL').mockReturnValueOnce(MirrorNetworkGRPC.TESTNET); - jest.mocked(getClientFromNetwork).mockResolvedValueOnce(client); - transactionsRepo.save.mockRejectedValueOnce(new Error('Failed to save')); + jest.mocked(isTransactionBodyOverMaxSize).mockReturnValueOnce(true); - await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.FST); + await expect(service.createTransaction(dto, user as User)).rejects.toThrow(ErrorCodes.TOS); client.close(); }); diff --git a/back-end/apps/api/src/transactions/transactions.service.ts b/back-end/apps/api/src/transactions/transactions.service.ts index 5c4b26949..ddb4aa731 100644 --- a/back-end/apps/api/src/transactions/transactions.service.ts +++ b/back-end/apps/api/src/transactions/transactions.service.ts @@ -36,6 +36,7 @@ import { notifyWaitingForSignatures, notifySyncIndicators, ErrorCodes, + isTransactionBodyOverMaxSize, } from '@app/common'; import { CreateTransactionDto } from './dto'; @@ -326,6 +327,11 @@ export class TransactionsService { const sdkTransaction = SDKTransaction.fromBytes(dto.transactionBytes); if (isExpired(sdkTransaction)) throw new BadRequestException(ErrorCodes.TE); + /* Check if the transaction body is over the max size */ + if (isTransactionBodyOverMaxSize(sdkTransaction)) { + throw new BadRequestException(ErrorCodes.TOS); + } + /* Check if the transaction already exists */ const countExisting = await this.repo.count({ where: [ diff --git a/back-end/apps/api/test/spec/transaction.e2e-spec.ts b/back-end/apps/api/test/spec/transaction.e2e-spec.ts index bbc10b7c8..8c83830d8 100644 --- a/back-end/apps/api/test/spec/transaction.e2e-spec.ts +++ b/back-end/apps/api/test/spec/transaction.e2e-spec.ts @@ -5,6 +5,8 @@ import { AccountCreateTransaction, AccountUpdateTransaction, Hbar, + KeyList, + PrivateKey, TransferTransaction, } from '@hashgraph/sdk'; @@ -293,6 +295,50 @@ describe('Transactions (e2e)', () => { expect(countAfter).toEqual(countBefore); }); + it( + '(POST) should not create a transaction that is oversized', + async () => { + const countBefore = await repo.count(); + + const keylist = new KeyList(); + for (let i = 0; i < 180; i++) { + keylist.push(PrivateKey.generate().publicKey); + } + const transaction = new AccountCreateTransaction() + .setTransactionId(createTransactionId(localnet1003.accountId, new Date())) + .setKey(keylist); + + const buffer = Buffer.from(transaction.toBytes()).toString('hex'); + + const userKey = await getUserKey(user.id, localnet1003.publicKeyRaw); + + if (userKey === null) throw new Error('User key not found'); + + const dto = { + name: 'Oversized Account Create Transaction', + description: 'This is a oversized account create transaction', + transactionBytes: buffer, + creatorKeyId: userKey.id, + signature: Buffer.from(localnet1003.privateKey.sign(transaction.toBytes())).toString( + 'hex', + ), + mirrorNetwork: localnet1003.mirrorNetwork, + }; + + const { status, body } = await endpoint.post(dto, null, userAuthToken); + const countAfter = await repo.count(); + + expect(status).toEqual(400); + expect(body).toMatchObject( + expect.objectContaining({ + code: ErrorCodes.TOS, + }), + ); + expect(countAfter).toEqual(countBefore); + }, + 30 * 1_000, + ); + it('(GET) should get all transactions for user', async () => { const { status, body } = await endpoint.get(null, userAuthToken, 'page=1&size=99'); diff --git a/back-end/libs/common/src/constants/errorCodes.ts b/back-end/libs/common/src/constants/errorCodes.ts index ee5d891f0..d3dba31ae 100644 --- a/back-end/libs/common/src/constants/errorCodes.ts +++ b/back-end/libs/common/src/constants/errorCodes.ts @@ -30,6 +30,7 @@ export enum ErrorCodes { KNF = 'KNF', NNF = 'NNF', IB = 'IB', + TOS = 'TOS', } export const ErrorMessages: { [key in ErrorCodes]: string } = { @@ -65,4 +66,5 @@ export const ErrorMessages: { [key in ErrorCodes]: string } = { [ErrorCodes.KNF]: 'Key not found', [ErrorCodes.NNF]: 'Notification not found', [ErrorCodes.IB]: 'Invalid body', + [ErrorCodes.TOS]: 'Transaction is over the size limit and cannot be executed', }; diff --git a/back-end/libs/common/src/utils/sdk/transaction.ts b/back-end/libs/common/src/utils/sdk/transaction.ts index 3ec8fac82..fc681bae9 100644 --- a/back-end/libs/common/src/utils/sdk/transaction.ts +++ b/back-end/libs/common/src/utils/sdk/transaction.ts @@ -253,3 +253,8 @@ export async function isTransactionOverMaxSize(transaction: SDKTransaction) { const request = await transaction._makeRequestAsync(); return proto.Transaction.encode(request).finish().length > MAX_TRANSACTION_BYTE_SIZE; } + +export function isTransactionBodyOverMaxSize(transaction: SDKTransaction) { + const bodyBytes = getTransactionBodyBytes(transaction); + return bodyBytes.length > MAX_TRANSACTION_BYTE_SIZE; +} diff --git a/front-end/src/main/shared/constants/errorCodes.ts b/front-end/src/main/shared/constants/errorCodes.ts index ee5d891f0..d3dba31ae 100644 --- a/front-end/src/main/shared/constants/errorCodes.ts +++ b/front-end/src/main/shared/constants/errorCodes.ts @@ -30,6 +30,7 @@ export enum ErrorCodes { KNF = 'KNF', NNF = 'NNF', IB = 'IB', + TOS = 'TOS', } export const ErrorMessages: { [key in ErrorCodes]: string } = { @@ -65,4 +66,5 @@ export const ErrorMessages: { [key in ErrorCodes]: string } = { [ErrorCodes.KNF]: 'Key not found', [ErrorCodes.NNF]: 'Notification not found', [ErrorCodes.IB]: 'Invalid body', + [ErrorCodes.TOS]: 'Transaction is over the size limit and cannot be executed', };