Skip to content

Commit

Permalink
Clear transaction from cache immediately upon deletion (safe-global#1092
Browse files Browse the repository at this point in the history 
)

This immediately clears a deleted transaction from the cache in case an invalidation event is not received.

---

* Clear transaction from cache immediately upon deletion

* Fix existing test and add coverage to test cache clearing
  • Loading branch information
@iamacook
iamacook authored Jan 31, 2024
1 parent 194c500 commit 430a0e9
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 7 deletions.
11 changes: 10 additions & 1 deletion src/domain/safe/safe.repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,16 @@ export class SafeRepository implements ISafeRepository {
}): Promise<void> {
const transactionService =
await this.transactionApiManager.getTransactionApi(args.chainId);
return transactionService.deleteTransaction(args);
const { safe } = await transactionService.getMultisigTransaction(
args.safeTxHash,
);
await transactionService.deleteTransaction(args);

// Ensure transaction is removed from cache in case event is not received
Promise.all([
transactionService.clearMultisigTransaction(args.safeTxHash),
transactionService.clearMultisigTransactions(safe),
]);
}

async clearMultisigTransactions(args: {
Expand Down
84 changes: 78 additions & 6 deletions ...tes/transactions/__tests__/controllers/delete-transaction.transactions.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,19 @@ import { AccountDataSourceModule } from '@/datasources/account/account.datasourc
import { NetworkModule } from '@/datasources/network/network.module';
import { RequestScopedLoggingModule } from '@/logging/logging.module';
import { NetworkResponseError } from '@/datasources/network/entities/network.error.entity';
import {
multisigTransactionBuilder,
toJson as multisigToJson,
} from '@/domain/safe/entities/__tests__/multisig-transaction.builder';
import { CacheService } from '@/datasources/cache/cache.service.interface';
import { CacheDir } from '@/datasources/cache/entities/cache-dir.entity';
import { FakeCacheService } from '@/datasources/cache/__tests__/fake.cache.service';

describe('Delete Transaction - Transactions Controller (Unit', () => {
let app: INestApplication;
let safeConfigUrl: string;
let networkService: jest.MockedObjectDeep<INetworkService>;
let fakeCacheService: FakeCacheService;

beforeEach(async () => {
jest.clearAllMocks();
Expand All @@ -46,6 +54,7 @@ describe('Delete Transaction - Transactions Controller (Unit', () => {
const configurationService = moduleFixture.get(IConfigurationService);
safeConfigUrl = configurationService.get('safeConfig.baseUri');
networkService = moduleFixture.get(NetworkService);
fakeCacheService = moduleFixture.get(CacheService);

app = await new TestAppProvider().provide(moduleFixture);
await app.init();
Expand Down Expand Up @@ -75,7 +84,7 @@ describe('Delete Transaction - Transactions Controller (Unit', () => {

it('should delete a multisig transaction', async () => {
const chain = chainBuilder().build();
const safeTxHash = faker.string.hexadecimal({ length: 16 });
const tx = multisigTransactionBuilder().build();
const deleteTransactionDto: DeleteTransactionDto = {
signature: faker.string.hexadecimal({ length: 16 }),
};
Expand All @@ -84,39 +93,102 @@ describe('Delete Transaction - Transactions Controller (Unit', () => {
if (url === `${safeConfigUrl}/api/v1/chains/${chain.chainId}`) {
return Promise.resolve({ data: chain, status: 200 });
}
if (
url ===
`${chain.transactionService}/api/v1/multisig-transactions/${tx.safeTxHash}/`
) {
return Promise.resolve({ data: multisigToJson(tx), status: 200 });
}
fail(`No matching rule for url: ${url}`);
});
networkService.delete.mockImplementation((url) => {
if (
url === `${chain.transactionService}/api/v1/transactions/${safeTxHash}`
url ===
`${chain.transactionService}/api/v1/transactions/${tx.safeTxHash}`
) {
return Promise.resolve({ data: {}, status: 204 });
}
fail(`No matching rule for url: ${url}`);
});

await request(app.getHttpServer())
.delete(`/v1/chains/${chain.chainId}/transactions/${safeTxHash}`)
.delete(`/v1/chains/${chain.chainId}/transactions/${tx.safeTxHash}`)
.send(deleteTransactionDto)
.expect(200);
});

it('should clear the cache after deleting a multisig transaction', async () => {
const chain = chainBuilder().build();
const tx = multisigTransactionBuilder().build();
const deleteTransactionDto: DeleteTransactionDto = {
signature: faker.string.hexadecimal({ length: 16 }),
};

networkService.get.mockImplementation((url) => {
if (url === `${safeConfigUrl}/api/v1/chains/${chain.chainId}`) {
return Promise.resolve({ data: chain, status: 200 });
}
if (
url ===
`${chain.transactionService}/api/v1/multisig-transactions/${tx.safeTxHash}/`
) {
return Promise.resolve({ data: multisigToJson(tx), status: 200 });
}
fail(`No matching rule for url: ${url}`);
});
networkService.delete.mockImplementation((url) => {
if (
url ===
`${chain.transactionService}/api/v1/transactions/${tx.safeTxHash}`
) {
return Promise.resolve({ data: {}, status: 204 });
}
fail(`No matching rule for url: ${url}`);
});

await request(app.getHttpServer())
.delete(`/v1/chains/${chain.chainId}/transactions/${tx.safeTxHash}`)
.send(deleteTransactionDto)
.expect(200);

await expect(
fakeCacheService.get(
new CacheDir(
`${chain.chainId}_multisig_transaction_${tx.safeTxHash}`,
'',
),
),
).resolves.toBeUndefined();
await expect(
fakeCacheService.get(
new CacheDir(`${chain.chainId}_multisig_transactions_${tx.safe}`, ''),
),
).resolves.toBeUndefined();
});

it('should forward an error from the Transaction Service', async () => {
const chain = chainBuilder().build();
const safeTxHash = faker.string.hexadecimal({ length: 16 });
const deleteTransactionDto: DeleteTransactionDto = {
signature: faker.string.hexadecimal({ length: 16 }),
};

const tx = multisigTransactionBuilder().build();
networkService.get.mockImplementation((url) => {
if (url === `${safeConfigUrl}/api/v1/chains/${chain.chainId}`) {
return Promise.resolve({ data: chain, status: 200 });
}
if (
url ===
`${chain.transactionService}/api/v1/multisig-transactions/${tx.safeTxHash}/`
) {
return Promise.resolve({ data: multisigToJson(tx), status: 200 });
}
fail(`No matching rule for url: ${url}`);
});
networkService.delete.mockImplementation((url) => {
if (
url === `${chain.transactionService}/api/v1/transactions/${safeTxHash}`
url ===
`${chain.transactionService}/api/v1/transactions/${tx.safeTxHash}`
) {
return Promise.reject(
new NetworkResponseError(
Expand All @@ -132,7 +204,7 @@ describe('Delete Transaction - Transactions Controller (Unit', () => {
});

await request(app.getHttpServer())
.delete(`/v1/chains/${chain.chainId}/transactions/${safeTxHash}`)
.delete(`/v1/chains/${chain.chainId}/transactions/${tx.safeTxHash}`)
.send(deleteTransactionDto)
.expect(404)
.expect({
Expand Down

0 comments on commit 430a0e9

Please sign in to comment.