Skip to content

Commit

Permalink
fix: sql transactional consistency bug with fetching chaintip in vari…
Browse files Browse the repository at this point in the history
…ous areas (#1853)
  • Loading branch information
zone117x committed Feb 2, 2024
1 parent 2dd1de8 commit 07339c0
Show file tree
Hide file tree
Showing 11 changed files with 28 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/api/controllers/cache-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/api/routes/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions src/datastore/pg-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()])}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/datastore/pg-write-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/event-stream/event-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/tests-event-replay/import-export-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down Expand Up @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion src/tests-event-replay/poison-microblock-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/tests/cache-control-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/tests/datastore-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down
12 changes: 6 additions & 6 deletions src/tests/mempool-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/tests/microblock-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ describe('microblock tests', () => {
],
});

const chainTip1 = await db.getChainTip();
const chainTip1 = await db.getChainTip(db.sql);
expect(chainTip1.block_hash).toBe(block1.block_hash);
expect(chainTip1.block_height).toBe(block1.block_height);
expect(chainTip1.index_block_hash).toBe(block1.index_block_hash);
Expand Down Expand Up @@ -549,7 +549,7 @@ describe('microblock 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);
Expand Down

0 comments on commit 07339c0

Please sign in to comment.