Fix/804

[zone117x]

Closes #804

This PR fixes a couple issues with microblock re-org handling.

It also likely fixes #624 -- confirmed transactions showing up in mempool as pending -- still need to finish testing this one.

The serious bug here resulted in the API not showing certain transactions in rare scenarios involving both a micro-reorg and anchor-block-reorg. This also resulted in other queries like account balances to be incorrect.

Only around ~30 transactions in total appear to have been affected (and incorrectly appearing as orphaned).

This was caused by a bug similar to the one addressed in #686. Immediately non-canonical anchor blocks (referring to /new_block payloads received by the API for blocks that already have a canonical block at the same height) would end up marking transactions as micro-orphaned, and in very rare re-org scenarios, these transactions were not restored after a re-org, i.e. they incorrectly remained orphaned.

The fix for this involved A) prevent immediately-non-canonical new blocks from marking microblock data as micro-orphaned because they could be potentially shared with other blocks at the same height, and B) reverse the order of re-org handling so that txs in a given block are marked orphaned before the canonical txs are restored, fixing an issue where anchor block forks clobber over each other's microblock pointers.

So far I've tested the fix by running a query to return txs that are orphaned, and comparing the results from the old db and a db populated using these changes. This query does the job:

select DISTINCT ON (tx_id) tx_id, canonical, microblock_canonical, block_height from (
  select * from txs a
  where not exists (select 1 from txs b where a.tx_id=b.tx_id and canonical=true and microblock_canonical=true)
) orphaned
where block_height <= 34425
order by tx_id, block_height desc

Then run through the txs that are missing from the old db, find ones that include a STX transfer, then use the https://stacks-node-api.mainnet.stacks.co/v2/accounts/<address> core-node API to verify balances are correct.

---

A different, less serious bug resulted in an incorrect/partial list of microblock hashes in the /block response object for the microblocks_accepted: [] and microblocks_streamed: [] arrays. It looks like this could cause display issues in the Explorer's "recent block" list during rare situations involving both a micro-reorg and anchor-block-reorg.

This was fixed by removing an UPDATE query which was incorrectly identifying rows in the microblocks table by the cached index_block_hash column:

UPDATE microblocks
SET canonical = $2
WHERE index_block_hash = $1 AND canonical != $2
RETURNING microblock_hash

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blockstack/stacks-blockchain-api/FVmpJsuRubSoUkYP732fipsSxNDi
✅ Preview: https://stacks-blockchain-api-git-fix-804-blockstack.vercel.app

[codecov]

Codecov Report

Merging #817 (d21ff14) into develop (4ade5ee) will increase coverage by 0.01%.
The diff coverage is 80.43%.

@@             Coverage Diff             @@
##           develop     #817      +/-   ##
===========================================
+ Coverage    65.39%   65.40%   +0.01%     
===========================================
  Files           90       90              
  Lines         9122     9142      +20     
  Branches      1459     1461       +2     
===========================================
+ Hits          5965     5979      +14     
- Misses        3152     3158       +6     
  Partials         5        5              

Impacted Files	Coverage Δ
src/datastore/postgres-store.ts	83.26% <80.43%> (-0.16%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ade5ee...d21ff14. Read the comment docs.

[kyranjamie]

Is this an issue we think might be often be a problem. The wallet could help here, right? Displaying a warning if API returns a different balance from the node?

[zone117x]

Is this an issue we think might be often be a problem. The wallet could help here, right? Displaying a warning if API returns a different balance from the node?

Historically that would result in more false positive alerts due to sync issues between the node pool and the API. The client would need to fetch the balance for a given (current) block height using ?tip=<block> (e.g. https://stacks-node-api.mainnet.stacks.co/v2/accounts/SPYQW63PYXA8DFQS34GRMT5DPNRA6WR0TN19RY9F?proof=0&tip=ccf77c96f5ba4093b8ddcc92f372752d5fc20926723a2ce0a396c9fe634663cd) -- but that code is bugged and requires an index_block_hash rather than a normal block hash/height, and the index_block_hash is not readily available to clients.

@rafaelcr is working on a script we can run internally that can perform this kind of testing/validation #819

[rafaelcr]

LGTM

[zone117x]

Manually verified this PR fix balance issues with the following known addresses:

• https://stacks-node-api-inactive.mainnet.stacks.co/extended/v1/address/SP1VA9HEH5VJFBCGJ6F8VF9BGS8WF6DJE9AD7856/stx
• https://stacks-node-api-inactive.mainnet.stacks.co/extended/v1/address/SP16661ZGFH2Y9NGSQXTBH6TYTJ9YZV3B2TARW92J/stx
• https://stacks-node-api-inactive.mainnet.stacks.co/extended/v1/address/SP2HNGAYYWR6SN445WBE30HD2YBK3D5GE713YCEDZ/stx

And the addresses/txs listed in issue #818

[blockstack-devops]

🎉 This PR is included in version 0.70.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀