fix: reorg txs by inserting txs that are missing from the mempool table #1429

BLuEScioN · 2022-11-09T20:11:55Z

Addressing issue #1231

Recap:
Txs that are not in the mempool when they get reorged do not get reinserted into the mempool.

This PR fixes this.

Run the same update query and return the number of rows updated
If the updated row count is not the same as the reorged tx count, then we have tx that need inserting
Query for full tx details
Insert into mempool txs

I used this approach rather than a "ON CONFLICT" query approach because this function does not recieve the information needed to insert txs into mempool_txs

Testing:
Added a test that reorgs blocks and their txs and checks that they are inserted into mempool_txs when they are not in the mempool_txs previously. Also checks that txs in mempool_txs get updated

… and fixed test

github-actions · 2022-11-09T20:15:37Z

🚀 Deployed on https://deploy-preview-1429--stacks-blockchain-api.netlify.app

codecov-commenter · 2022-11-09T20:16:56Z

Codecov Report

Attention: Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/datastore/helpers.ts	80.00%	1 Missing and 1 partial ⚠️
src/datastore/pg-write-store.ts	87.50%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

rafaelcr

Looks great so far, a few comments

.vscode/launch.json

src/datastore/helpers.ts

src/datastore/pg-write-store.ts

jest.config.mempool.js

.vscode/launch.json

zone117x · 2022-11-15T17:16:41Z

src/datastore/helpers.ts

+      pruned: false,
+      receipt_time: Math.round(Date.now() / 1000),
+      fee_rate: BigInt(tx.fee_rate),
+      token_transfer_amount: BigInt(tx.token_transfer_amount ?? 0),


It doesn't seem like this function fulfills the "convert mined tx to mempool tx" correctly, or it at least it has some confusing lines. For example, isn't the difference between the two object types only the receipt_time and pruned properties?

I see the line token_transfer_amount which is specific only to token_transfer tx types -- why aren't the other tx-type-specific properties also here? It looks like perhaps that is just an unnecessary property? If it is necessary, something seems wrong, because isn't token_transfer_amount supposed to be null/undefined for txs that are not of type token_transfer?

These other properties are here because the types TxQueryResult and DbMempoolTx do not match up 1:1. There are type differences that I am accounting for in addition to adding receipt_time and pruned properties.

Your're right about token_transfer_amount. I was converting the type incorrectly here.

rafaelcr

LGTM, but I'd defer to @zone117x to see if he has any more concerns

…k file

zone117x

The code looks good. However, anything that touches /new_block event ingestion and/or re-org handling is very sensitive to bugs in production. If something goes wrong, it halts API functionality (and therefore halts nearly the entire ecosystem).

There are many different scenarios in mainnet/production involving behavior of the mempool, anchor block re-orgs, and microblock re-orgs, which do not have complete e2e test coverage.

I'm pretty familiar with the code around this, yet I'm still wary of critical bugs that might be introduced here. Before we merge this, I think we should first:

Deploy to staging env using a full/archival event-replay (not using the pruned mode).
Let it run in the staging it env for at least a few days -- ensure the new code path introduced in this PR are hit (make sure logging is added to check).

If no critical API-halting bugs happen during that, then I'm comfortable merging this.

@BLuEScioN can you coordinate with devops to perform this testing?

BLuEScioN · 2022-11-30T16:08:33Z

lay (not using the pruned mode).

I appreciate the concern for potential critical bugs here @zone117x. I will add logging and coordinate with devops to get this deployed to the staging env

BLuEScioN · 2022-12-22T20:50:42Z

lay (not using the pruned mode).

I appreciate the concern for potential critical bugs here @zone117x. I will add logging and coordinate with devops to get this deployed to the staging env

@zone117x

Here we see reorg activity based on logs is consistent between my branch and a branch without my changes -https://grafana.dev.hiro.tools/d/logs_log_search/log-search?orgId=1&var-namespace=mainnet-api&var-pod=mainnet-api-blue-writer-0&var-pod=mainnet-api-nick-reorg-txs-writer-0&var-container=stacks-blockchain-api-writer&var-log_level=All&var-text_search=Restoring%20mempool%20tx:&from=now-7d&to=now

Here we can see that my code is running as expected, with the number of txs inserted matching the number that should have been inserted -
https://grafana.dev.hiro.tools/d/logs_log_search/log-search?orgId=1&var-namespace=mainnet-api&var-pod=mainnet-api-blue-writer-0&var-pod=mainnet-api-nick-reorg-txs-writer-0&var-container=stacks-blockchain-api-writer&var-log_level=All&var-text_search=&from=1671412209316&to=1671412217572

And here we can see that there was never a mismatch between the number needing inserting and the number that were inserted based on searching for the logs that would indicate this circumstance -
https://grafana.dev.hiro.tools/d/logs_log_search/log-search?orgId=1&var-namespace=mainnet-api&var-pod=mainnet-api-blue-writer-0&var-pod=mainnet-api-nick-reorg-txs-writer-0&var-container=stacks-blockchain-api-writer&var-log_level=All&var-text_search=Not%20all%20txs%20requiring%20insertion%20were%20found&from=1671560574628&to=1671733374628

If this looks good to you, I can merge

zone117x

This looks great, thanks for the thorough testing @BLuEScioN!

## [7.0.0-beta.1](v6.2.1...v7.0.0-beta.1) (2023-01-10) ### ⚠ BREAKING CHANGES * a sync from genesis is required to use with a Stacks v2.1-rc node ### Features * **agg-paging-limits:** aggregated all paging query limits ([#1401](#1401)) ([0203d36](0203d36)), closes [#1379](#1379) [#1379](#1379) * Stacks 2.1 support ([#1498](#1498)) ([dcbdfb9](dcbdfb9)), closes [#1279](#1279) [#1280](#1280) [#1283](#1283) [#1285](#1285) [#1289](#1289) [#1290](#1290) [#1295](#1295) [#1339](#1339) [#1363](#1363) [#1367](#1367) [#1372](#1372) [#1413](#1413) [#1449](#1449) [#1205](#1205) [#1197](#1197) [#1206](#1206) [#1179](#1179) [#1190](#1190) [#1167](#1167) [#1363](#1363) [#1193](#1193) [#1162](#1162) [#1216](#1216) [#1289](#1289) [#1290](#1290) [#1241](#1241) [#1168](#1168) [#1218](#1218) [#1339](#1339) [#1413](#1413) [#1283](#1283) [#1280](#1280) [#1285](#1285) [#1403](#1403) [#1456](#1456) [#1454](#1454) [#1454](#1454) [#1456](#1456) [#1403](#1403) [#1461](#1461) [#1476](#1476) [#1329](#1329) [#1287](#1287) [#1476](#1476) [#1366](#1366) [#1304](#1304) [#1331](#1331) [#1332](#1332) [#1379](#1379) [#1379](#1379) [#1355](#1355) [#1287](#1287) [#1389](#1389) [#1323](#1323) [#1368](#1368) [#1348](#1348) [#1314](#1314) [#1303](#1303) [#1425](#1425) [#1334](#1334) [#1309](#1309) [#1445](#1445) [#1374](#1374) [#1345](#1345) [#1353](#1353) [#1433](#1433) [#1424](#1424) [#1427](#1427) [#1301](#1301) [#1458](#1458) [#1379](#1379) [#1270](#1270) [#1324](#1324) [#1356](#1356) [#1360](#1360) [#1315](#1315) [#1326](#1326) [#1440](#1440) [#1351](#1351) [#1410](#1410) [#1337](#1337) [#1420](#1420) [#1328](#1328) [#1329](#1329) [#1343](#1343) [#1329](#1329) [#1495](#1495) ### Bug Fixes * add bnsImportUpdate to event emitter to fix BNS import test ([#1491](#1491)) ([2f9cb0c](2f9cb0c)) * guarantee db is empty before performing a replay ([#1374](#1374)) ([ef8e7a9](ef8e7a9)) * lint docs ci dependencies ([#1458](#1458)) ([90d0c7b](90d0c7b)) * make query limits backwards compatible ([#1509](#1509)) ([a0cebf5](a0cebf5)) * reorg txs by inserting txs that are missing from the mempool table ([#1429](#1429)) ([a512511](a512511)) * synthetic tx parsing for pox2 bitcoin-ops ([#1505](#1505)) ([720dc87](720dc87)) ### Miscellaneous Chores * support for Stacks 2.1 ([e88ec29](e88ec29))

blockstack-devops · 2023-01-10T18:53:32Z

🎉 This PR is included in version 7.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

## [7.0.0](v6.3.4...v7.0.0) (2023-02-07) ### ⚠ BREAKING CHANGES * support for upcoming Stacks 2.1 features, event-replay required; * a sync from genesis is required to use with a Stacks v2.1-rc node ### Features * [Stacks 2.1] `delegate-stx` Bitcoin-op parsing ([#1527](#1527)) ([ea01587](ea01587)) * **agg-paging-limits:** aggregated all paging query limits ([#1401](#1401)) ([0203d36](0203d36)), closes [#1379](#1379) [#1379](#1379) * Stacks 2.1 support ([#1498](#1498)) ([dcbdfb9](dcbdfb9)), closes [#1279](#1279) [#1280](#1280) [#1283](#1283) [#1285](#1285) [#1289](#1289) [#1290](#1290) [#1295](#1295) [#1339](#1339) [#1363](#1363) [#1367](#1367) [#1372](#1372) [#1413](#1413) [#1449](#1449) [#1205](#1205) [#1197](#1197) [#1206](#1206) [#1179](#1179) [#1190](#1190) [#1167](#1167) [#1363](#1363) [#1193](#1193) [#1162](#1162) [#1216](#1216) [#1289](#1289) [#1290](#1290) [#1241](#1241) [#1168](#1168) [#1218](#1218) [#1339](#1339) [#1413](#1413) [#1283](#1283) [#1280](#1280) [#1285](#1285) [#1403](#1403) [#1456](#1456) [#1454](#1454) [#1454](#1454) [#1456](#1456) [#1403](#1403) [#1461](#1461) [#1476](#1476) [#1329](#1329) [#1287](#1287) [#1476](#1476) [#1366](#1366) [#1304](#1304) [#1331](#1331) [#1332](#1332) [#1379](#1379) [#1379](#1379) [#1355](#1355) [#1287](#1287) [#1389](#1389) [#1323](#1323) [#1368](#1368) [#1348](#1348) [#1314](#1314) [#1303](#1303) [#1425](#1425) [#1334](#1334) [#1309](#1309) [#1445](#1445) [#1374](#1374) [#1345](#1345) [#1353](#1353) [#1433](#1433) [#1424](#1424) [#1427](#1427) [#1301](#1301) [#1458](#1458) [#1379](#1379) [#1270](#1270) [#1324](#1324) [#1356](#1356) [#1360](#1360) [#1315](#1315) [#1326](#1326) [#1440](#1440) [#1351](#1351) [#1410](#1410) [#1337](#1337) [#1420](#1420) [#1328](#1328) [#1329](#1329) [#1343](#1343) [#1329](#1329) [#1495](#1495) ### Bug Fixes * add bnsImportUpdate to event emitter to fix BNS import test ([#1491](#1491)) ([2f9cb0c](2f9cb0c)) * build rosetta with node 16 ([654b64f](654b64f)) * datastore tests ([bb96507](bb96507)) * guarantee db is empty before performing a replay ([#1374](#1374)) ([ef8e7a9](ef8e7a9)) * lint docs ci dependencies ([#1458](#1458)) ([90d0c7b](90d0c7b)) * make query limits backwards compatible ([#1509](#1509)) ([a0cebf5](a0cebf5)) * prevent token metadata processor from blocking api launch ([#1514](#1514)) ([63da7e1](63da7e1)) * reorg txs by inserting txs that are missing from the mempool table ([#1429](#1429)) ([a512511](a512511)) * synthetic tx parsing for pox2 bitcoin-ops ([#1505](#1505)) ([720dc87](720dc87)) * test tx types ([11b9013](11b9013)) * use correct `pox-addr` arg while parsing `stack-stx` bitcoin-op [#415](#415) ([#1533](#1533)) ([ab14ad5](ab14ad5)) * use pg bigint for `pox_v1_unlock_height` column ([#1521](#1521)) ([d3fd685](d3fd685)) ### Miscellaneous Chores * note for Stacks 2.1 support and major version bump ([d27f956](d27f956)) * support for Stacks 2.1 ([e88ec29](e88ec29))

blockstack-devops · 2023-02-07T19:40:57Z

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

BLuEScioN added 6 commits October 31, 2022 15:56

feat(reorg-txs): work in progress

060c423

feat(re-org-tx): work in progress

c0db5db

fix(mempool-txs-reorg): work in progress

7b33019

fix(reorg-tx): work in progress

992c371

fix(re-org-txs): query is working. types for tx ids are not

c29ed63

fix(re-org-txs): fixed insertion logic for reorged txs not in mempool…

a057b73

… and fixed test

BLuEScioN requested review from rafaelcr and zone117x November 9, 2022 20:12

BLuEScioN added 2 commits November 9, 2022 14:14

fix(re-org-txs): cleaning pr

37719a0

Merge branch 'develop' into nickb/reorg-txs

6801d00

BLuEScioN changed the base branch from master to develop November 9, 2022 20:15

github-actions bot temporarily deployed to pull request November 9, 2022 20:17 Inactive

github-actions bot temporarily deployed to commit November 9, 2022 20:18 Inactive

rafaelcr reviewed Nov 11, 2022

View reviewed changes

fix(re-org-txs): addressed pr feedback

7dbd599

github-actions bot temporarily deployed to pull request November 13, 2022 19:56 Inactive

github-actions bot temporarily deployed to commit November 13, 2022 19:56 Inactive

rafaelcr reviewed Nov 15, 2022

View reviewed changes

jest.config.mempool.js Outdated Show resolved Hide resolved

.vscode/launch.json Outdated Show resolved Hide resolved

BLuEScioN requested a review from rafaelcr November 15, 2022 16:00

zone117x reviewed Nov 15, 2022

View reviewed changes

fix(re-org-tx): addressing pr feedback

3d521d4

BLuEScioN requested a review from zone117x November 15, 2022 23:48

github-actions bot temporarily deployed to commit November 15, 2022 23:50 Inactive

github-actions bot temporarily deployed to pull request November 15, 2022 23:51 Inactive

fix(reorg-tx): cleaning

e3dcf62

github-actions bot temporarily deployed to pull request November 16, 2022 20:16 Inactive

github-actions bot temporarily deployed to commit November 16, 2022 20:16 Inactive

rafaelcr approved these changes Nov 17, 2022

View reviewed changes

fix(reorg-txs): trying to fix lint error

35455c4

github-actions bot temporarily deployed to pull request November 29, 2022 15:02 Inactive

github-actions bot temporarily deployed to commit November 29, 2022 15:02 Inactive

Merge branch 'develop' into nickb/reorg-txs

3dc0c75

github-actions bot temporarily deployed to commit November 29, 2022 17:03 Inactive

github-actions bot temporarily deployed to pull request November 29, 2022 17:04 Inactive

fix(reorg-txs): reverting changes to develop for package.json and loc…

811f9ba

…k file

github-actions bot temporarily deployed to commit November 29, 2022 17:42 Inactive

github-actions bot temporarily deployed to pull request November 29, 2022 17:42 Inactive

zone117x reviewed Nov 30, 2022

View reviewed changes

fix(reorg-txs): adding logging to restoremempooltxs

3f7d8fd

github-actions bot temporarily deployed to pull request November 30, 2022 18:39 Inactive

github-actions bot temporarily deployed to commit November 30, 2022 18:40 Inactive

BLuEScioN changed the title ~~Nickb/reorg txs~~ fix: reorg txs by inserting txs that are missing from the mempool table Dec 20, 2022

zone117x approved these changes Jan 2, 2023

View reviewed changes

fix: merged develop to remove merge conflicts

3876cc2

github-actions bot temporarily deployed to commit January 3, 2023 16:04 Inactive

github-actions bot temporarily deployed to pull request January 3, 2023 16:04 Inactive

BLuEScioN merged commit a512511 into develop Jan 3, 2023

BLuEScioN deleted the nickb/reorg-txs branch January 3, 2023 16:26

BLuEScioN mentioned this pull request Jan 3, 2023

re-orgd transactions return 404 if they weren't in the mempool before #1231

Closed

blockstack-devops added the released on @beta label Jan 10, 2023

blockstack-devops added the released label Feb 7, 2023

fix: reorg txs by inserting txs that are missing from the mempool table #1429

fix: reorg txs by inserting txs that are missing from the mempool table #1429

Uh oh!

Conversation

BLuEScioN commented Nov 9, 2022

Uh oh!

github-actions bot commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Nov 9, 2022 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rafaelcr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zone117x Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

BLuEScioN Nov 15, 2022

Choose a reason for hiding this comment

Uh oh!

rafaelcr left a comment

Choose a reason for hiding this comment

Uh oh!

zone117x left a comment

Choose a reason for hiding this comment

Uh oh!

BLuEScioN commented Nov 30, 2022

Uh oh!

BLuEScioN commented Dec 22, 2022

Uh oh!

zone117x left a comment

Choose a reason for hiding this comment

Uh oh!

blockstack-devops commented Jan 10, 2023

Uh oh!

blockstack-devops commented Feb 7, 2023

Uh oh!

Uh oh!

github-actions bot commented Nov 9, 2022 •

edited

Loading

codecov-commenter commented Nov 9, 2022 •

edited by codecov bot

Loading