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

Jump to bottom

IF: Support replay over instant finality transition #2287

Merged
merged 7 commits into from
Mar 7, 2024
Merged

IF: Support replay over instant finality transition #2287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
574ac4c
GH-2141 Mainly refactoring. complete_block now holds a block_handle.
heifner Mar 5, 2024
ac673af
GH-2141 More descriptive error on block signed by unexpected key
heifner Mar 6, 2024
09dcff4
GH-2141 Open fork database after replay of block log
heifner Mar 6, 2024
07f3282
Merge remote-tracking branch 'origin/hotstuff_integration' into GH-21…
heifner Mar 6, 2024
dc51cb8
GH-2141 Add include_root option to search_on_branch. Rename check_roo…
heifner Mar 6, 2024
81388a5
GH-2141 Add back replay tests
heifner Mar 6, 2024
7169abc
GH-2141 Fix log output
heifner Mar 7, 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
4 changes: 2 additions & 2 deletions libraries/chain/block_header_state_legacy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ namespace eosio::chain {
std::tie(is_satisfied, relevant_sig_count) = producer_authority::keys_satisfy_and_relevant(keys, valid_block_signing_authority);

EOS_ASSERT(relevant_sig_count == keys.size(), wrong_signing_key,
"block signed by unexpected key",
("signing_keys", keys)("authority", valid_block_signing_authority));
"block signed by unexpected key: ${signing_keys}, expected: ${authority}. ${c} != ${s}",
greg7mdp marked this conversation as resolved.
Show resolved Hide resolved
("signing_keys", keys)("authority", valid_block_signing_authority)("c", relevant_sig_count)("s", keys.size()));

EOS_ASSERT(is_satisfied, wrong_signing_key,
"block signatures do not satisfy the block signing authority",
Expand Down
476 changes: 263 additions & 213 deletions libraries/chain/controller.cpp
View file
Open in desktop

Large diffs are not rendered by default.

44 changes: 23 additions & 21 deletions libraries/chain/fork_database.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ namespace eosio::chain {
void close_impl( const std::filesystem::path& fork_db_file );
void add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator );

bsp_t get_block_impl( const block_id_type& id, check_root_t check_root = check_root_t::no ) const;
bsp_t get_block_impl( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
bool block_exists_impl( const block_id_type& id ) const;
void reset_root_impl( const bsp_t& root_bs );
void rollback_head_to_root_impl();
Expand All @@ -139,8 +139,8 @@ namespace eosio::chain {
branch_t fetch_branch_impl( const block_id_type& h, uint32_t trim_after_block_num ) const;
block_branch_t fetch_block_branch_impl( const block_id_type& h, uint32_t trim_after_block_num ) const;
full_branch_t fetch_full_branch_impl(const block_id_type& h) const;
bsp_t search_on_branch_impl( const block_id_type& h, uint32_t block_num ) const;
bsp_t search_on_head_branch_impl( uint32_t block_num ) const;
bsp_t search_on_branch_impl( const block_id_type& h, uint32_t block_num, include_root_t include_root ) const;
bsp_t search_on_head_branch_impl( uint32_t block_num, include_root_t include_root ) const;
void mark_valid_impl( const bsp_t& h );
branch_pair_t fetch_branch_from_impl( const block_id_type& first, const block_id_type& second ) const;

Expand Down Expand Up @@ -377,7 +377,7 @@ namespace eosio::chain {
EOS_ASSERT( root, fork_database_exception, "root not yet set" );
EOS_ASSERT( n, fork_database_exception, "attempt to add null block state" );

auto prev_bh = get_block_impl( n->previous(), check_root_t::yes );
auto prev_bh = get_block_impl( n->previous(), include_root_t::yes );
EOS_ASSERT( prev_bh, unlinkable_block_exception,
"forkdb unlinkable block ${id} previous ${p}", ("id", n->id())("p", n->previous()) );

Expand Down Expand Up @@ -511,13 +511,17 @@ namespace eosio::chain {
}

template<class BSP>
BSP fork_database_t<BSP>::search_on_branch( const block_id_type& h, uint32_t block_num ) const {
BSP fork_database_t<BSP>::search_on_branch( const block_id_type& h, uint32_t block_num, include_root_t include_root /* = include_root_t::no */ ) const {
std::lock_guard g( my->mtx );
return my->search_on_branch_impl( h, block_num );
return my->search_on_branch_impl( h, block_num, include_root );
}

template<class BSP>
BSP fork_database_impl<BSP>::search_on_branch_impl( const block_id_type& h, uint32_t block_num ) const {
BSP fork_database_impl<BSP>::search_on_branch_impl( const block_id_type& h, uint32_t block_num, include_root_t include_root ) const {
if( include_root == include_root_t::yes && root->id() == h ) {
return root;
}

for( auto i = index.find(h); i != index.end(); i = index.find( (*i)->previous() ) ) {
if ((*i)->block_num() == block_num)
return *i;
Expand All @@ -527,14 +531,14 @@ namespace eosio::chain {
}

template<class BSP>
BSP fork_database_t<BSP>::search_on_head_branch( uint32_t block_num ) const {
BSP fork_database_t<BSP>::search_on_head_branch( uint32_t block_num, include_root_t include_root /* = include_root_t::no */ ) const {
std::lock_guard g(my->mtx);
return my->search_on_head_branch_impl(block_num);
return my->search_on_head_branch_impl(block_num, include_root);
}

template<class BSP>
BSP fork_database_impl<BSP>::search_on_head_branch_impl( uint32_t block_num ) const {
return search_on_branch_impl(head->id(), block_num);
BSP fork_database_impl<BSP>::search_on_head_branch_impl( uint32_t block_num, include_root_t include_root ) const {
return search_on_branch_impl(head->id(), block_num, include_root);
}

/**
Expand Down Expand Up @@ -666,15 +670,15 @@ namespace eosio::chain {

template<class BSP>
BSP fork_database_t<BSP>::get_block(const block_id_type& id,
check_root_t check_root /* = check_root_t::no */) const {
include_root_t include_root /* = include_root_t::no */) const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the /* = include_root_t::no */

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It provides doc on what the default is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought include_root was not a parameter with a default value. Don't make any changes for this PR. In general, should we stay away from default parameters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I'm not a fan of default parameters. Here I was just changing the name.

std::lock_guard g( my->mtx );
return my->get_block_impl(id, check_root);
return my->get_block_impl(id, include_root);
}

template<class BSP>
BSP fork_database_impl<BSP>::get_block_impl(const block_id_type& id,
check_root_t check_root /* = check_root_t::no */) const {
if( check_root == check_root_t::yes && root->id() == id ) {
include_root_t include_root /* = include_root_t::no */) const {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the /* = include_root_t::no */

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It provides doc on what the default is.

if( include_root == include_root_t::yes && root->id() == id ) {
return root;
}
auto itr = index.find( id );
Expand All @@ -698,7 +702,7 @@ namespace eosio::chain {

fork_database::fork_database(const std::filesystem::path& data_dir)
: data_dir(data_dir)
// currently needed because chain_head is accessed before fork database open
// genesis starts with legacy
, fork_db_l{std::make_unique<fork_database_legacy_t>(fork_database_legacy_t::legacy_magic_number)}
{
}
Expand Down Expand Up @@ -751,19 +755,17 @@ namespace eosio::chain {
}
}

block_handle fork_database::switch_from_legacy(const block_handle& bh) {
void fork_database::switch_from_legacy(const block_handle& bh) {
// no need to close fork_db because we don't want to write anything out, file is removed on open
// threads may be accessing (or locked on mutex about to access legacy forkdb) so don't delete it until program exit
assert(legacy);
assert(std::holds_alternative<block_state_legacy_ptr>(bh.internal()));
block_state_legacy_ptr head = std::get<block_state_legacy_ptr>(bh.internal()); // will throw if called after transistion
auto new_head = std::make_shared<block_state>(*head);
assert(std::holds_alternative<block_state_ptr>(bh.internal()));
block_state_ptr new_head = std::get<block_state_ptr>(bh.internal());
fork_db_s = std::make_unique<fork_database_if_t>(fork_database_if_t::magic_number);
legacy = false;
apply_s<void>([&](auto& forkdb) {
forkdb.reset_root(new_head);
});
return block_handle{new_head};
}

block_branch_t fork_database::fetch_branch_from_head() const {
Expand Down
12 changes: 6 additions & 6 deletions libraries/chain/include/eosio/chain/fork_database.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace eosio::chain {
using block_branch_t = std::vector<signed_block_ptr>;
enum class mark_valid_t { no, yes };
enum class ignore_duplicate_t { no, yes };
enum class check_root_t { no, yes };
enum class include_root_t { no, yes };

// Used for logging of comparison values used for best fork determination
std::string log_fork_comparison(const block_state& bs);
Expand Down Expand Up @@ -52,7 +52,7 @@ namespace eosio::chain {
void open( const std::filesystem::path& fork_db_file, validator_t& validator );
void close( const std::filesystem::path& fork_db_file );

bsp_t get_block( const block_id_type& id, check_root_t check_root = check_root_t::no ) const;
bsp_t get_block( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
bool block_exists( const block_id_type& id ) const;

/**
Expand Down Expand Up @@ -107,12 +107,12 @@ namespace eosio::chain {
* Returns the block state with a block number of `block_num` that is on the branch that
* contains a block with an id of`h`, or the empty shared pointer if no such block can be found.
*/
bsp_t search_on_branch( const block_id_type& h, uint32_t block_num ) const;
bsp_t search_on_branch( const block_id_type& h, uint32_t block_num, include_root_t include_root = include_root_t::no ) const;

/**
* search_on_branch( head()->id(), block_num)
*/
bsp_t search_on_head_branch( uint32_t block_num ) const;
bsp_t search_on_head_branch( uint32_t block_num, include_root_t include_root = include_root_t::no ) const;

/**
* Given two head blocks, return two branches of the fork graph that
Expand Down Expand Up @@ -148,8 +148,8 @@ namespace eosio::chain {
void open( validator_t& validator );
void close();

// expected to be called from main thread, accesses chain_head
block_handle switch_from_legacy(const block_handle& bh);
// expected to be called from main thread
void switch_from_legacy(const block_handle& bh);

bool fork_db_if_present() const { return !!fork_db_s; }
bool fork_db_legacy_present() const { return !!fork_db_l; }
Expand Down
20 changes: 11 additions & 9 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,8 @@ add_test(NAME restart-scenarios-if-test-resync COMMAND tests/restart-scenarios-t
set_property(TEST restart-scenarios-if-test-resync PROPERTY LABELS nonparallelizable_tests)
add_test(NAME restart-scenarios-test-hard_replay COMMAND tests/restart-scenarios-test.py -c hardReplay -p4 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST restart-scenarios-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
# requires https://github.com/AntelopeIO/leap/issues/2141
#add_test(NAME restart-scenarios-if-test-hard_replay COMMAND tests/restart-scenarios-test.py -c hardReplay -p4 -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
#set_property(TEST restart-scenarios-if-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME restart-scenarios-if-test-hard_replay COMMAND tests/restart-scenarios-test.py -c hardReplay -p4 -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST restart-scenarios-if-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME restart-scenarios-test-none COMMAND tests/restart-scenarios-test.py -c none --kill-sig term -p4 -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST restart-scenarios-test-none PROPERTY LABELS nonparallelizable_tests)
add_test(NAME restart-scenarios-if-test-none COMMAND tests/restart-scenarios-test.py -c none --kill-sig term -p4 -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
Expand All @@ -206,11 +205,14 @@ add_test(NAME terminate-scenarios-test-hard_replay COMMAND tests/terminate-scena
set_property(TEST terminate-scenarios-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME terminate-scenarios-if-test-resync COMMAND tests/terminate-scenarios-test.py -c resync --terminate-at-block 10 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST terminate-scenarios-if-test-resync PROPERTY LABELS nonparallelizable_tests)
# requires https://github.com/AntelopeIO/leap/issues/2141
#add_test(NAME terminate-scenarios-if-test-replay COMMAND tests/terminate-scenarios-test.py -c replay --terminate-at-block 10 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
#set_property(TEST terminate-scenarios-if-test-replay PROPERTY LABELS nonparallelizable_tests)
#add_test(NAME terminate-scenarios-if-test-hard_replay COMMAND tests/terminate-scenarios-test.py -c hardReplay --terminate-at-block 10 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
#set_property(TEST terminate-scenarios-if-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME terminate-scenarios-if-test-replay COMMAND tests/terminate-scenarios-test.py -c replay --terminate-at-block 10 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST terminate-scenarios-if-test-replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME terminate-scenarios-if-test-hard_replay COMMAND tests/terminate-scenarios-test.py -c hardReplay --terminate-at-block 10 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST terminate-scenarios-if-test-hard_replay PROPERTY LABELS nonparallelizable_tests)
add_test(NAME terminate-scenarios-if-test-replay-pass-transition COMMAND tests/terminate-scenarios-test.py -c replay --terminate-at-block 150 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST terminate-scenarios-if-test-replay-pass-transition PROPERTY LABELS nonparallelizable_tests)
add_test(NAME terminate-scenarios-if-test-hard_replay-pass-transition COMMAND tests/terminate-scenarios-test.py -c hardReplay --terminate-at-block 150 --kill-sig term --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST terminate-scenarios-if-test-hard_replay-pass-transition PROPERTY LABELS nonparallelizable_tests)
add_test(NAME validate_dirty_db_test COMMAND tests/validate-dirty-db.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST validate_dirty_db_test PROPERTY LABELS nonparallelizable_tests)
add_test(NAME keosd_auto_launch_test COMMAND tests/keosd_auto_launch_test.py WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
Expand Down Expand Up @@ -285,7 +287,7 @@ set_property(TEST nodeos_under_min_avail_ram_if_lr_test PROPERTY LABELS long_run

add_test(NAME nodeos_irreversible_mode_lr_test COMMAND tests/nodeos_irreversible_mode_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST nodeos_irreversible_mode_lr_test PROPERTY LABELS long_running_tests)
# requires https://github.com/AntelopeIO/leap/issues/2141
# requires https://github.com/AntelopeIO/leap/issues/2286
#add_test(NAME nodeos_irreversible_mode_if_lr_test COMMAND tests/nodeos_irreversible_mode_test.py -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
#set_property(TEST nodeos_irreversible_mode_if_lr_test PROPERTY LABELS long_running_tests)

Expand Down
3 changes: 3 additions & 0 deletions tests/terminate-scenarios-test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@
if not cluster.waitOnClusterBlockNumSync(3):
errorExit("Cluster never stabilized")

# make sure enough blocks produced to verify truncate works on restart
cluster.getNode(0).waitForBlock(terminate+5)

Print("Kill cluster node instance.")
if cluster.killSomeEosInstances(1, killSignal) is False:
errorExit("Failed to kill Eos instances")
Expand Down
4 changes: 2 additions & 2 deletions unittests/protocol_feature_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1726,7 +1726,7 @@ BOOST_AUTO_TEST_CASE( producer_schedule_change_extension_test ) { try {
// ensure it is accepted (but rejected because it doesn't match expected state)
BOOST_REQUIRE_EXCEPTION(
remote.push_block(bad_block), wrong_signing_key,
fc_exception_message_is( "block signed by unexpected key" )
fc_exception_message_starts_with( "block signed by unexpected key" )
);
}

Expand All @@ -1752,7 +1752,7 @@ BOOST_AUTO_TEST_CASE( producer_schedule_change_extension_test ) { try {
// ensure it is rejected because it doesn't match expected state (but the extention was accepted)
BOOST_REQUIRE_EXCEPTION(
remote.push_block(bad_block), wrong_signing_key,
fc_exception_message_is( "block signed by unexpected key" )
fc_exception_message_starts_with( "block signed by unexpected key" )
);
}

Expand Down
Loading