Skip to content

Commit

Permalink
feat: allow manual execution (#1455)
Browse files Browse the repository at this point in the history
Signed-off-by: Svetoslav Borislavov <[email protected]>
Signed-off-by: John Bair <[email protected]>
Co-authored-by: John Bair <[email protected]>
  • Loading branch information
SvetBorislavov and jbair06 authored Jan 16, 2025
1 parent eee4bdf commit 8abd3c3
Show file tree
Hide file tree
Showing 35 changed files with 335 additions and 290 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,9 @@ export class ApproversService {
/* Check if the transaction exists */
if (!transaction) throw new BadRequestException(ErrorCodes.TNF);

/* Checks if the transaction is executed */
/* Checks if the transaction is requires approval */
if (
transaction.status !== TransactionStatus.WAITING_FOR_SIGNATURES &&
transaction.status !== TransactionStatus.SIGN_ONLY &&
transaction.status !== TransactionStatus.WAITING_FOR_EXECUTION
)
throw new BadRequestException(ErrorCodes.TNRA);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,5 @@ export class CreateTransactionDto {

@IsOptional()
@IsBoolean()
isSignOnly?: boolean;
isManual?: boolean;
}
3 changes: 3 additions & 0 deletions back-end/apps/api/src/transactions/dto/transaction.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ export class TransactionDto {
@Expose()
validStart: Date;

@Expose()
isManual: boolean;

@Expose()
cutoffAt?: Date;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,13 @@ export class SignersService {
/* Checks if the transaction is canceled */
if (
transaction.status !== TransactionStatus.WAITING_FOR_SIGNATURES &&
transaction.status !== TransactionStatus.SIGN_ONLY &&
transaction.status !== TransactionStatus.WAITING_FOR_EXECUTION
)
throw new BadRequestException(ErrorCodes.TNRS);

/* Checks if the transaction is expired */
let sdkTransaction = SDKTransaction.fromBytes(transaction.transactionBytes);
if (isExpired(sdkTransaction) && transaction.status !== TransactionStatus.SIGN_ONLY)
throw new BadRequestException(ErrorCodes.TE);
if (isExpired(sdkTransaction)) throw new BadRequestException(ErrorCodes.TE);

/* Validates the signatures */
const { data: publicKeys, error } = safe<PublicKey[]>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ describe('TransactionsController', () => {
),
status: TransactionStatus.NEW,
mirrorNetwork: 'testnet',
isManual: false,
cutoffAt: new Date(),
createdAt: new Date(),
updatedAt: new Date(),
Expand Down Expand Up @@ -297,25 +298,6 @@ describe('TransactionsController', () => {
});
});

describe('markAsSignOnlyTransaction', () => {
it('should return a boolean indicating if the transaction has been marked as sign only', async () => {
const result = true;
transactionService.markAsSignOnlyTransaction.mockResolvedValue(result);

expect(await controller.markAsSignOnlyTransaction(user, 1)).toBe(result);
});

it('should return a boolean indicating if the transaction has not been marked as sign only', async () => {
jest
.spyOn(controller, 'markAsSignOnlyTransaction')
.mockRejectedValue(new BadRequestException('Transaction cannot be marked as sign only'));

await expect(controller.markAsSignOnlyTransaction(user, 1)).rejects.toThrow(
'Transaction cannot be marked as sign only',
);
});
});

describe('archiveTransaction', () => {
it('should return a boolean indicating if the transaction has been archiveed', async () => {
const result = true;
Expand Down
21 changes: 10 additions & 11 deletions back-end/apps/api/src/transactions/transactions.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,36 +210,35 @@ export class TransactionsController {
}

@ApiOperation({
summary: 'Marks the transaction as sign-only',
description:
'Marks a transaction as sign-only, meaning it would not be executed. Only signatures will be gathered.',
summary: 'Archives a transaction',
description: 'Archive a transaction that is marked as sign only',
})
@ApiResponse({
status: 200,
type: Boolean,
})
@Patch('/mark-sign-only/:id')
async markAsSignOnlyTransaction(
@Patch('/archive/:id')
async archiveTransaction(
@GetUser() user,
@Param('id', ParseIntPipe) id: number,
): Promise<boolean> {
return this.transactionsService.markAsSignOnlyTransaction(id, user);
return this.transactionsService.archiveTransaction(id, user);
}

@ApiOperation({
summary: 'Archives a transaction',
description: 'Archive a transaction that is marked as sign only',
summary: 'Send a transaction for execution',
description: 'Send a manual transaction to the chain service that will execute it',
})
@ApiResponse({
status: 200,
type: Boolean,
})
@Patch('/archive/:id')
async archiveTransaction(
@Patch('/execute/:id')
async executeTransaction(
@GetUser() user,
@Param('id', ParseIntPipe) id: number,
): Promise<boolean> {
return this.transactionsService.archiveTransaction(id, user);
return this.transactionsService.executeTransaction(id, user);
}

@ApiOperation({
Expand Down
78 changes: 43 additions & 35 deletions back-end/apps/api/src/transactions/transactions.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
Client,
} from '@hashgraph/sdk';

import { NOTIFICATIONS_SERVICE, MirrorNodeService, ErrorCodes } from '@app/common';
import { NOTIFICATIONS_SERVICE, MirrorNodeService, ErrorCodes, CHAIN_SERVICE } from '@app/common';
import {
attachKeys,
getClientFromNetwork,
Expand All @@ -31,6 +31,7 @@ import {
notifySyncIndicators,
MirrorNetworkGRPC,
isTransactionBodyOverMaxSize,
emitExecuteTranasction,
} from '@app/common/utils';
import {
Transaction,
Expand All @@ -52,6 +53,7 @@ describe('TransactionsService', () => {
let service: TransactionsService;

const transactionsRepo = mockDeep<Repository<Transaction>>();
const chainService = mock<ClientProxy>();
const notificationsService = mock<ClientProxy>();
const approversService = mock<ApproversService>();
const mirrorNodeService = mock<MirrorNodeService>();
Expand Down Expand Up @@ -89,6 +91,10 @@ describe('TransactionsService', () => {
provide: NOTIFICATIONS_SERVICE,
useValue: notificationsService,
},
{
provide: CHAIN_SERVICE,
useValue: chainService,
},
{
provide: ApproversService,
useValue: approversService,
Expand Down Expand Up @@ -427,7 +433,7 @@ describe('TransactionsService', () => {
client.close();
});

it('should create a sign-only transaction', async () => {
it('should create a manual transaction', async () => {
const sdkTransaction = new AccountCreateTransaction().setTransactionId(
new TransactionId(AccountId.fromString('0.0.1'), Timestamp.fromDate(new Date())),
);
Expand All @@ -439,7 +445,7 @@ describe('TransactionsService', () => {
creatorKeyId: 1,
signature: Buffer.from('0xabc02'),
mirrorNetwork: 'testnet',
isSignOnly: true,
isManual: true,
};

const client = Client.forTestnet();
Expand All @@ -464,7 +470,7 @@ describe('TransactionsService', () => {
await service.createTransaction(dto, user as User);

expect(transactionsRepo.save).toHaveBeenCalledWith(
expect.objectContaining({ status: TransactionStatus.SIGN_ONLY }),
expect.objectContaining({ isManual: true }),
);
expect(notifyWaitingForSignatures).toHaveBeenCalledWith(notificationsService, 1);
expect(notifyTransactionAction).toHaveBeenCalledWith(notificationsService);
Expand Down Expand Up @@ -668,97 +674,98 @@ describe('TransactionsService', () => {
});
});

describe('markAsSignOnlyTransaction', () => {
describe('archiveTransaction', () => {
beforeEach(() => {
jest.resetAllMocks();
});

it('should throw if transaction status is not in progress', async () => {
it('should throw if transaction status is not archiveable', async () => {
const transaction = {
creatorKey: { userId: 1 },
status: TransactionStatus.EXECUTED,
status: TransactionStatus.CANCELED,
};

jest
.spyOn(service, 'getTransactionForCreator')
.mockResolvedValueOnce(transaction as Transaction);

await expect(service.markAsSignOnlyTransaction(123, { id: 1 } as User)).rejects.toThrow(
ErrorCodes.OTIP,
await expect(service.archiveTransaction(123, { id: 1 } as User)).rejects.toThrow(
ErrorCodes.OMTIP,
);
});

it('should update transaction status to SIGN_ONLY and return true', async () => {
it('should update transaction status to ARCHIVED and return true', async () => {
const transaction = {
id: 123,
creatorKey: { userId: 1 },
status: TransactionStatus.WAITING_FOR_SIGNATURES,
isManual: true,
status: TransactionStatus.WAITING_FOR_EXECUTION,
};

jest
.spyOn(service, 'getTransactionForCreator')
.mockResolvedValueOnce(transaction as Transaction);

const result = await service.markAsSignOnlyTransaction(123, { id: 1 } as User);
const result = await service.archiveTransaction(123, { id: 1 } as User);

expect(transactionsRepo.update).toHaveBeenCalledWith(
{ id: 123 },
{ status: TransactionStatus.SIGN_ONLY },
{ status: TransactionStatus.ARCHIVED },
);
expect(result).toBe(true);
expect(notifySyncIndicators).toHaveBeenCalledWith(
notificationsService,
transaction.id,
TransactionStatus.SIGN_ONLY,
TransactionStatus.ARCHIVED,
);
expect(notifyTransactionAction).toHaveBeenCalledWith(notificationsService);
});
});

describe('archiveTransaction', () => {
describe('executeTransaction', () => {
beforeEach(() => {
jest.resetAllMocks();
});

it('should throw if transaction status is not archiveable', async () => {
it('should throw if transaction is not manual', async () => {
const transaction = {
creatorKey: { userId: 1 },
status: TransactionStatus.WAITING_FOR_SIGNATURES, //Only SIGN_ONLY transactions can be archived
id: 123,
creatorKey: { userId: user.id },
isManual: false,
};

jest
.spyOn(service, 'getTransactionForCreator')
.mockResolvedValueOnce(transaction as Transaction);

await expect(service.archiveTransaction(123, { id: 1 } as User)).rejects.toThrow(
ErrorCodes.OSONT,
);
await expect(service.executeTransaction(123, user as User)).rejects.toThrow(ErrorCodes.IO);
});

it('should update transaction status to ARCHIVED and return true', async () => {
it('should emit execute transaction event and return true if transaction is manual', async () => {
const transaction = {
id: 123,
creatorKey: { userId: 1 },
status: TransactionStatus.SIGN_ONLY,
creatorKey: { userId: user.id },
isManual: true,
status: TransactionStatus.WAITING_FOR_EXECUTION,
transactionBytes: Buffer.from('transactionBytes'),
mirrorNetwork: 'testnet',
validStart: new Date(),
};

jest
.spyOn(service, 'getTransactionForCreator')
.mockResolvedValueOnce(transaction as Transaction);

const result = await service.archiveTransaction(123, { id: 1 } as User);
const result = await service.executeTransaction(123, user as User);

expect(transactionsRepo.update).toHaveBeenCalledWith(
{ id: 123 },
{ status: TransactionStatus.ARCHIVED },
);
expect(result).toBe(true);
expect(notifySyncIndicators).toHaveBeenCalledWith(
notificationsService,
transaction.id,
TransactionStatus.ARCHIVED,
);
expect(notifyTransactionAction).toHaveBeenCalledWith(notificationsService);
expect(emitExecuteTranasction).toHaveBeenCalledWith(chainService, {
id: transaction.id,
status: transaction.status,
transactionBytes: transaction.transactionBytes.toString('hex'),
mirrorNetwork: transaction.mirrorNetwork,
validStart: transaction.validStart,
});
});
});

Expand Down Expand Up @@ -936,6 +943,7 @@ describe('TransactionsService', () => {
);
});
});

describe('shouldApproveTransaction', () => {
beforeEach(() => {
jest.resetAllMocks();
Expand Down
Loading

0 comments on commit 8abd3c3

Please sign in to comment.