From d763da6360d7d61676057a88d93dfc214d89f5a9 Mon Sep 17 00:00:00 2001 From: Filippo Vecchiato Date: Tue, 10 Sep 2024 16:22:23 +0200 Subject: [PATCH] Moves the LRUcache to Controller level --- src/controllers/AbstractController.ts | 1 + src/controllers/blocks/BlocksController.ts | 48 +++++++-- .../blocks/BlocksExtrinsicsController.ts | 21 +++- .../blocks/BlocksRawExtrinsicsController.ts | 2 +- src/services/blocks/BlocksService.spec.ts | 100 ++---------------- src/services/blocks/BlocksService.ts | 25 +---- 6 files changed, 70 insertions(+), 127 deletions(-) diff --git a/src/controllers/AbstractController.ts b/src/controllers/AbstractController.ts index b45a1b36c..dfbc5a3f1 100644 --- a/src/controllers/AbstractController.ts +++ b/src/controllers/AbstractController.ts @@ -50,6 +50,7 @@ type SidecarRequestHandler = */ export default abstract class AbstractController { private _router: Router = express.Router(); + constructor( protected api: ApiPromise, private _path: string, diff --git a/src/controllers/blocks/BlocksController.ts b/src/controllers/blocks/BlocksController.ts index dc3ae9c94..a3de0ce16 100644 --- a/src/controllers/blocks/BlocksController.ts +++ b/src/controllers/blocks/BlocksController.ts @@ -18,6 +18,7 @@ import type { ApiPromise } from '@polkadot/api'; import { isHex } from '@polkadot/util'; import { RequestHandler, Response } from 'express'; import { BadRequest } from 'http-errors'; +import type { LRUCache } from 'lru-cache'; import type client from 'prom-client'; import { validateBoolean } from '../../middleware/validate'; @@ -102,16 +103,14 @@ import AbstractController from '../AbstractController'; * - `OnFinalize`: https://crates.parity.io/frame_support/traits/trait.OnFinalize.html */ export default class BlocksController extends AbstractController { + private blockStore: LRUCache; constructor( api: ApiPromise, private readonly options: ControllerOptions, ) { - super( - api, - '/blocks', - new BlocksService(api, options.minCalcFeeRuntime, options.blockStore, options.hasQueryFeeApi), - ); + super(api, '/blocks', new BlocksService(api, options.minCalcFeeRuntime, options.hasQueryFeeApi)); this.initRoutes(); + this.blockStore = options.blockStore; } protected initRoutes(): void { @@ -205,10 +204,28 @@ export default class BlocksController extends AbstractController checkDecodedXcm: decodedXcmMsgsArg, paraId: paraIdArg, }; + // Create a key for the cache that is a concatenation of the block hash and all the query params included in the request + const cacheKey = + hash.toString() + + Number(eventDocs) + + Number(extrinsicDocs) + + Number(options.checkFinalized) + + Number(noFees) + + Number(options.checkDecodedXcm) + + Number(paraId); + + const isBlockCached = this.blockStore.get(cacheKey); + + if (isBlockCached) { + BlocksController.sanitizedSend(res, isBlockCached); + return; + } const historicApi = await this.api.at(hash); const block = await this.service.fetchBlock(hash, historicApi, options); + this.blockStore.set(cacheKey, block); + BlocksController.sanitizedSend(res, block); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -263,9 +280,28 @@ export default class BlocksController extends AbstractController paraId: paraIdArg, }; + // Create a key for the cache that is a concatenation of the block hash and all the query params included in the request + + const cacheKey = + hash.toString() + + Number(eventDocs) + + Number(extrinsicDocs) + + Number(options.checkFinalized) + + Number(noFees) + + Number(options.checkDecodedXcm) + + Number(paraId); + + const isBlockCached = this.blockStore.get(cacheKey); + + if (isBlockCached) { + BlocksController.sanitizedSend(res, isBlockCached); + return; + } // HistoricApi to fetch any historic information that doesnt include the current runtime const historicApi = await this.api.at(hash); const block = await this.service.fetchBlock(hash, historicApi, options); + + this.blockStore.set(cacheKey, block); // We set the last param to true because we haven't queried the finalizedHead BlocksController.sanitizedSend(res, block); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access @@ -348,7 +384,7 @@ export default class BlocksController extends AbstractController } const blocks = (await Promise.all(blocksPromise)) as IBlock[]; - + blocksPromise.length = 0; /** * Sort blocks from least to greatest. */ diff --git a/src/controllers/blocks/BlocksExtrinsicsController.ts b/src/controllers/blocks/BlocksExtrinsicsController.ts index b1f31d864..03e377cad 100644 --- a/src/controllers/blocks/BlocksExtrinsicsController.ts +++ b/src/controllers/blocks/BlocksExtrinsicsController.ts @@ -16,6 +16,8 @@ import type { ApiPromise } from '@polkadot/api'; import { RequestHandler } from 'express'; +import { LRUCache } from 'lru-cache'; +import { IBlock } from 'src/types/responses'; import { BlocksService } from '../../services'; import type { ControllerOptions } from '../../types/chains-config'; @@ -23,13 +25,15 @@ import type { INumberParam } from '../../types/requests'; import AbstractController from '../AbstractController'; export default class BlocksExtrinsicsController extends AbstractController { + private blockStore: LRUCache; constructor(api: ApiPromise, options: ControllerOptions) { super( api, '/blocks/:blockId/extrinsics', - new BlocksService(api, options.minCalcFeeRuntime, options.blockStore, options.hasQueryFeeApi), + new BlocksService(api, options.minCalcFeeRuntime, options.hasQueryFeeApi), ); this.initRoutes(); + this.blockStore = options.blockStore; } protected initRoutes(): void { @@ -62,10 +66,23 @@ export default class BlocksExtrinsicsController extends AbstractController Promise>; -// LRU cache used to cache blocks -// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -const cache = new LRUCache({ max: 2 }) as LRUCache; - // Block Service -const blocksService = new BlocksService(mockApi, 0, cache, new QueryFeeDetailsCache(null, null)); +const blocksService = new BlocksService(mockApi, 0, new QueryFeeDetailsCache(null, null)); describe('BlocksService', () => { describe('fetchBlock', () => { it('works when ApiPromise works (block 789629)', async () => { - // Reset LRU cache - cache.clear(); - // fetchBlock options const options = { eventDocs: true, @@ -160,8 +149,6 @@ describe('BlocksService', () => { }); it('throws when an extrinsic is undefined', async () => { - // Reset LRU cache - cache.clear(); // Create a block with undefined as the first extrinisic and the last extrinsic removed const mockBlock789629BadExt = polkadotRegistry.createType('Block', block789629); @@ -194,8 +181,6 @@ describe('BlocksService', () => { }); it('Returns the finalized tag as undefined when omitFinalizedTag equals true', async () => { - // Reset LRU cache - cache.clear(); // fetchBlock options const options = { eventDocs: true, @@ -215,9 +200,6 @@ describe('BlocksService', () => { }); describe('BlocksService.parseGenericCall', () => { - // Reset LRU cache - cache.clear(); - const transfer = createCall('balances', 'transfer', { value: 12, dest: kusamaRegistry.createType('AccountId', '14E5nqKAp3oAJcmzgZhUD2RcptBeUBScxKHgJKU4HPNcKVf3'), // Bob @@ -351,9 +333,6 @@ describe('BlocksService', () => { const blockNumber = polkadotRegistry.createType('Compact', 789629); it('Returns false when queried blockId is not canonical', async () => { - // Reset LRU cache - cache.clear(); - const getHeader = (_hash: Hash) => Promise.resolve().then(() => mockForkedBlock789629.header); const getBlockHash = (_zero: number) => Promise.resolve().then(() => finalizedHead); @@ -378,12 +357,7 @@ describe('BlocksService', () => { }); it('Returns true when queried blockId is canonical', async () => { - const blocksService = new BlocksService( - mockApi, - 0, - new LRUCache({ max: 2 }), - new QueryFeeDetailsCache(null, null), - ); + const blocksService = new BlocksService(mockApi, 0, new QueryFeeDetailsCache(null, null)); expect(await blocksService['isFinalizedBlock'](mockApi, blockNumber, finalizedHead, finalizedHead, true)).toEqual( true, ); @@ -404,9 +378,6 @@ describe('BlocksService', () => { }; it('Returns the correct extrinisics object for block 789629', async () => { - // Reset LRU cache - cache.clear(); - const block = await blocksService.fetchBlock(blockHash789629, mockHistoricApi, options); /** @@ -419,9 +390,6 @@ describe('BlocksService', () => { }); it("Throw an error when `extrinsicIndex` doesn't exist", async () => { - // Reset LRU cache - cache.clear(); - const block = await blocksService.fetchBlock(blockHash789629, mockHistoricApi, options); expect(() => { @@ -460,60 +428,18 @@ describe('BlocksService', () => { }, }; it('Returns the correct summary for the latest block', async () => { - // Reset LRU cache - cache.clear(); - const blockSummary = await blocksService.fetchBlockHeader(blockHash789629); expect(sanitizeNumbers(blockSummary)).toStrictEqual(expectedResponse); }); it('Returns the correct summary for the given block number', async () => { - // Reset LRU cache - cache.clear(); - const blockSummary = await blocksService.fetchBlockHeader(); expect(sanitizeNumbers(blockSummary)).toStrictEqual(expectedResponse); }); }); - describe('Block LRUcache', () => { - // fetchBlock options - const options = { - eventDocs: true, - extrinsicDocs: true, - checkFinalized: false, - queryFinalizedHead: false, - omitFinalizedTag: false, - noFees: false, - checkDecodedXcm: false, - paraId: undefined, - }; - - it('Should correctly store the most recent queried blocks', async () => { - // Reset LRU cache - cache.clear(); - - await blocksService.fetchBlock(blockHash789629, mockHistoricApi, options); - await blocksService.fetchBlock(blockHash20000, mockHistoricApi, options); - - expect(cache.size).toBe(2); - }); - - it('Should have a max of 2 blocks within the LRUcache, and should save the most recent and remove the oldest block', async () => { - // Reset LRU cache - cache.clear(); - - await blocksService.fetchBlock(blockHash789629, mockHistoricApi, options); - await blocksService.fetchBlock(blockHash20000, mockHistoricApi, options); - await blocksService.fetchBlock(blockHash100000, mockHistoricApi, options); - - expect(cache.get(blockHash789629.toString())).toBe(undefined); - expect(cache.size).toBe(2); - }); - }); - describe('fetchBlockRaw', () => { it('works when ApiPromise works (block 789629)', async () => { expect(sanitizeNumbers(await blocksService.fetchBlockRaw(blockHash789629))).toMatchObject(blocks789629Raw); @@ -521,9 +447,6 @@ describe('BlocksService', () => { }); describe('BlockService.decodedXcmMsgsArg', () => { - // Reset LRU cache - cache.clear(); - const validatorsAt = (_hash: Hash) => Promise.resolve().then(() => polkadotRegistryV1000001.createType('Vec', validators18468942Hex)); @@ -578,7 +501,7 @@ describe('BlocksService', () => { } as unknown as ApiPromise; // Block Service - const blocksServiceXCM = new BlocksService(mockApiXCM, 0, cache, new QueryFeeDetailsCache(null, null)); + const blocksServiceXCM = new BlocksService(mockApiXCM, 0, new QueryFeeDetailsCache(null, null)); it('Should give back two decoded upward XCM messages for Polkadot block 18468942, one for paraId=2000 and one for paraId=2012', async () => { // fetchBlock options @@ -599,9 +522,6 @@ describe('BlocksService', () => { }); it('Should give back one decoded upward XCM message for Polkadot block 18468942 only for paraId=2000', async () => { - // Reset LRU cache - cache.clear(); - // fetchBlock options const options = { eventDocs: true, @@ -620,9 +540,6 @@ describe('BlocksService', () => { }); it('Should give back two decoded XCM messages, one horizontal and one downward, for Kusama Asset Hub block 3356195', async () => { - // Reset LRU cache - cache.clear(); - // fetchBlock options const options = { eventDocs: true, @@ -691,15 +608,13 @@ describe('BlocksService', () => { } as unknown as ApiPromise; // Block Service - const blocksServiceXCM = new BlocksService(mockApiXCM, 0, cache, new QueryFeeDetailsCache(null, null)); + const blocksServiceXCM = new BlocksService(mockApiXCM, 0, new QueryFeeDetailsCache(null, null)); const block = await blocksServiceXCM.fetchBlock(blockHash3356195, mockHistoricApiXCM, options); expect(sanitizeNumbers(block)).toMatchObject(block3356195Response); }); it('Should give back one of the two available horizontal messages, the one for paraId 2087 for Kusama Asset Hub block 6202603', async () => { - // Reset LRU cache - cache.clear(); // fetchBlock options const options = { eventDocs: true, @@ -767,16 +682,13 @@ describe('BlocksService', () => { } as unknown as ApiPromise; // Block Service - const blocksServiceXCM = new BlocksService(mockApiXCM, 0, cache, new QueryFeeDetailsCache(null, null)); + const blocksServiceXCM = new BlocksService(mockApiXCM, 0, new QueryFeeDetailsCache(null, null)); const block = await blocksServiceXCM.fetchBlock(blockHash6202603, mockHistoricApiXCM, options); expect(sanitizeNumbers(block)).toMatchObject(block6202603pId2087Response); }); it('Should give back two decoded horizontal XCM messages (with different origin & destination paraId) that are `in transit` in Polkadot Relay block 19772575', async () => { - // Reset LRU cache - cache.clear(); - // fetchBlock options const options = { eventDocs: true, @@ -843,7 +755,7 @@ describe('BlocksService', () => { } as unknown as ApiPromise; // Block Service - const blocksServiceXCM = new BlocksService(mockApiXCM, 0, cache, new QueryFeeDetailsCache(null, null)); + const blocksServiceXCM = new BlocksService(mockApiXCM, 0, new QueryFeeDetailsCache(null, null)); const block = await blocksServiceXCM.fetchBlock(blockHash19772575, mockHistoricApiXCM, options); expect(sanitizeNumbers(block)).toMatchObject(block19772575Response); diff --git a/src/services/blocks/BlocksService.ts b/src/services/blocks/BlocksService.ts index 48b70be96..b37258365 100644 --- a/src/services/blocks/BlocksService.ts +++ b/src/services/blocks/BlocksService.ts @@ -41,7 +41,6 @@ import { blake2AsU8a } from '@polkadot/util-crypto'; import { calc_partial_fee } from '@substrate/calc'; import BN from 'bn.js'; import { BadRequest, InternalServerError } from 'http-errors'; -import type { LRUCache } from 'lru-cache'; import { QueryFeeDetailsCache } from '../../chains-config/cache'; import { @@ -96,7 +95,6 @@ export class BlocksService extends AbstractService { constructor( api: ApiPromise, private minCalcFeeRuntime: IOption, - private blockStore: LRUCache, private hasQueryFeeApi: QueryFeeDetailsCache, ) { super(api); @@ -123,24 +121,6 @@ export class BlocksService extends AbstractService { }: FetchBlockOptions, ): Promise { const { api } = this; - - // Create a key for the cache that is a concatenation of the block hash and all the query params included in the request - const cacheKey = - hash.toString() + - Number(eventDocs) + - Number(extrinsicDocs) + - Number(checkFinalized) + - Number(noFees) + - Number(checkDecodedXcm) + - Number(paraId); - - // Before making any api calls check the cache if the queried block exists - const isBlockCached = this.blockStore.get(cacheKey); - - if (isBlockCached && isBlockCached.finalized !== false) { - return isBlockCached; - } - const [{ block }, { specName, specVersion }, validators, events, finalizedHead] = await Promise.all([ api.rpc.chain.getBlock(hash), api.rpc.state.getRuntimeVersion(hash), @@ -223,6 +203,7 @@ export class BlocksService extends AbstractService { await Promise.all(feeTasks); + feeTasks.length = 0; const response = { number, hash, @@ -237,10 +218,6 @@ export class BlocksService extends AbstractService { finalized, decodedXcmMsgs, }; - - // Store the block in the cache - this.blockStore.set(cacheKey, response); - return response; }