Skip to content

fix: re-org bug introduced in microblock implementation #686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 6, 2021

Conversation

zone117x
Copy link
Member

@zone117x zone117x commented Jul 30, 2021

Closes #670

The gist of the problem is that immediately non-canonical anchor blocks were indiscriminately marking their associated microblock confirmations and txs as also non-canonical.

"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.

The fix was to add another condition to all sql queries that select or update from the microblocks table so that these anchor blocks have no effect on microblocks (and their associated transactions) that have already been confirmed by a previous, canonical anchor block.

EDIT: the above fix caused a second re-org related bug that caused transactions to be incorrectly marked as anchor-block re-orged when a different order of event streams involving micro-orphans and anchor-orphans were ingested. This required microblock re-org detection and restoration being performed on all the same code paths as anchor block re-org restoration.

I've manually confirmed the fix works by reproducing the bug using the event replay data, then verifying the corrected behavior with the replay data.

TODO:

  • Need to translate the event data into a unit test.
  • The changes for processing unanchored/unconfirmed microblocks from the /new_microblock payload, along with the subsequent confirmation or rejection from an anchor block from a /new_block payload isn't well tested. It looks like the event replay data got accidentally wiped from the mainnet microblocks deployment, so we'll have to deploy this branch and let it run for a while to confirm no regressions.

@vercel
Copy link

vercel bot commented Jul 30, 2021

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/DUGTGH9qEBQCSAB7uH3NFLvtJCMd
✅ Preview: https://stacks-blockchain-api-git-fix-microblocks-re-org-blockstack.vercel.app

@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #686 (2739b89) into develop (f7c5e28) will increase coverage by 2.05%.
The diff coverage is 87.32%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #686      +/-   ##
===========================================
+ Coverage    62.10%   64.15%   +2.05%     
===========================================
  Files           78       78              
  Lines         7763     7767       +4     
  Branches      1205     1203       -2     
===========================================
+ Hits          4821     4983     +162     
+ Misses        2938     2780     -158     
  Partials         4        4              
Impacted Files Coverage Δ
src/helpers.ts 64.25% <ø> (+4.25%) ⬆️
src/index.ts 0.00% <ø> (ø)
src/datastore/postgres-store.ts 83.24% <85.00%> (+6.15%) ⬆️
src/api/init.ts 86.74% <100.00%> (+0.08%) ⬆️
src/event-stream/event-server.ts 77.71% <100.00%> (+14.26%) ⬆️
src/migrations/1584619633448_txs.ts 100.00% <100.00%> (ø)
src/migrations/1620937804009_microblocks.ts 100.00% <100.00%> (ø)
src/event-stream/reader.ts 59.04% <0.00%> (+2.85%) ⬆️
... and 2 more

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 f7c5e28...2739b89. Read the comment docs.

@zone117x zone117x merged commit 7dfc5a9 into develop Aug 6, 2021
@zone117x zone117x deleted the fix/microblocks-re-org branch August 6, 2021 00:46
@blockstack-devops
Copy link

🎉 This PR is included in version 0.63.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@zone117x zone117x mentioned this pull request Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in micro-fork handling
2 participants