Test real blocks #601

Sword-Smith · 2025-05-20T18:42:06Z

test: Recover state from real main net blocks

Add a test of GlobalState::bootstrap_from_directory using real data
from main net. The test uses the blk0.dat file from a node that has
been online since genesis, thus containing reorganizations that some
previously untested branches of GlobalState::bootstrap_from_directory.

This test also serves as a check that the first 113 main net blocks are
still considered valid by future versions of the software.

Closes #515

This PR introduces a big file, blk0.dat, added with git lfs. But an alternative would be to download the blk0.dat from our test servers instead, and store it in test_data so that it would only ever have to be downloaded once.

dan-da · 2025-05-21T01:14:27Z

But an alternative would be to download the blk0.dat from our test servers instead, and store it in test_data so that it would only ever have to be downloaded once.

This seems preferable to me because:

It keeps the repo small(er) for anyone that doesn't need to run unit tests.
It still works for anyone that does not have git-lfs installed, but does wish to run unit tests.
in general seems to be more out-of-the-box compatible with git distros on other OSes, etc.

I note that git-lfs is installed by default on ubuntu 24.04, but according to gemini LLM:

windows: git lfs present
macos: requires install by homebrew
BSDs: not installed by default
other linux distros(debian, arch, fedora): not installed by default

and even if installed by default, ti requires user to git lfs install.

dan-da

I'd like to switch to downloading the blk0.dat within the unit test if we can, for reasons outlined in a previous comment.

just for reference: I performed a timed git checkout of this branch via my slow connection. It took 8 minutes, 48 seconds.

I'm not sure the GlobalStateLock constructor change is needed and/or I think it can be done more cleanly. see inline comments.

It's likely worth inviting another reviewer for another set of eyes.

dan-da · 2025-05-21T04:13:37Z

src/models/state/mod.rs

+        genesis: Block,
+        cli_args: cli_args::Args,
+        rpc_server_to_main_tx: tokio::sync::mpsc::Sender<RPCServerToMain>,
+    ) -> Result<Self> {


consider renaming to try_new(). So if we keep the old pub fn new() and add this try_new() then it can be constructed either way.

further musings

I don't (yet?) see why all this init code should be occurring inside GlobalStateLock's constructor rather than the caller(s), eg initialize() in lib.rs.

In theory GlobalStateLock should be just a thin wrapper around GlobalState, though admittedly its role has been growing over time.

I read the commit comment, but I'm still unclear on the motivation for the change. Is this necessary for the PR to "test real blocks"?

If the goal is to make the init code more reusable, perhaps consider a GlobalStateLock builder with a fallible build() method instead. It could be in a separate file.

also for a fallible constructor on such a central/public API type I'd really like to see it return a thiserror Error enum for all the error variants. Or alternatively not be pub.

I read the commit comment, but I'm still unclear on the motivation for the change. Is this necessary for the PR to "test real blocks"?

Yes. The genesis block under the test flag is different from the real genesis block. Because under the test flag initial difficulty is 6.000 and otherwise it's 1.000.000.000. So the refactor is needed to allow the genesis block to be injected by the caller when creating a new GlobalState/GlobalStateLock.

If it's just for test purposes, couldn't it be done after GlobalStateLock is created, eg?

let gsl = GlobalStateLock::new(...); // using the original new() method let mut gsm = gsl.lock_guard_mut().await; gsm.chain = BlockchainState::Archival( ... );

or perhaps cleaner the fallible try_new() could be in an impl block inside the test module, eg:

#[cfg(test}] mod tests { impl GlobalStateLock { fn try_new(...) -> anyhow::Result<Self> { .. } } }

If it's just for test purposes, couldn't it be done after GlobalStateLock is created, eg?

These state updates are quite tricky, as the wallet state also needs to be updated with the right block, in which potential premine rewards are received. The genesis block needs to be injected like this.

dan-da · 2025-05-21T04:45:32Z

src/models/state/mod.rs

@@ -161,8 +166,7 @@ pub struct GlobalStateLock {
 }

 impl GlobalStateLock {
-    /// the key to the watery kingdom.
-    pub fn new(
+    pub(crate) fn new_internal(


I feel like this should still be pub fn new().

dan-da · 2025-05-21T04:58:26Z

src/models/state/archival_state.rs

-        network: Network,
-    ) -> Self {
-        let genesis_block = Box::new(Block::genesis(network));
+    pub(crate) async fn new(data_dir: DataDirectory, genesis_block: Block) -> Self {


This method is now fallible. consider renaming to try_new() and returning a result rather than panicking within the method. Or at least document it can panic and under what conditions.

Sword-Smith · 2025-05-21T20:36:17Z

I addressed some concerns:

Blocks are now downloaded from "proof" servers, cf. 385479c5a20a25604378ba3506278e25b1db1f52
Less database initialization/opening in GlobalstateLock's constructor, as this has been moved to GlobalState's constructor.

dan-da

The cli_args::Args::default_with_network(network) seems a nice shortcut.

I left a few more comments/suggestions, but overall it seems ok to merge.

dan-da · 2025-05-22T05:31:40Z

src/models/state/mod.rs

-        net: NetworkingState,
-        cli: cli_args::Args,
-        mempool: Mempool,
+    pub(crate) fn from_global_state(


this is an interesting change. I think it is positive to build the GlobalState first, and then the GlobalStateLock from it, as this change implies.

I think where I'd really like to get to is impl From<GlobalState> for GlobalStateLock {..} seems really clean.

But that's not possible right now due to the mpsc::Sender for rpc. Maybe we can find a better place for that than GlobalStateLock.

Also, removing the pub fn new() entirely is a breaking change to the public API. I don't know if anyone is using it, but there could be, in the community. I would probably leave that API in place for now, but I don't feel so strongly about it.

I'll make the constructors for GlobalState and GlobalStateLock public again.

dan-da · 2025-05-22T05:34:38Z

src/models/state/mod.rs

@@ -616,6 +617,49 @@ impl Drop for GlobalState {
 }

 impl GlobalState {


I'm still not convinced this fallible initialization code needs to exist in state/mod.rs at all. However, since it is only pub(crate), I am satisfied, as we can always move it later without changing public API.

dan-da · 2025-05-22T06:01:32Z

src/tests/shared.rs

@@ -456,6 +452,160 @@ impl<Item> stream::Stream for Mock<Item> {
    }
 }

+/// Return path for the directory containing test data, like proofs and block


this file is growing large. maybe it's time to consider splitting into separate file(s).

dan-da · 2025-05-22T06:03:41Z

src/tests/shared.rs

@@ -1330,3 +1458,16 @@ where
    }
    Ok(())
 }
+
+#[cfg(test)]


redundant. This entire file is already #[cfg(test)].

Move initialization of databases used by the archival state into its constructor.

Change function signature and logic of `GlobalState::new` to move more of the database initialization into this constructor. Previously, this database initialization happened directly in lib.rs's `initialize`. Also allow caller of this constructor to specify the genesis block, as this will widen our testing capabilities somewhat. Big diff because this changes many tests. But the diff of non-testing, mainline, code is quite small.

Refactor this code to fetch proofs from server so it can be used for other data as well, specifically to also fetch blk files containing block data.

Add a test of `GlobalState::bootstrap_from_directory` using real data from main net. The test uses the `blk0.dat` file from a node that has been online since genesis, thus containing reorganizations that some previously untested branches of `GlobalState::bootstrap_from_directory`. This test also serves as a check that the first 113 main net blocks are still considered valid by future versions of the software.

Sword-Smith requested a review from dan-da May 20, 2025 18:42

dan-da requested changes May 21, 2025

View reviewed changes

Sword-Smith force-pushed the test-real-blocks branch 2 times, most recently from c576b96 to 59ee436 Compare May 21, 2025 20:34

Sword-Smith requested a review from dan-da May 21, 2025 20:50

dan-da approved these changes May 22, 2025

View reviewed changes

Sword-Smith force-pushed the test-real-blocks branch 2 times, most recently from 809a1e6 to b79b98d Compare May 22, 2025 08:05

Sword-Smith added 2 commits May 22, 2025 10:06

chore: cargo fmt

46f5bd3

refactor(ArchivalState): Improve encapsulation for constructor

863ff0a

Move initialization of databases used by the archival state into its constructor.

Sword-Smith force-pushed the test-real-blocks branch from b79b98d to 29597cb Compare May 22, 2025 08:07

Sword-Smith added 4 commits May 22, 2025 10:12

refactor(GlobalState): Change constructors

905f8a2

test: Refactor code to fetch proofs from server

65193f9

Refactor this code to fetch proofs from server so it can be used for other data as well, specifically to also fetch blk files containing block data.

Sword-Smith force-pushed the test-real-blocks branch from 29597cb to 568b952 Compare May 22, 2025 08:13

Sword-Smith merged commit 63a0af0 into master May 22, 2025
11 of 12 checks passed

		@@ -616,6 +617,49 @@ impl Drop for GlobalState {
		}

		impl GlobalState {

Test real blocks #601

Test real blocks #601

Uh oh!

Conversation

Sword-Smith commented May 20, 2025

Uh oh!

dan-da commented May 21, 2025

Uh oh!

dan-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

further musings

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sword-Smith May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sword-Smith commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-da left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Sword-Smith May 21, 2025 •

edited

Loading

Sword-Smith commented May 21, 2025 •

edited

Loading