fix: sql transactional consistency bug with fetching chaintip in various areas (#1853)

Author: zone117x

src/api/controllers/cache-controller.ts

@@ -252,7 +252,7 @@ async function calculateETag(
   switch (etagType) {
     case ETagType.chainTip:
       try {
-        const chainTip = await db.getChainTip();
+        const chainTip = await db.getChainTip(db.sql);
         if (chainTip.block_height === 0) {
           // This should never happen unless the API is serving requests before it has synced any
           // blocks.

src/api/routes/status.ts

@@ -19,7 +19,7 @@ export function createStatusRouter(db: PgStore): express.Router {
         response.pox_v2_unlock_height = poxForceUnlockHeights.result.pox2UnlockHeight as number;
         response.pox_v3_unlock_height = poxForceUnlockHeights.result.pox3UnlockHeight as number;
       }
-      const chainTip = await db.getChainTip();
+      const chainTip = await db.getChainTip(db.sql);
       if (chainTip.block_height > 0) {
         response.chain_tip = {
           block_height: chainTip.block_height,

src/datastore/pg-store.ts

@@ -204,8 +204,8 @@ export class PgStore extends BasePgStore {
     });
   }
 
-  async getChainTip(): Promise<DbChainTip> {
-    const tipResult = await this.sql<DbChainTip[]>`SELECT * FROM chain_tip`;
+  async getChainTip(sql: PgSqlClient): Promise<DbChainTip> {
+    const tipResult = await sql<DbChainTip[]>`SELECT * FROM chain_tip`;
     const tip = tipResult[0];
     return {
       block_height: tip?.block_height ?? 0,
@@ -607,7 +607,7 @@ export class PgStore extends BasePgStore {
 
   async getUnanchoredTxsInternal(sql: PgSqlClient): Promise<{ txs: DbTx[] }> {
     // Get transactions that have been streamed in microblocks but not yet accepted or rejected in an anchor block.
-    const { block_height } = await this.getChainTip();
+    const { block_height } = await this.getChainTip(sql);
     const unanchoredBlockHeight = block_height + 1;
     const query = await sql<ContractTxQueryResult[]>`
       SELECT ${unsafeCols(sql, [...TX_COLUMNS, abiColumn()])}
@@ -1402,7 +1402,7 @@ export class PgStore extends BasePgStore {
     sql: PgSqlClient,
     { includeUnanchored }: { includeUnanchored: boolean }
   ): Promise<number> {
-    const chainTip = await this.getChainTip();
+    const chainTip = await this.getChainTip(sql);
     if (includeUnanchored) {
       return chainTip.block_height + 1;
     } else {
@@ -2142,7 +2142,7 @@ export class PgStore extends BasePgStore {
 
   async getStxBalanceAtBlock(stxAddress: string, blockHeight: number): Promise<DbStxBalance> {
     return await this.sqlTransaction(async sql => {
-      const chainTip = await this.getChainTip();
+      const chainTip = await this.getChainTip(sql);
       const blockHeightToQuery =
         blockHeight > chainTip.block_height ? chainTip.block_height : blockHeight;
       const blockQuery = await this.getBlockByHeightInternal(sql, blockHeightToQuery);

src/datastore/pg-write-store.ts

@@ -183,7 +183,7 @@ export class PgWriteStore extends PgStore {
     let batchedTxData: DataStoreTxEventData[] = [];
 
     await this.sqlWriteTransaction(async sql => {
-      const chainTip = await this.getChainTip();
+      const chainTip = await this.getChainTip(sql);
       await this.handleReorg(sql, data.block, chainTip.block_height);
       const isCanonical = data.block.block_height > chainTip.block_height;
       if (!isCanonical) {
@@ -555,7 +555,7 @@ export class PgWriteStore extends PgStore {
       // Sanity check: ensure incoming microblocks have a `parent_index_block_hash` that matches the
       // API's current known canonical chain tip. We assume this holds true so incoming microblock
       // data is always treated as being built off the current canonical anchor block.
-      const chainTip = await this.getChainTip();
+      const chainTip = await this.getChainTip(sql);
       const nonCanonicalMicroblock = data.microblocks.find(
         mb => mb.parent_index_block_hash !== chainTip.index_block_hash
       );
@@ -1797,7 +1797,7 @@ export class PgWriteStore extends PgStore {
   async updateMempoolTxs({ mempoolTxs: txs }: { mempoolTxs: DbMempoolTxRaw[] }): Promise<void> {
     const updatedTxIds: string[] = [];
     await this.sqlWriteTransaction(async sql => {
-      const chainTip = await this.getChainTip();
+      const chainTip = await this.getChainTip(sql);
       updatedTxIds.push(...(await this.insertDbMempoolTxs(txs, chainTip, sql)));
     });
     if (!this.isEventReplay) {

src/event-stream/event-server.ts

@@ -888,7 +888,7 @@ export async function startEventServer(opts: {
   if (ibdHeight) {
     app.use(IBD_PRUNABLE_ROUTES, async (req, res, next) => {
       try {
-        const chainTip = await db.getChainTip();
+        const chainTip = await db.getChainTip(db.sql);
         if (chainTip.block_height > ibdHeight) {
           next();
         } else {

src/tests-event-replay/import-export-tests.ts

@@ -28,7 +28,7 @@ describe('import/export tests', () => {
   test('event import and export cycle', async () => {
     // Import from mocknet TSV
     await importEventsFromTsv('src/tests-event-replay/tsv/mocknet.tsv', 'archival', true, true);
-    const chainTip = await db.getChainTip();
+    const chainTip = await db.getChainTip(db.sql);
     expect(chainTip.block_height).toBe(28);
     expect(chainTip.index_block_hash).toBe(
       '0x76cd67a65c0dfd5ea450bb9efe30da89fa125bfc077c953802f718353283a533'
@@ -50,7 +50,7 @@ describe('import/export tests', () => {
     // Re-import with exported TSV and check that chain tip matches.
     try {
       await importEventsFromTsv(`${tmpDir}/export.tsv`, 'archival', true, true);
-      const newChainTip = await db.getChainTip();
+      const newChainTip = await db.getChainTip(db.sql);
       expect(newChainTip.block_height).toBe(28);
       expect(newChainTip.index_block_hash).toBe(
         '0x76cd67a65c0dfd5ea450bb9efe30da89fa125bfc077c953802f718353283a533'
@@ -196,14 +196,14 @@ describe('IBD', () => {
     process.env.IBD_MODE_UNTIL_BLOCK = '1000';
     // TSV has 1 microblock message.
     await expect(getIbdInterceptCountFromTsvEvents()).resolves.toBe(1);
-    await expect(db.getChainTip()).resolves.toHaveProperty('block_height', 28);
+    await expect(db.getChainTip(db.sql)).resolves.toHaveProperty('block_height', 28);
   });
 
   test('IBD mode does NOT block certain API routes once the threshold number of blocks are ingested', async () => {
     process.env.IBD_MODE_UNTIL_BLOCK = '1';
     // Microblock processed normally.
     await expect(getIbdInterceptCountFromTsvEvents()).resolves.toBe(0);
-    await expect(db.getChainTip()).resolves.toHaveProperty('block_height', 28);
+    await expect(db.getChainTip(db.sql)).resolves.toHaveProperty('block_height', 28);
   });
 
   test('IBD mode covers prune mode', async () => {

src/tests-event-replay/poison-microblock-tests.ts

@@ -25,7 +25,7 @@ describe('poison microblock for height 80743', () => {
       true
     );
     const poisonTxId = '0x58ffe62029f94f7101b959536ea4953b9bce0ec3f6e2a06254c511bdd5cfa9e7';
-    const chainTip = await db.getChainTip();
+    const chainTip = await db.getChainTip(db.sql);
     // query the txs table and check the transaction type
     const searchResult = await db.searchHash({ hash: poisonTxId });
     let entityData: any;

src/tests/cache-control-tests.ts

@@ -321,7 +321,7 @@ describe('cache-control tests', () => {
       ],
     });
 
-    const chainTip2 = await db.getChainTip();
+    const chainTip2 = await db.getChainTip(db.sql);
     expect(chainTip2.block_hash).toBe(block1.block_hash);
     expect(chainTip2.block_height).toBe(block1.block_height);
     expect(chainTip2.index_block_hash).toBe(block1.index_block_hash);

src/tests/datastore-tests.ts

@@ -4101,7 +4101,7 @@ describe('postgres datastore', () => {
 
     const blockQuery1 = await db.getBlock({ hash: block2b.block_hash });
     expect(blockQuery1.result?.canonical).toBe(false);
-    const chainTip1 = await db.getChainTip();
+    const chainTip1 = await db.getChainTip(db.sql);
     expect(chainTip1).toEqual({
       block_hash: '0x33',
       block_height: 3,
@@ -4169,7 +4169,7 @@ describe('postgres datastore', () => {
     const blockQuery2 = await db.getBlock({ hash: block3b.block_hash });
     expect(blockQuery2.result?.canonical).toBe(false);
     // Chain tip doesn't change yet.
-    const chainTip2 = await db.getChainTip();
+    const chainTip2 = await db.getChainTip(db.sql);
     expect(chainTip2).toEqual({
       block_hash: '0x33',
       block_height: 3,
@@ -4220,7 +4220,7 @@ describe('postgres datastore', () => {
 
     const blockQuery3 = await db.getBlock({ hash: block3b.block_hash });
     expect(blockQuery3.result?.canonical).toBe(true);
-    const chainTip3 = await db.getChainTip();
+    const chainTip3 = await db.getChainTip(db.sql);
     expect(chainTip3).toEqual({
       block_count: 4,
       block_hash: '0x44bb',

src/tests/mempool-tests.ts

@@ -1666,7 +1666,7 @@ describe('mempool tests', () => {
     // Simulate the bug with a txs being in the mempool at confirmed at the same time by
     // directly inserting the mempool-tx and mined-tx, bypassing the normal update functions.
     await db.updateBlock(db.sql, dbBlock1);
-    const chainTip = await db.getChainTip();
+    const chainTip = await db.getChainTip(db.sql);
     await db.insertDbMempoolTxs([mempoolTx], chainTip, db.sql);
     await db.updateTx(db.sql, dbTx1);
 
@@ -1828,7 +1828,7 @@ describe('mempool tests', () => {
 
     await db.updateMempoolTxs({ mempoolTxs: [mempoolTx] });
 
-    let chainTip = await db.getChainTip();
+    let chainTip = await db.getChainTip(db.sql);
     expect(chainTip.mempool_tx_count).toBe(1);
 
     // Verify tx shows up in mempool (non-pruned)
@@ -1852,7 +1852,7 @@ describe('mempool tests', () => {
     expect(mempoolResult2.body.results).toHaveLength(0);
     const mempoolCount2 = await supertest(api.server).get(`/extended/v1/tx/mempool`);
     expect(mempoolCount2.body.total).toBe(0);
-    chainTip = await db.getChainTip();
+    chainTip = await db.getChainTip(db.sql);
     expect(chainTip.mempool_tx_count).toBe(0);
 
     // Re-broadcast mempool tx
@@ -1865,7 +1865,7 @@ describe('mempool tests', () => {
     expect(mempoolResult3.body.results[0].tx_id).toBe(txId);
     const mempoolCount3 = await supertest(api.server).get(`/extended/v1/tx/mempool`);
     expect(mempoolCount3.body.total).toBe(1);
-    chainTip = await db.getChainTip();
+    chainTip = await db.getChainTip(db.sql);
     expect(chainTip.mempool_tx_count).toBe(1);
 
     // Mine tx in block to prune from mempool
@@ -1898,7 +1898,7 @@ describe('mempool tests', () => {
     expect(mempoolResult4.body.results).toHaveLength(0);
     const mempoolCount4 = await supertest(api.server).get(`/extended/v1/tx/mempool`);
     expect(mempoolCount4.body.total).toBe(0);
-    chainTip = await db.getChainTip();
+    chainTip = await db.getChainTip(db.sql);
     expect(chainTip.mempool_tx_count).toBe(0);
 
     // Verify tx is mined
@@ -1931,7 +1931,7 @@ describe('mempool tests', () => {
     expect(mempoolResult5.body.results[0].tx_id).toBe(txId);
     const mempoolCount5 = await supertest(api.server).get(`/extended/v1/tx/mempool`);
     expect(mempoolCount5.body.total).toBe(1);
-    chainTip = await db.getChainTip();
+    chainTip = await db.getChainTip(db.sql);
     expect(chainTip.mempool_tx_count).toBe(1);
 
     // Re-broadcast mempool tx