diff --git a/category/statesync/statesync_client.cpp b/category/statesync/statesync_client.cpp index 6a7d0925b7..d22ee58d7f 100644 --- a/category/statesync/statesync_client.cpp +++ b/category/statesync/statesync_client.cpp @@ -18,7 +18,9 @@ #include #include #include +#include #include +#include // NOLINT #include #include #include @@ -195,10 +197,10 @@ bool monad_statesync_client_finalize(monad_statesync_client_context *const ctx) } // Roll one db forward from its synced version to the target, replaying the - // trailing block headers, then mark the target finalized. Applied to the - // primary and, in dual-db mode, the page-encoded secondary, so both - // timelines are version- and finalize-consistent at the target. - auto const roll_forward_and_finalize = + // trailing block headers. Applied to the primary and, in dual-db mode, the + // page-encoded secondary, so both timelines are version-consistent at the + // target. + auto const roll_forward = [ctx, &tgrt, latest_version](mpt::Db &db) -> bool { if (latest_version != tgrt.number) { db.move_trie_version_forward(latest_version, tgrt.number); @@ -237,14 +239,13 @@ bool monad_statesync_client_finalize(monad_statesync_client_context *const ctx) false); } } - db.update_finalized_version(tgrt.number); return true; }; - if (!roll_forward_and_finalize(ctx->db)) { + if (!roll_forward(ctx->db)) { return false; } - if (ctx->secondary_db && !roll_forward_and_finalize(*ctx->secondary_db)) { + if (ctx->secondary_db && !roll_forward(*ctx->secondary_db)) { return false; } @@ -262,9 +263,16 @@ bool monad_statesync_client_finalize(monad_statesync_client_context *const ctx) page_encoded ? "page" : "slot"); db = ctx->secondary_tdb.get(); } - db->set_block_and_prefix(ctx->db.get_latest_finalized_version()); - MONAD_ASSERT(db->get_block_number() == tgrt.number); - return db->state_root() == tgrt.state_root; + db->set_block_and_prefix(tgrt.number); + if (auto const root = db->state_root(); root != tgrt.state_root) { + return false; + } + + ctx->db.update_finalized_version(tgrt.number); + if (ctx->secondary_db) { + ctx->secondary_db->update_finalized_version(tgrt.number); + } + return true; } void monad_statesync_client_context_destroy( diff --git a/category/statesync/test/test_statesync.cpp b/category/statesync/test/test_statesync.cpp index c1c9c72ccb..91d1ae1c18 100644 --- a/category/statesync/test/test_statesync.cpp +++ b/category/statesync/test/test_statesync.cpp @@ -805,6 +805,73 @@ TEST_F(StateSyncFixture, sync_one_account) EXPECT_TRUE(monad_statesync_client_finalize(cctx)); } +TEST_F(StateSyncFixture, retry_after_failed_root_validation) +{ + constexpr auto N = 1'000'000; + bytes32_t parent_hash{NULL_HASH}; + load_header( + sdb.load_root_for_version(N - 257), + sdb, + BlockHeader{.number = N - 257}); + for (size_t i = N - 256; i < N; ++i) { + stdb.set_block_and_prefix(i - 1); + commit_sequential( + stdb, + StateDeltas({}), + {}, + BlockHeader{.parent_hash = parent_hash, .number = i}); + parent_hash = to_bytes( + keccak256(rlp::encode_block_header(stdb.read_eth_header()))); + } + commit_sequential( + stdb, + StateDeltas( + {{ADDR_A, + StateDelta{ + .account = {std::nullopt, Account{.balance = 100}}, + .storage = {}}}}), + Code{}, + BlockHeader{.number = N}); + init(); + BlockHeader const tgrt{ + .parent_hash = parent_hash, + .state_root = stdb.state_root(), + .number = N}; + handle_target(cctx, tgrt); + // A syntactically valid account the server never sent corrupts the synced + // state: the sync completes normally but the root comparison in finalize + // must fail against the honest target. + auto const corrupt = encode_account_db( + 0x00000000000000000000000000000000cafebabe_address, + Account{.balance = 42}); + EXPECT_TRUE(monad_statesync_client_handle_upsert( + cctx, 0, SYNC_TYPE_UPSERT_ACCOUNT, corrupt.data(), corrupt.size())); + run(); + EXPECT_TRUE(monad_statesync_client_has_reached_target(cctx)); + EXPECT_FALSE(monad_statesync_client_finalize(cctx)); + + monad_statesync_client_context_destroy(cctx); + cctx = nullptr; + monad_statesync_server_destroy(server); + server = nullptr; + + // The failed validation must not have recorded the corrupt target as + // finalized: that is the baseline a restarted client rewinds to. + { + mpt::Db cdb{ + std::make_unique(), + mpt::OnDiskDbConfig{.append = true, .dbname_paths = {cdbname}}}; + EXPECT_EQ(cdb.get_latest_finalized_version(), INVALID_BLOCK_NUM); + } + + // A restarted client syncing the same honest target must succeed rather + // than inherit the corrupt state as its sync baseline. + init(); + handle_target(cctx, tgrt); + run(); + EXPECT_TRUE(monad_statesync_client_finalize(cctx)); +} + // Pre-fork dual-timeline server -> dual-timeline client TEST_F(StateSyncFixture, pre_fork_dual_timeline_server_to_dual_db_client) {