From 3ad5c9a951e08d9c352a60b2f26cd4539b696df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20C=C3=A1rdenas?= Date: Wed, 6 Jul 2022 11:51:18 -0500 Subject: [PATCH 1/5] feat: add transaction etag type --- src/api/controllers/cache-controller.ts | 24 ++++++++++++++++++++--- src/api/routes/tx.ts | 14 +++++-------- src/datastore/common.ts | 8 +++----- src/datastore/postgres-store.ts | 26 +++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/api/controllers/cache-controller.ts b/src/api/controllers/cache-controller.ts index dbda1aed6d..72ca5768bd 100644 --- a/src/api/controllers/cache-controller.ts +++ b/src/api/controllers/cache-controller.ts @@ -1,6 +1,6 @@ import { RequestHandler, Request, Response } from 'express'; import * as prom from 'prom-client'; -import { logger } from '../../helpers'; +import { logger, normalizeHashString } from '../../helpers'; import { DataStore } from '../../datastore/common'; import { asyncHandler } from '../async-handler'; @@ -24,6 +24,8 @@ export enum ETagType { chainTip = 'chain_tip', /** ETag based on a digest of all pending mempool `tx_id`s. */ mempool = 'mempool', + /** ETag based on the status of a single transaction across the mempool or canonical chain. */ + transaction = 'transaction', } /** Value that means the ETag did get calculated but it is empty. */ @@ -166,7 +168,7 @@ async function checkETagCacheOK( etagType: ETagType ): Promise { const metrics = getETagMetrics(); - const etag = await calculateETag(db, etagType); + const etag = await calculateETag(db, etagType, req); if (!etag || etag === ETAG_EMPTY) { return; } @@ -240,7 +242,11 @@ export function getETagCacheHandler( return requestHandler; } -async function calculateETag(db: DataStore, etagType: ETagType): Promise { +async function calculateETag( + db: DataStore, + etagType: ETagType, + req: Request +): Promise { switch (etagType) { case ETagType.chainTip: const chainTip = await db.getUnanchoredChainTip(); @@ -261,5 +267,17 @@ async function calculateETag(db: DataStore, etagType: ETagType): Promise { const { tx_id } = req.params; if (!has0xPrefix(tx_id)) { @@ -309,20 +310,14 @@ export function createTxRouter(db: DataStore): express.Router { res.status(404).json({ error: `could not find transaction by ID ${tx_id}` }); return; } - // TODO: this validation needs fixed now that the mempool-tx and mined-tx types no longer overlap - /* - const schemaPath = require.resolve( - '@stacks/stacks-blockchain-api-types/entities/transactions/transaction.schema.json' - ); - await validate(schemaPath, txQuery.result); - */ + setETagCacheHeaders(res, ETagType.transaction); res.json(txQuery.result); }) ); - // TODO: Add cache headers. Impossible right now since this tx might be from a block or from the mempool. router.get( '/:tx_id/raw', + txIdCacheHandler, asyncHandler(async (req, res) => { const { tx_id } = req.params; if (!has0xPrefix(tx_id)) { @@ -336,6 +331,7 @@ export function createTxRouter(db: DataStore): express.Router { const response: GetRawTransactionResult = { raw_tx: bufferToHexPrefixString(rawTxQuery.result.raw_tx), }; + setETagCacheHeaders(res, ETagType.transaction); res.json(response); } else { res.status(404).json({ error: `could not find transaction by ID ${tx_id}` }); diff --git a/src/datastore/common.ts b/src/datastore/common.ts index 1632c5c9d2..0eae9b2bfd 100644 --- a/src/datastore/common.ts +++ b/src/datastore/common.ts @@ -13,11 +13,7 @@ import { TxPayloadTypeID, PostConditionAuthFlag, } from 'stacks-encoding-native-js'; -import { - AddressTokenOfferingLocked, - MempoolTransaction, - TransactionType, -} from '@stacks/stacks-blockchain-api-types'; +import { AddressTokenOfferingLocked, TransactionType } from '@stacks/stacks-blockchain-api-types'; import { getTxSenderAddress } from '../event-stream/reader'; import { RawTxQueryResult } from './postgres-store'; import { ChainID, ClarityAbi } from '@stacks/transactions'; @@ -865,6 +861,8 @@ export interface DataStore extends DataStoreEventEmitter { getRawTx(txId: string): Promise>; + getTxStatus(txId: string): Promise>; + /** * Returns a list of NFTs owned by the given principal filtered by optional `asset_identifiers`, * including optional transaction metadata. diff --git a/src/datastore/postgres-store.ts b/src/datastore/postgres-store.ts index c39c56f909..645c480182 100644 --- a/src/datastore/postgres-store.ts +++ b/src/datastore/postgres-store.ts @@ -103,6 +103,8 @@ import { TransactionType, AddressUnlockSchedule, Block, + MempoolTransactionStatus, + TransactionStatus, } from '@stacks/stacks-blockchain-api-types'; import { getTxTypeId } from '../api/controllers/db-controller'; import { isProcessableTokenMetadata } from '../token-metadata/helpers'; @@ -4053,6 +4055,30 @@ export class PgDataStore }); } + async getTxStatus(txId: string): Promise> { + return this.queryTx(async client => { + const chainResult = await client.query<{ status: number }>( + `SELECT status + FROM txs + WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, + [hexToBuffer(txId)] + ); + if (chainResult.rowCount > 0) { + return { found: true, result: chainResult.rows[0].status }; + } + const mempoolResult = await client.query<{ status: number }>( + `SELECT status + FROM mempool_txs + WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, + [hexToBuffer(txId)] + ); + if (mempoolResult.rowCount > 0) { + return { found: true, result: mempoolResult.rows[0].status }; + } + return { found: false } as const; + }); + } + async getMaxBlockHeight( client: ClientBase, { includeUnanchored }: { includeUnanchored: boolean } From 39e1fb4729c57d01aadb54702e030dfd4b1614b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20C=C3=A1rdenas?= Date: Wed, 6 Jul 2022 13:04:39 -0500 Subject: [PATCH 2/5] test: cache behavior --- src/api/controllers/cache-controller.ts | 6 +- src/api/controllers/db-controller.ts | 1 + src/api/routes/tx.ts | 6 +- src/datastore/postgres-store.ts | 2 +- src/tests/cache-control-tests.ts | 131 ++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 7 deletions(-) diff --git a/src/api/controllers/cache-controller.ts b/src/api/controllers/cache-controller.ts index 72ca5768bd..acc2164340 100644 --- a/src/api/controllers/cache-controller.ts +++ b/src/api/controllers/cache-controller.ts @@ -269,8 +269,8 @@ async function calculateETag( return digest.result.digest; case ETagType.transaction: - const { txId } = req.params; - const normalizedTxId = normalizeHashString(txId); + const { tx_id } = req.params; + const normalizedTxId = normalizeHashString(tx_id); if (normalizedTxId === false) { return ETAG_EMPTY; } @@ -278,6 +278,6 @@ async function calculateETag( if (!status.found) { return ETAG_EMPTY; } - return `${normalizedTxId}-${status.result}`; + return `${normalizedTxId}:${status.result}`; } } diff --git a/src/api/controllers/db-controller.ts b/src/api/controllers/db-controller.ts index 80627de5c4..9a7409c6c0 100644 --- a/src/api/controllers/db-controller.ts +++ b/src/api/controllers/db-controller.ts @@ -159,6 +159,7 @@ export function getTxStatusString( case DbTxStatus.DroppedTooExpensive: return 'dropped_too_expensive'; case DbTxStatus.DroppedStaleGarbageCollect: + case DbTxStatus.DroppedApiGarbageCollect: return 'dropped_stale_garbage_collect'; default: throw new Error(`Unexpected DbTxStatus: ${txStatus}`); diff --git a/src/api/routes/tx.ts b/src/api/routes/tx.ts index 7fdec2b248..d039f773d0 100644 --- a/src/api/routes/tx.ts +++ b/src/api/routes/tx.ts @@ -67,7 +67,7 @@ export function createTxRouter(db: DataStore): express.Router { const cacheHandler = getETagCacheHandler(db); const mempoolCacheHandler = getETagCacheHandler(db, ETagType.mempool); - const txIdCacheHandler = getETagCacheHandler(db, ETagType.transaction); + const txCacheHandler = getETagCacheHandler(db, ETagType.transaction); router.get( '/', @@ -288,7 +288,7 @@ export function createTxRouter(db: DataStore): express.Router { router.get( '/:tx_id', - txIdCacheHandler, + txCacheHandler, asyncHandler(async (req, res, next) => { const { tx_id } = req.params; if (!has0xPrefix(tx_id)) { @@ -317,7 +317,7 @@ export function createTxRouter(db: DataStore): express.Router { router.get( '/:tx_id/raw', - txIdCacheHandler, + txCacheHandler, asyncHandler(async (req, res) => { const { tx_id } = req.params; if (!has0xPrefix(tx_id)) { diff --git a/src/datastore/postgres-store.ts b/src/datastore/postgres-store.ts index 645c480182..16dd25eded 100644 --- a/src/datastore/postgres-store.ts +++ b/src/datastore/postgres-store.ts @@ -4069,7 +4069,7 @@ export class PgDataStore const mempoolResult = await client.query<{ status: number }>( `SELECT status FROM mempool_txs - WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, + WHERE tx_id = $1`, [hexToBuffer(txId)] ); if (mempoolResult.rowCount > 0) { diff --git a/src/tests/cache-control-tests.ts b/src/tests/cache-control-tests.ts index 0f6b3cabfa..3b2db020df 100644 --- a/src/tests/cache-control-tests.ts +++ b/src/tests/cache-control-tests.ts @@ -425,6 +425,137 @@ describe('cache-control tests', () => { expect(request8.headers['etag']).toBeUndefined(); }); + test('adaptive cache handler', async () => { + const txId1 = '0x0153a41ed24a0e1d32f66ea98338df09f942571ca66359e28bdca79ccd0305cf'; + const txId2 = '0xfb4bfc274007825dfd2d8f6c3f429407016779e9954775f82129108282d4c4ce'; + + const block1 = new TestBlockBuilder({ + block_height: 1, + index_block_hash: '0x01', + }) + .addTx() + .build(); + await db.update(block1); + + // No tx yet. + const request1 = await supertest(api.server).get(`/extended/v1/tx/${txId1}`); + expect(request1.status).toBe(404); + expect(request1.type).toBe('application/json'); + + // Add mempool tx. + const mempoolTx1 = testMempoolTx({ tx_id: txId1 }); + await db.updateMempoolTxs({ mempoolTxs: [mempoolTx1] }); + + // Valid mempool ETag. + const request2 = await supertest(api.server).get(`/extended/v1/tx/${txId1}`); + expect(request2.status).toBe(200); + expect(request2.type).toBe('application/json'); + expect(request2.headers['etag']).toBeTruthy(); + const etag1 = request2.headers['etag']; + + // Cache works with valid ETag. + const request3 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag1); + expect(request3.status).toBe(304); + expect(request3.text).toBe(''); + + // Mine the same tx into a block + const block2 = new TestBlockBuilder({ + block_height: 2, + index_block_hash: '0x02', + parent_index_block_hash: '0x01', + }) + .addTx({ tx_id: txId1 }) + .build(); + await db.update(block2); + + // Cache no longer works with mempool ETag but we get updated ETag. + const request4 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag1); + expect(request4.status).toBe(200); + expect(request4.headers['etag']).toBeTruthy(); + const etag2 = request4.headers['etag']; + + // Cache works with new ETag. + const request5 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag2); + expect(request5.status).toBe(304); + expect(request5.text).toBe(''); + + // No tx #2 yet. + const request6 = await supertest(api.server).get(`/extended/v1/tx/${txId2}`); + expect(request6.status).toBe(404); + expect(request6.type).toBe('application/json'); + + // Tx #2 directly into a block + const block3 = new TestBlockBuilder({ + block_height: 3, + index_block_hash: '0x03', + parent_index_block_hash: '0x02', + }) + .addTx({ tx_id: txId2 }) + .build(); + await db.update(block3); + + // Valid block ETag. + const request7 = await supertest(api.server).get(`/extended/v1/tx/${txId2}`); + expect(request7.status).toBe(200); + expect(request7.type).toBe('application/json'); + expect(request7.headers['etag']).toBeTruthy(); + const etag3 = request7.headers['etag']; + + // Cache works with valid ETag. + const request8 = await supertest(api.server) + .get(`/extended/v1/tx/${txId2}`) + .set('If-None-Match', etag3); + expect(request8.status).toBe(304); + expect(request8.text).toBe(''); + + // Oops, new blocks came, all txs before are non-canonical + const block2a = new TestBlockBuilder({ + block_height: 2, + index_block_hash: '0x02ff', + parent_index_block_hash: '0x01', + }) + .addTx({ tx_id: '0x1111' }) + .build(); + await db.update(block2a); + const block3a = new TestBlockBuilder({ + block_height: 3, + index_block_hash: '0x03ff', + parent_index_block_hash: '0x02ff', + }) + .addTx({ tx_id: '0x1112' }) + .build(); + await db.update(block3a); + const block4 = new TestBlockBuilder({ + block_height: 4, + index_block_hash: '0x04', + parent_index_block_hash: '0x03ff', + }) + .addTx({ tx_id: '0x1113' }) + .build(); + await db.update(block4); + + // Cache no longer works for tx #1. + const request9 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag2); + expect(request9.status).toBe(200); + expect(request9.headers['etag']).toBeTruthy(); + const etag4 = request9.headers['etag']; + + // Cache works again with new ETag. + const request10 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag4); + expect(request10.status).toBe(304); + expect(request10.text).toBe(''); + }); + afterEach(async () => { await api.terminate(); client.release(); From d9cff9c3ca46d7572b89919f1c349ed7809c7aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20C=C3=A1rdenas?= Date: Wed, 6 Jul 2022 15:04:17 -0500 Subject: [PATCH 3/5] feat: include index_block_hash in etag --- src/api/controllers/cache-controller.ts | 5 +++-- src/datastore/common.ts | 2 +- src/datastore/postgres-store.ts | 21 ++++++++++++++++----- src/tests/cache-control-tests.ts | 21 ++++++++++++++++++++- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/api/controllers/cache-controller.ts b/src/api/controllers/cache-controller.ts index acc2164340..27485a223e 100644 --- a/src/api/controllers/cache-controller.ts +++ b/src/api/controllers/cache-controller.ts @@ -1,6 +1,6 @@ import { RequestHandler, Request, Response } from 'express'; import * as prom from 'prom-client'; -import { logger, normalizeHashString } from '../../helpers'; +import { bufferToHexPrefixString, logger, normalizeHashString } from '../../helpers'; import { DataStore } from '../../datastore/common'; import { asyncHandler } from '../async-handler'; @@ -278,6 +278,7 @@ async function calculateETag( if (!status.found) { return ETAG_EMPTY; } - return `${normalizedTxId}:${status.result}`; + const indexBlockHash = bufferToHexPrefixString(status.result.index_block_hash); + return `${normalizedTxId}:${indexBlockHash}:${status.result.status}`; } } diff --git a/src/datastore/common.ts b/src/datastore/common.ts index 0eae9b2bfd..dcc34499ae 100644 --- a/src/datastore/common.ts +++ b/src/datastore/common.ts @@ -861,7 +861,7 @@ export interface DataStore extends DataStoreEventEmitter { getRawTx(txId: string): Promise>; - getTxStatus(txId: string): Promise>; + getTxStatus(txId: string): Promise>; /** * Returns a list of NFTs owned by the given principal filtered by optional `asset_identifiers`, diff --git a/src/datastore/postgres-store.ts b/src/datastore/postgres-store.ts index 16dd25eded..03fe8c8f63 100644 --- a/src/datastore/postgres-store.ts +++ b/src/datastore/postgres-store.ts @@ -4055,16 +4055,24 @@ export class PgDataStore }); } - async getTxStatus(txId: string): Promise> { + async getTxStatus( + txId: string + ): Promise> { return this.queryTx(async client => { - const chainResult = await client.query<{ status: number }>( - `SELECT status + const chainResult = await client.query<{ status: number; index_block_hash: Buffer }>( + `SELECT status, index_block_hash FROM txs WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, [hexToBuffer(txId)] ); if (chainResult.rowCount > 0) { - return { found: true, result: chainResult.rows[0].status }; + return { + found: true, + result: { + status: chainResult.rows[0].status, + index_block_hash: chainResult.rows[0].index_block_hash, + }, + }; } const mempoolResult = await client.query<{ status: number }>( `SELECT status @@ -4073,7 +4081,10 @@ export class PgDataStore [hexToBuffer(txId)] ); if (mempoolResult.rowCount > 0) { - return { found: true, result: mempoolResult.rows[0].status }; + return { + found: true, + result: { status: mempoolResult.rows[0].status, index_block_hash: Buffer.from([]) }, + }; } return { found: false } as const; }); diff --git a/src/tests/cache-control-tests.ts b/src/tests/cache-control-tests.ts index 3b2db020df..14d6beb0f6 100644 --- a/src/tests/cache-control-tests.ts +++ b/src/tests/cache-control-tests.ts @@ -425,7 +425,7 @@ describe('cache-control tests', () => { expect(request8.headers['etag']).toBeUndefined(); }); - test('adaptive cache handler', async () => { + test('transaction cache control', async () => { const txId1 = '0x0153a41ed24a0e1d32f66ea98338df09f942571ca66359e28bdca79ccd0305cf'; const txId2 = '0xfb4bfc274007825dfd2d8f6c3f429407016779e9954775f82129108282d4c4ce'; @@ -554,6 +554,25 @@ describe('cache-control tests', () => { .set('If-None-Match', etag4); expect(request10.status).toBe(304); expect(request10.text).toBe(''); + + // Mine tx in a new block + const block5 = new TestBlockBuilder({ + block_height: 5, + index_block_hash: '0x05', + parent_index_block_hash: '0x04', + }) + .addTx({ tx_id: txId1 }) + .build(); + await db.update(block5); + + // Make sure old cache for confirmed tx doesn't work, because the index_block_hash has changed. + const request11 = await supertest(api.server) + .get(`/extended/v1/tx/${txId1}`) + .set('If-None-Match', etag2); + expect(request11.status).toBe(200); + expect(request11.headers['etag']).toBeTruthy(); + const etag5 = request11.headers['etag']; + expect(etag2).not.toBe(etag5); }); afterEach(async () => { From b60336c4552c8fa253865891955324e579ae397e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20C=C3=A1rdenas?= Date: Wed, 6 Jul 2022 16:18:46 -0500 Subject: [PATCH 4/5] feat: add microblock_hash to etag --- src/api/controllers/cache-controller.ts | 9 +++++++-- src/datastore/common.ts | 8 +++++++- src/datastore/postgres-store.ts | 16 ++++++++++------ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/api/controllers/cache-controller.ts b/src/api/controllers/cache-controller.ts index 27485a223e..d3ba27b860 100644 --- a/src/api/controllers/cache-controller.ts +++ b/src/api/controllers/cache-controller.ts @@ -278,7 +278,12 @@ async function calculateETag( if (!status.found) { return ETAG_EMPTY; } - const indexBlockHash = bufferToHexPrefixString(status.result.index_block_hash); - return `${normalizedTxId}:${indexBlockHash}:${status.result.status}`; + const elements: string[] = [ + normalizedTxId, + bufferToHexPrefixString(status.result.index_block_hash), + bufferToHexPrefixString(status.result.microblock_hash), + status.result.status.toString(), + ]; + return elements.join(':'); } } diff --git a/src/datastore/common.ts b/src/datastore/common.ts index dcc34499ae..e257b9901c 100644 --- a/src/datastore/common.ts +++ b/src/datastore/common.ts @@ -611,6 +611,12 @@ export interface DbChainTip { microblockSequence?: number; } +export interface DbTxGlobalStatus { + status: DbTxStatus; + index_block_hash: Buffer; + microblock_hash: Buffer; +} + export interface DataStore extends DataStoreEventEmitter { storeRawEventRequest(eventPath: string, payload: string): Promise; getSubdomainResolver(name: { name: string }): Promise>; @@ -861,7 +867,7 @@ export interface DataStore extends DataStoreEventEmitter { getRawTx(txId: string): Promise>; - getTxStatus(txId: string): Promise>; + getTxStatus(txId: string): Promise>; /** * Returns a list of NFTs owned by the given principal filtered by optional `asset_identifiers`, diff --git a/src/datastore/postgres-store.ts b/src/datastore/postgres-store.ts index 03fe8c8f63..b1862214ac 100644 --- a/src/datastore/postgres-store.ts +++ b/src/datastore/postgres-store.ts @@ -97,6 +97,7 @@ import { NftHoldingInfoWithTxMetadata, NftEventWithTxMetadata, DbAssetEventTypeId, + DbTxGlobalStatus, } from './common'; import { AddressTokenOfferingLocked, @@ -4055,12 +4056,10 @@ export class PgDataStore }); } - async getTxStatus( - txId: string - ): Promise> { + async getTxStatus(txId: string): Promise> { return this.queryTx(async client => { - const chainResult = await client.query<{ status: number; index_block_hash: Buffer }>( - `SELECT status, index_block_hash + const chainResult = await client.query( + `SELECT status, index_block_hash, microblock_hash FROM txs WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, [hexToBuffer(txId)] @@ -4071,6 +4070,7 @@ export class PgDataStore result: { status: chainResult.rows[0].status, index_block_hash: chainResult.rows[0].index_block_hash, + microblock_hash: chainResult.rows[0].microblock_hash, }, }; } @@ -4083,7 +4083,11 @@ export class PgDataStore if (mempoolResult.rowCount > 0) { return { found: true, - result: { status: mempoolResult.rows[0].status, index_block_hash: Buffer.from([]) }, + result: { + status: mempoolResult.rows[0].status, + index_block_hash: Buffer.from([]), + microblock_hash: Buffer.from([]), + }, }; } return { found: false } as const; From 29b4b6e49b4e088982b87fb450750578ca22a706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20C=C3=A1rdenas?= Date: Thu, 7 Jul 2022 09:59:09 -0500 Subject: [PATCH 5/5] chore: add limit 1 to queries --- src/datastore/postgres-store.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/datastore/postgres-store.ts b/src/datastore/postgres-store.ts index b1862214ac..aa8d7a5007 100644 --- a/src/datastore/postgres-store.ts +++ b/src/datastore/postgres-store.ts @@ -4061,7 +4061,8 @@ export class PgDataStore const chainResult = await client.query( `SELECT status, index_block_hash, microblock_hash FROM txs - WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE`, + WHERE tx_id = $1 AND canonical = TRUE AND microblock_canonical = TRUE + LIMIT 1`, [hexToBuffer(txId)] ); if (chainResult.rowCount > 0) { @@ -4077,7 +4078,8 @@ export class PgDataStore const mempoolResult = await client.query<{ status: number }>( `SELECT status FROM mempool_txs - WHERE tx_id = $1`, + WHERE tx_id = $1 + LIMIT 1`, [hexToBuffer(txId)] ); if (mempoolResult.rowCount > 0) {