fix: seed forking handler with unconfirmed blocks to improve startup stability #505

MicaiahReid · 2024-02-15T21:43:26Z

Description

If Chainhook restarts in the middle of a reorg taking place, it doesn't have any context for choosing the canonical fork. Then when block collisions take place, Chainhook fails to process the new blocks, causing gaps in the blocks Chainhook has available for evaluation.

This PR seeds the stacks block indexer with unconfirmed blocks on startup, so that Chainhook has the necessary context to handle a reorg.

Most of the PR is to add two tests:

I've added some new functionality to our very thorough indexer tests
- In addition to providing the blocks to be mined and the order to mine them, we also now allow providing some "unconfirmed" blocks to seed the block pool with.
- I've added some test cases that reproduce what caused outages on the Platform's Chainhook node
I've added a new service test that:
- Verifies that unconfirmed blocks are stored on restart
- Verifies that those blocks are used to seed the block pool, and that a reorg is handled correctly.

I committed these tests before adding the fix, so you can confirm the fix by checking out the commits with the new tests (96d8249 and 9aad55e), seeing that the tests break, then pulling the last commit to see that the fix works.

Fixes #487

MicaiahReid · 2024-02-21T15:42:47Z

components/chainhook-cli/src/service/mod.rs

@@ -38,6 +39,7 @@ impl Service {
    pub async fn run(
        &mut self,
        predicates_from_startup: Vec<ChainhookFullSpecification>,
+        observer_commands_tx_rx: Option<(Sender<ObserverCommand>, Receiver<ObserverCommand>)>,


Being able to create the observer commands tx externally allows stopping the service from a test.

I don't usually love changing non-test code for tests, but I was stuck on this one.

MicaiahReid · 2024-02-21T15:53:59Z

components/chainhook-cli/src/service/tests/helpers/mock_service.rs

@@ -1,33 +1,37 @@
-use crate::config::Config;


This file is mostly just moving some test setup functions here from a different file, and adding the TestSetupResult struct to clean up the response a bit.

MicaiahReid · 2024-02-21T15:55:06Z

components/chainhook-cli/src/service/tests/helpers/mock_stacks_node.rs

@@ -8,7 +8,7 @@ use chainhook_sdk::types::{
    STXTransferEventData, SmartContractEventData, StacksTransactionEventPayload,


This file's changes are to allow adding a fork_id and parent_fork_id to the stacks blocks that are mined for tests.

MicaiahReid · 2024-02-21T15:57:30Z

components/chainhook-cli/src/service/tests/mod.rs

-    ChainhookFullSpecification, ChainhookSpecification, StacksChainhookFullSpecification,
-};
-use chainhook_sdk::types::{Chain, StacksNetwork};
+use chainhook_sdk::chainhooks::types::ChainhookFullSpecification;


A lot of the changes in this file are more cleanup:

moving out helpers

our mine_block function now takes fork ids, so updating existing calls to that function to pass an fork id

our helpers have a new struct for the return value, so updating the tests that use that helper

simplifying our code to clean up temp test data

The actual change is the addition of the it_seeds_block_pool_on_startup test all the way at the bottom

MicaiahReid · 2024-02-21T16:02:11Z

components/chainhook-sdk/src/indexer/stacks/tests.rs

@@ -15,262 +15,271 @@ use test_case::test_case;

 #[test]
 fn test_stacks_vector_001() {
-    process_stacks_blocks_and_check_expectations(helpers::stacks_shapes::get_vector_001());
+    process_stacks_blocks_and_check_expectations((helpers::stacks_shapes::get_vector_001(), None));


I know it's not ideal that we have this additional argument being passed. The other option is to duplicate the process_stacks_blocks_and_check_expectations function with the changes I made to seed the block pool, so only the two new tests need to call it. Pick your poison ¯_(ツ)_/¯

MicaiahReid · 2024-02-21T16:02:59Z

components/chainhook-sdk/src/indexer/tests/helpers/stacks_shapes.rs

+
+/// Vector 053: Generate the following blocks
+///
+///  A1(0) - B1(1) - C1(3)


A1(0)

I've added the (0) to mean that this data already existed on startup (so we're seeding the block pool with this block)

- allow adding a fork id to new stacks blocks in service test harness - move out helpers and add struct types for return values - add cleanup helper - allow passing observer_commands_tx/rx to service.run

…ocks

lgalabru

LGTM!

…stability (#505) ### Description If Chainhook restarts in the middle of a reorg taking place, it doesn't have any context for choosing the canonical fork. Then when block collisions take place, Chainhook fails to process the new blocks, causing gaps in the blocks Chainhook has available for evaluation. This PR seeds the stacks block indexer with unconfirmed blocks on startup, so that Chainhook has the necessary context to handle a reorg. Most of the PR is to add two tests: - I've added some new functionality to our very thorough indexer tests - In addition to providing the blocks to be mined and the order to mine them, we also now allow providing some "unconfirmed" blocks to seed the block pool with. - I've added some test cases that reproduce what caused outages on the Platform's Chainhook node - I've added a new service test that: - Verifies that unconfirmed blocks are stored on restart - Verifies that those blocks are used to seed the block pool, and that a reorg is handled correctly. I committed these tests _before_ adding the fix, so you can confirm the fix by checking out the commits with the new tests (96d8249 and 9aad55e), seeing that the tests break, then pulling the last commit to see that the fix works. Fixes #487

## [1.4.0](v1.3.1...v1.4.0) (2024-03-27) ### Features * detect http / rpc errors as early as possible ([ad78669](ad78669)) * use stacks.rocksdb for predicate scan ([#514](#514)) ([a4f1663](a4f1663)), closes [#513](#513) [#485](#485) ### Bug Fixes * enable debug logs in release mode ([#537](#537)) ([fb49e28](fb49e28)) * improve error handling, and more! ([#524](#524)) ([86b5c78](86b5c78)), closes [#498](#498) [#521](#521) [#404](#404) [/github.com//issues/517#issuecomment-1992135101](https://github.com/hirosystems//github.com/hirosystems/chainhook/issues/517/issues/issuecomment-1992135101) [#517](#517) [#506](#506) [#510](#510) [#519](#519) * log errors on block download failure; implement max retries ([#503](#503)) ([0fc38cb](0fc38cb)) * **metrics:** update latest ingested block on reorg ([#515](#515)) ([8f728f7](8f728f7)) * order and filter blocks used to seed forking block pool ([#534](#534)) ([a11bc1c](a11bc1c)) * seed forking handler with unconfirmed blocks to improve startup stability ([#505](#505)) ([485394e](485394e)), closes [#487](#487) * skip db consolidation if no new dataset was downloaded ([#513](#513)) ([983a165](983a165)) * update scan status for non-triggering predicates ([#511](#511)) ([9073f42](9073f42)), closes [#498](#498)

## [1.5.0](v1.4.1...v1.5.0) (2024-04-06) ### Features * add brc-20 schemas for ordhook ([#551](#551)) ([1e25a8f](1e25a8f)) * detect http / rpc errors as early as possible ([e515116](e515116)) * rm blocks ([f35498d](f35498d)) * use stacks.rocksdb for predicate scan ([#514](#514)) ([0baae10](0baae10)), closes [#513](#513) [#485](#485) ### Bug Fixes * build error ([03b3938](03b3938)) * log errors on block download failure; implement max retries ([#503](#503)) ([3bfb0e1](3bfb0e1)) * **metrics:** update latest ingested block on reorg ([#515](#515)) ([823713a](823713a)) * order and filter blocks used to seed forking block pool ([#534](#534)) ([a2865b7](a2865b7)) * revisit 7+ blocks reorg handling ([#553](#553)) ([184fd07](184fd07)) * seed forking handler with unconfirmed blocks to improve startup stability ([#505](#505)) ([b77aca2](b77aca2)), closes [#487](#487) * skip db consolidation if no new dataset was downloaded ([#513](#513)) ([b1469a6](b1469a6)) * update scan status for non-triggering predicates ([#511](#511)) ([32cdfee](32cdfee)), closes [#498](#498)

github-actions · 2024-04-06T03:47:28Z

🎉 This PR is included in version 1.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## [1.5.0](hirosystems/chainhook@v1.4.1...v1.5.0) (2024-04-06) ### Features * add brc-20 schemas for ordhook ([#551](hirosystems/chainhook#551)) ([4cfa300](hirosystems/chainhook@4cfa300)) * detect http / rpc errors as early as possible ([4ef6650](hirosystems/chainhook@4ef6650)) * rm blocks ([3d57290](hirosystems/chainhook@3d57290)) * use stacks.rocksdb for predicate scan ([#514](hirosystems/chainhook#514)) ([e8bc60b](hirosystems/chainhook@e8bc60b)), closes [#513](hirosystems/chainhook#513) [#485](hirosystems/chainhook#485) ### Bug Fixes * build error ([f0fb623](hirosystems/chainhook@f0fb623)) * log errors on block download failure; implement max retries ([#503](hirosystems/chainhook#503)) ([7bd6efd](hirosystems/chainhook@7bd6efd)) * **metrics:** update latest ingested block on reorg ([#515](hirosystems/chainhook#515)) ([5a7d64e](hirosystems/chainhook@5a7d64e)) * order and filter blocks used to seed forking block pool ([#534](hirosystems/chainhook#534)) ([bb2a545](hirosystems/chainhook@bb2a545)) * revisit 7+ blocks reorg handling ([#553](hirosystems/chainhook#553)) ([3632176](hirosystems/chainhook@3632176)) * seed forking handler with unconfirmed blocks to improve startup stability ([#505](hirosystems/chainhook#505)) ([a84fc4c](hirosystems/chainhook@a84fc4c)), closes [#487](hirosystems/chainhook#487) * skip db consolidation if no new dataset was downloaded ([#513](hirosystems/chainhook#513)) ([440a8e3](hirosystems/chainhook@440a8e3)) * update scan status for non-triggering predicates ([#511](hirosystems/chainhook#511)) ([115ac28](hirosystems/chainhook@115ac28)), closes [#498](hirosystems/chainhook#498)

MicaiahReid changed the title ~~fix: seed forking handler with unconfirmed blocks to improve startup stability~~ [wip] fix: seed forking handler with unconfirmed blocks to improve startup stability Feb 15, 2024

MicaiahReid temporarily deployed to Development-mainnet February 15, 2024 22:00 — with GitHub Actions Inactive

MicaiahReid had a problem deploying to Development-testnet February 15, 2024 22:00 — with GitHub Actions Error

MicaiahReid force-pushed the fix-reorg-handler branch 2 times, most recently from 4a1a635 to e7b7573 Compare February 20, 2024 21:23

MicaiahReid requested a review from lgalabru February 20, 2024 21:35

MicaiahReid changed the title ~~[wip] fix: seed forking handler with unconfirmed blocks to improve startup stability~~ fix: seed forking handler with unconfirmed blocks to improve startup stability Feb 20, 2024

MicaiahReid had a problem deploying to Development-mainnet February 20, 2024 21:40 — with GitHub Actions Error

MicaiahReid had a problem deploying to Development-testnet February 20, 2024 21:40 — with GitHub Actions Error

MicaiahReid commented Feb 21, 2024

View reviewed changes

MicaiahReid had a problem deploying to Development-mainnet February 21, 2024 16:15 — with GitHub Actions Error

MicaiahReid had a problem deploying to Development-testnet February 21, 2024 16:15 — with GitHub Actions Error

MicaiahReid added 9 commits March 14, 2024 16:29

add storage helper to get unconfirmed blocks

5c808c9

add command to print unconfirmed blocks

c3d3d15

new stacks indexer tests that should pass but fail

6f1b302

refactor service tests

f606776

- allow adding a fork id to new stacks blocks in service test harness - move out helpers and add struct types for return values - add cleanup helper - allow passing observer_commands_tx/rx to service.run

add new service test that should pass but fails

a75c5ec

fix reorg on startup by seeding stacks block pool with unconfirmed bl…

e186d57

…ocks

remove temporary cli changes

97d2d9b

clean up storage file

4bb16df

rename test

f042e6c

MicaiahReid force-pushed the fix-reorg-handler branch from 4ce8aef to f042e6c Compare March 14, 2024 20:29

lgalabru approved these changes Mar 14, 2024

View reviewed changes

MicaiahReid merged commit b77aca2 into develop Mar 14, 2024
3 checks passed

MicaiahReid deleted the fix-reorg-handler branch March 14, 2024 20:42

MicaiahReid had a problem deploying to Development-mainnet March 14, 2024 20:55 — with GitHub Actions Failure

MicaiahReid had a problem deploying to Development-testnet March 14, 2024 20:55 — with GitHub Actions Failure

MicaiahReid mentioned this pull request Mar 19, 2024

fix: order and filter blocks used to seed forking block pool #534

Merged

github-actions bot added the released label Apr 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: seed forking handler with unconfirmed blocks to improve startup stability #505

fix: seed forking handler with unconfirmed blocks to improve startup stability #505

MicaiahReid commented Feb 15, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024

MicaiahReid Feb 21, 2024

MicaiahReid Feb 21, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024

MicaiahReid Feb 21, 2024 •

edited

Loading

lgalabru left a comment

github-actions bot commented Apr 6, 2024

		@@ -8,7 +8,7 @@ use chainhook_sdk::types::{
		STXTransferEventData, SmartContractEventData, StacksTransactionEventPayload,

fix: seed forking handler with unconfirmed blocks to improve startup stability #505

fix: seed forking handler with unconfirmed blocks to improve startup stability #505

Conversation

MicaiahReid commented Feb 15, 2024 • edited Loading

Description

MicaiahReid Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

MicaiahReid Feb 21, 2024

Choose a reason for hiding this comment

MicaiahReid Feb 21, 2024

Choose a reason for hiding this comment

MicaiahReid Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

MicaiahReid Feb 21, 2024

Choose a reason for hiding this comment

MicaiahReid Feb 21, 2024 • edited Loading

Choose a reason for hiding this comment

lgalabru left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 6, 2024

MicaiahReid commented Feb 15, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024 •

edited

Loading

MicaiahReid Feb 21, 2024 •

edited

Loading