Skip to content
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.

Sign up for GitHub

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

node: main chain db txn management #2112

Merged
merged 16 commits into from
Jun 18, 2024
+173 −6
Merged

node: main chain db txn management #2112

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
55df74b
Add ExecutionEngine tests
JacekGlen Jun 12, 2024
dfbd7ef
Comment out failing test
JacekGlen Jun 12, 2024
eadf9ea
Fix separate forks issue
JacekGlen Jun 14, 2024
465f7d1
Merge remote-tracking branch 'origin/master' into execution_engine_in…
JacekGlen Jun 14, 2024
7f45490
make fmt
web-flow Jun 14, 2024
47ace1c
Fix build
JacekGlen Jun 14, 2024
22b3967
Merge branch 'execution_engine_integration_tests' of https://github.c…
JacekGlen Jun 14, 2024
5c99ed1
make fmt
web-flow Jun 14, 2024
58e34e1
Fix tests
JacekGlen Jun 14, 2024
865089b
Add keep_db_txn_open and transaction handling
JacekGlen Jun 17, 2024
2fe4b7e
Simplify transaction handling code
JacekGlen Jun 17, 2024
494d4fb
Merge branch 'master' into main-chain-db-txn-management
JacekGlen Jun 17, 2024
27e5837
Clean-up code
JacekGlen Jun 17, 2024
b487649
Remove redundant picohttpparser
JacekGlen Jun 17, 2024
0f22d41
Fix build error
JacekGlen Jun 17, 2024
46bbc03
make fmt
web-flow Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'master' into main-chain-db-txn-management
@JacekGlen
JacekGlen committed Jun 17, 2024
commit 494d4fb49d1fa107f7e907ac69963f6d9034a4e8
102 changes: 55 additions & 47 deletions silkworm/node/stagedsync/execution_engine_test.cpp
View file
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@
#include <silkworm/db/test_util/test_database_context.hpp>
#include <silkworm/infra/common/environment.hpp>
#include <silkworm/infra/test_util/log.hpp>
#include <silkworm/infra/test_util/task_runner.hpp>
#include <silkworm/node/common/node_settings.hpp>
#include <silkworm/node/common/preverified_hashes.hpp>
#include <silkworm/node/test_util/sample_blocks.hpp>
@@ -48,9 +49,9 @@ class ExecutionEngine_ForTest : public stagedsync::ExecutionEngine {
};

TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engine]") {
test_util::SetLogVerbosityGuard log_guard(log::Level::kNone);

asio::io_context io;
test_util::SetLogVerbosityGuard log_guard(log::Level::kNone);
test_util::TaskRunner runner;
Environment::set_stop_before_stage(db::stages::kSendersKey); // only headers, block hashes and bodies

auto db_context = db::test_util::TestDatabaseContext();
@@ -59,15 +60,15 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
.chaindata_env_config = db_context.get_env_config(),
.chain_config = db_context.get_chain_config(),
.parallel_fork_tracking_enabled = false,
.keep_db_txn_open = false,
.keep_db_txn_open = true,
};

db::RWAccess db_access{db_context.get_mdbx_env()};

ExecutionEngine_ForTest exec_engine{io, node_settings, db_access};
ExecutionEngine_ForTest exec_engine{runner.context(), node_settings, db_access};
exec_engine.open();

// auto& tx = exec_engine.main_chain_.tx(); // mdbx refuses to open a ROTxn when there is a RWTxn in the same thread
auto& tx = exec_engine.main_chain_.tx(); // mdbx refuses to open a ROTxn when there is a RWTxn in the same thread

const auto header0_hash = exec_engine.get_canonical_hash(0).value();
const silkworm::Hash header1_hash{0x7cb4dd3daba1f739d0c1ec7d998b4a2f6fd83019116455afa54ca4f49dfa0ad4_bytes32};
@@ -82,24 +83,24 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
}

SECTION("get_header by hash") {
// auto db_block_number = silkworm::db::read_block_number(tx, header1_hash);
// silkworm::Block db_block;
// auto db_read = silkworm::db::read_block(tx, header1_hash, *db_block_number, db_block);
// REQUIRE(db_read);
auto db_block_number = silkworm::db::read_block_number(tx, header1_hash);
silkworm::Block db_block;
auto db_read = silkworm::db::read_block(tx, header1_hash, *db_block_number, db_block);
REQUIRE(db_read);

auto header1 = exec_engine.get_header(header1_hash);
REQUIRE(header1.has_value());
// CHECK(header1->hash() == db_block.header.hash());
CHECK(header1->hash() == db_block.header.hash());
CHECK(header1->number == 1);
}

SECTION("get_header by hash not found") {
const silkworm::Hash header_not_found_hash{0x00000000000000000000000000000000000000000000000000000000deadbeef_bytes32};

// auto db_block_number = silkworm::db::read_block_number(tx, header_not_found_hash);
// silkworm::Block db_block;
// auto db_read = silkworm::db::read_block(tx, header_not_found_hash, *db_block_number, db_block);
// REQUIRE(!db_read);
auto db_block_number = silkworm::db::read_block_number(tx, header_not_found_hash);
silkworm::Block db_block;
auto db_read = silkworm::db::read_block(tx, header_not_found_hash, *db_block_number, db_block);
REQUIRE(!db_read);

auto header = exec_engine.get_header(header_not_found_hash);
REQUIRE(!header.has_value());
@@ -178,7 +179,7 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto is_canonical = exec_engine.is_canonical(new_block_hash);
CHECK(!is_canonical);

auto verification = exec_engine.verify_chain(new_block_hash).get();
auto verification = runner.run(exec_engine.verify_chain(new_block_hash));
CHECK(!holds_alternative<ValidationError>(verification));
REQUIRE(holds_alternative<InvalidChain>(verification));
auto invalid_chain = std::get<InvalidChain>(verification);
@@ -209,19 +210,19 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto is_canonical3 = exec_engine.is_canonical(block3_hash);
CHECK(!is_canonical3);

auto verification1 = exec_engine.verify_chain(block1_hash).get();
auto verification1 = runner.run(exec_engine.verify_chain(block1_hash));
CHECK(!holds_alternative<ValidationError>(verification1));
REQUIRE(holds_alternative<ValidChain>(verification1));
auto valid_chain1 = std::get<ValidChain>(verification1);
CHECK(valid_chain1.current_head == BlockId{10, block1_hash});

auto verification2 = exec_engine.verify_chain(block2_hash).get();
auto verification2 = runner.run(exec_engine.verify_chain(block2_hash));
CHECK(!holds_alternative<ValidationError>(verification2));
REQUIRE(holds_alternative<ValidChain>(verification2));
auto valid_chain2 = std::get<ValidChain>(verification2);
CHECK(valid_chain2.current_head == BlockId{11, block2_hash});

auto verification3 = exec_engine.verify_chain(block3_hash).get();
auto verification3 = runner.run(exec_engine.verify_chain(block3_hash));
CHECK(!holds_alternative<ValidationError>(verification3));
REQUIRE(holds_alternative<ValidChain>(verification3));
auto valid_chain3 = std::get<ValidChain>(verification3);
@@ -352,7 +353,7 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin

auto new_block_hash = new_block->header.hash();

auto verification = exec_engine.verify_chain(new_block_hash).get();
auto verification = runner.run(exec_engine.verify_chain(new_block_hash));
REQUIRE(holds_alternative<ValidChain>(verification));

auto fcu_updated = exec_engine.notify_fork_choice_update(new_block_hash, {}, {});
@@ -400,9 +401,9 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block2_hash = block2->header.hash();
auto block3_hash = block3->header.hash();

exec_engine.verify_chain(block1_hash).get();
exec_engine.verify_chain(block2_hash).get();
exec_engine.verify_chain(block3_hash).get();
runner.run(exec_engine.verify_chain(block1_hash));
runner.run(exec_engine.verify_chain(block2_hash));
runner.run(exec_engine.verify_chain(block3_hash));

auto fcu_updated = exec_engine.notify_fork_choice_update(block1_hash, {}, {});
CHECK(fcu_updated);
@@ -434,9 +435,9 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block2_hash = block2->header.hash();
auto block3_hash = block3->header.hash();

exec_engine.verify_chain(block1_hash).get();
exec_engine.verify_chain(block2_hash).get();
exec_engine.verify_chain(block3_hash).get();
runner.run(exec_engine.verify_chain(block1_hash));
runner.run(exec_engine.verify_chain(block2_hash));
runner.run(exec_engine.verify_chain(block3_hash));

auto fcu_updated = exec_engine.notify_fork_choice_update(block3_hash, {}, {});
CHECK(fcu_updated);
@@ -548,11 +549,11 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block2a_hash = block2a->header.hash();
auto block2b_hash = block2b->header.hash();

auto verification1 = exec_engine.verify_chain(block1b_hash).get();
auto verification1 = runner.run(exec_engine.verify_chain(block1b_hash));
CHECK(!holds_alternative<ValidationError>(verification1));
REQUIRE(holds_alternative<ValidChain>(verification1));

auto verification2 = exec_engine.verify_chain(block2b_hash).get();
auto verification2 = runner.run(exec_engine.verify_chain(block2b_hash));
CHECK(!holds_alternative<ValidationError>(verification2));
REQUIRE(holds_alternative<ValidChain>(verification2));

@@ -595,11 +596,11 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin

INFO(to_hex(block_f1_hash.bytes));

auto verification1 = exec_engine.verify_chain(block_f1_hash).get();
auto verification1 = runner.run(exec_engine.verify_chain(block_f1_hash));
CHECK(!holds_alternative<ValidationError>(verification1));
REQUIRE(holds_alternative<ValidChain>(verification1));

auto verification2 = exec_engine.verify_chain(block_f2_hash).get();
auto verification2 = runner.run(exec_engine.verify_chain(block_f2_hash));
CHECK(!holds_alternative<ValidationError>(verification2));
REQUIRE(holds_alternative<ValidChain>(verification2));

@@ -640,11 +641,11 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block2a_hash = block2a->header.hash();
auto block2b_hash = block2b->header.hash();

auto verification1 = exec_engine.verify_chain(block1b_hash).get();
auto verification1 = runner.run(exec_engine.verify_chain(block1b_hash));
CHECK(!holds_alternative<ValidationError>(verification1));
REQUIRE(holds_alternative<ValidChain>(verification1));

auto verification2 = exec_engine.verify_chain(block2b_hash).get();
auto verification2 = runner.run(exec_engine.verify_chain(block2b_hash));
CHECK(!holds_alternative<ValidationError>(verification2));
REQUIRE(holds_alternative<ValidChain>(verification2));

@@ -674,17 +675,24 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block1 = generate_sample_child_blocks(current_head);
auto block2 = generate_sample_child_blocks(block1->header);

// auto block1_hash = block1->header.hash();
// auto block2_hash = block2->header.hash();
auto block1_hash = block1->header.hash();
auto block2_hash = block2->header.hash();

// CHECK(!db::read_block_number(tx, block1_hash).has_value());
// CHECK(!db::read_block_number(tx, block2_hash).has_value());
CHECK(!db::read_block_number(tx, block1_hash).has_value());
CHECK(!db::read_block_number(tx, block2_hash).has_value());

auto blocks = std::vector<std::shared_ptr<Block>>{block1, block2};
exec_engine.insert_blocks(blocks);

// CHECK(db::read_block_number(tx, block1_hash).has_value());
// CHECK(db::read_block_number(tx, block2_hash).has_value());
CHECK(db::read_block_number(tx, block1_hash).has_value());
CHECK(db::read_block_number(tx, block2_hash).has_value());

tx.commit_and_renew(); // exec_engine.insert_blocks() automatically commits every 1000 blocks
exec_engine.close();

auto tx2 = db_access.start_ro_tx();
CHECK(db::read_block_number(tx2, block1_hash).has_value());
CHECK(db::read_block_number(tx2, block2_hash).has_value());
}

SECTION("verify_chain updates chain database") {
@@ -694,15 +702,15 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block1_hash = block1->header.hash();
auto block2_hash = block2->header.hash();

// CHECK(!db::read_block_number(tx, block1_hash).has_value());
// CHECK(!db::read_block_number(tx, block2_hash).has_value());
CHECK(!db::read_block_number(tx, block1_hash).has_value());
CHECK(!db::read_block_number(tx, block2_hash).has_value());

auto blocks = std::vector<std::shared_ptr<Block>>{block1, block2};
exec_engine.insert_blocks(blocks);
exec_engine.verify_chain(block2_hash).get();
runner.run(exec_engine.verify_chain(block2_hash));

// CHECK(db::read_block_number(tx, block1_hash).has_value());
// CHECK(db::read_block_number(tx, block2_hash).has_value());
CHECK(db::read_block_number(tx, block1_hash).has_value());
CHECK(db::read_block_number(tx, block2_hash).has_value());

exec_engine.close();

@@ -719,16 +727,16 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
auto block1_hash = block1->header.hash();
auto block2_hash = block2->header.hash();

// CHECK(!db::read_block_number(tx, block1_hash).has_value());
// CHECK(!db::read_block_number(tx, block2_hash).has_value());
CHECK(!db::read_block_number(tx, block1_hash).has_value());
CHECK(!db::read_block_number(tx, block2_hash).has_value());

auto blocks = std::vector<std::shared_ptr<Block>>{block1, block2};
exec_engine.insert_blocks(blocks);
exec_engine.verify_chain(block2_hash).get();
runner.run(exec_engine.verify_chain(block2_hash));
exec_engine.notify_fork_choice_update(block2_hash, current_head_id.hash, {});

// CHECK(db::read_block_number(tx, block1_hash).has_value());
// CHECK(db::read_block_number(tx, block2_hash).has_value());
CHECK(db::read_block_number(tx, block1_hash).has_value());
CHECK(db::read_block_number(tx, block2_hash).has_value());

exec_engine.close();

12 changes: 3 additions & 9 deletions silkworm/node/stagedsync/forks/main_chain.cpp
View file
Original file line number Diff line number Diff line change
@@ -161,9 +161,7 @@ std::optional<BlockId> MainChain::find_forking_point(const Hash& header_hash) co

bool MainChain::is_finalized_canonical(BlockId block) const {
TransactionHandler tx_handler{tx_, db_access_, node_settings_.keep_db_txn_open};
if (block.number > last_fork_choice_.number) {
return false;
}
if (block.number > last_fork_choice_.number) return false;
return (interim_canonical_chain_.get_hash(block.number) == block.hash);
}

@@ -497,12 +495,8 @@ bool MainChain::extends(BlockId block, BlockId supposed_parent) const {
bool MainChain::is_finalized_canonical(Hash block_hash) const {
TransactionHandler tx_handler{tx_, db_access_, node_settings_.keep_db_txn_open};
auto header = get_header(block_hash);
if (!header) {
return false;
}
if (header->number > last_fork_choice_.number) {
return false;
}
if (!header) return false;
if (header->number > last_fork_choice_.number) return false;
auto canonical_hash_at_same_height = interim_canonical_chain_.get_hash(header->number);
return canonical_hash_at_same_height == block_hash;
}
You are viewing a condensed version of this merge commit. You can view the full changes here.