Skip to content

Commit

Permalink
Merge branch 'GH-259-avoid-voting' into GH-135-update-fsi
Browse files Browse the repository at this point in the history
  • Loading branch information
heifner committed Jun 10, 2024
2 parents 7b0ed77 + 7b47b0e commit 10e7728
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 77 deletions.
19 changes: 10 additions & 9 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,7 @@ struct controller_impl {
chain_id( chain_id ),
read_mode( cfg.read_mode ),
thread_pool(),
my_finalizers(fc::time_point::now(), cfg.finalizers_dir / "safety.dat"),
my_finalizers(cfg.finalizers_dir / "safety.dat"),
wasmif( conf.wasm_runtime, conf.eosvmoc_tierup, db, conf.state_dir, conf.eosvmoc_config, !conf.profile_accounts.empty() )
{
assert(cfg.chain_thread_pool_size > 0);
Expand Down Expand Up @@ -1323,15 +1323,17 @@ struct controller_impl {
return fork_db.apply_l<bool>([&](const auto& forkdb_l) {
block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id());
fork_db.switch_to(fork_database::in_use_t::legacy); // apply block uses to know what types to create
block_state_ptr prev = forkdb.get_block(legacy->previous(), include_root_t::yes);
assert(prev);
if( apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}) ) {
fc::scoped_exit<std::function<void()>> e([&]{fork_db.switch_to(fork_database::in_use_t::both);});
// irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna()
assert(legacy->action_mroot_savanna);
block_state_ptr prev = forkdb.get_block(legacy->previous(), include_root_t::yes);
assert(prev);
transition_add_to_savanna_fork_db(forkdb, legacy, bsp, prev);
return true;
}
// add to forkdb as it expects root != head
transition_add_to_savanna_fork_db(forkdb, legacy, bsp, prev);
fork_db.switch_to(fork_database::in_use_t::legacy);
return false;
});
Expand Down Expand Up @@ -3699,8 +3701,7 @@ struct controller_impl {
return;

// Each finalizer configured on the node which is present in the active finalizer policy may create and sign a vote.
my_finalizers.maybe_vote(
*bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message_ptr& vote) {
my_finalizers.maybe_vote(bsp, [&](const vote_message_ptr& vote) {
// net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal
emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}, __FILE__, __LINE__);

Expand Down Expand Up @@ -4612,8 +4613,8 @@ struct controller_impl {
wasmif.code_block_num_last_used(code_hash, vm_type, vm_version, block_num);
}

void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys) {
my_finalizers.set_keys(finalizer_keys);
void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting) {
my_finalizers.set_keys(finalizer_keys, enable_immediate_voting);
}

bool irreversible_mode() const { return read_mode == db_read_mode::IRREVERSIBLE; }
Expand Down Expand Up @@ -5763,8 +5764,8 @@ void controller::code_block_num_last_used(const digest_type& code_hash, uint8_t
return my->code_block_num_last_used(code_hash, vm_type, vm_version, block_num);
}

void controller::set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys) {
my->set_node_finalizer_keys(finalizer_keys);
void controller::set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting) {
my->set_node_finalizer_keys(finalizer_keys, enable_immediate_voting);
}

/// Protocol feature activation handlers:
Expand Down
4 changes: 3 additions & 1 deletion libraries/chain/finality/finalizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() {
}

// ----------------------------------------------------------------------------------------
void my_finalizers_t::set_keys(const std::map<std::string, std::string>& finalizer_keys) {
void my_finalizers_t::set_keys(const std::map<std::string, std::string>& finalizer_keys, bool enable_immediate_voting) {
if (finalizer_keys.empty())
return;

Expand All @@ -274,6 +274,8 @@ void my_finalizers_t::set_keys(const std::map<std::string, std::string>& finaliz

// now only inactive finalizers remain in safety_info => move it to inactive_safety_info
inactive_safety_info = std::move(safety_info);

enable_voting = enable_immediate_voting;
}


Expand Down
44 changes: 24 additions & 20 deletions libraries/chain/fork_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ namespace eosio::chain {

explicit fork_database_impl() = default;

void open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
void open_impl( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
void close_impl( std::ofstream& out );
void add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator );
bool is_valid() const;
Expand Down Expand Up @@ -153,13 +153,13 @@ namespace eosio::chain {
fork_database_t<BSP>::~fork_database_t() = default; // close is performed in fork_database::~fork_database()

template<class BSP>
void fork_database_t<BSP>::open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
void fork_database_t<BSP>::open( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
std::lock_guard g( my->mtx );
my->open_impl( fork_db_file, ds, validator );
my->open_impl( desc, fork_db_file, ds, validator );
}

template<class BSP>
void fork_database_impl<BSP>::open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
void fork_database_impl<BSP>::open_impl( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
bsp_t _root = std::make_shared<bs_t>();
fc::raw::unpack( ds, *_root );
reset_root_impl( _root );
Expand All @@ -182,20 +182,20 @@ namespace eosio::chain {
if (!head)
std::filesystem::remove( fork_db_file );
EOS_ASSERT( head, fork_database_exception,
"could not find head while reconstructing fork database from file; "
"could not find head while reconstructing fork database from file; ${d} "
"'${filename}' is likely corrupted and has been removed",
("filename", fork_db_file) );
("d", desc)("filename", fork_db_file) );
}

auto candidate = index.template get<by_best_branch>().begin();
if( candidate == index.template get<by_best_branch>().end() || !bs_accessor_t::is_valid(**candidate) ) {
EOS_ASSERT( head->id() == root->id(), fork_database_exception,
"head not set to root despite no better option available; '${filename}' is likely corrupted",
("filename", fork_db_file) );
"head not set to root despite no better option available; ${d} '${filename}' is likely corrupted",
("d", desc)("filename", fork_db_file) );
} else {
EOS_ASSERT( !first_preferred( **candidate, *head ), fork_database_exception,
"head not set to best available option available; '${filename}' is likely corrupted",
("filename", fork_db_file) );
"head not set to the best available option available; ${d} '${filename}' is likely corrupted",
("d", desc)("filename", fork_db_file) );
}
}

Expand Down Expand Up @@ -354,17 +354,21 @@ namespace eosio::chain {
EOS_RETHROW_EXCEPTIONS( fork_database_exception, "serialized fork database is incompatible with configured protocol features" )
}

if (mark_valid == mark_valid_t::yes)
bs_accessor_t::set_valid(*n, true);

auto inserted = index.insert(n);
if( !inserted.second ) {
if( ignore_duplicate == ignore_duplicate_t::yes ) return;
EOS_THROW( fork_database_exception, "duplicate block added", ("id", n->id()) );
EOS_ASSERT(ignore_duplicate == ignore_duplicate_t::yes || inserted.second, fork_database_exception,
"duplicate block added: ${id}", ("id", n->id()));

if (mark_valid == mark_valid_t::yes) {
// if just inserted and was inserted already valid then no update needed
if (!inserted.second || !bs_accessor_t::is_valid(*n)) {
index.modify( inserted.first, []( auto& i ) {
bs_accessor_t::set_valid(*i, true);
} );
}
}

auto candidate = index.template get<by_best_branch>().begin();
if( bs_accessor_t::is_valid(**candidate) ) {
if (bs_accessor_t::is_valid(**candidate)) {
head = *candidate;
}
}
Expand Down Expand Up @@ -786,7 +790,7 @@ namespace eosio::chain {
{
// ---------- pre-Savanna format. Just a single fork_database_l ----------------
in_use = in_use_t::legacy;
fork_db_l.open(fork_db_file, ds, validator);
fork_db_l.open("legacy", fork_db_file, ds, validator);
break;
}

Expand All @@ -800,13 +804,13 @@ namespace eosio::chain {
bool legacy_valid { false };
fc::raw::unpack( ds, legacy_valid );
if (legacy_valid) {
fork_db_l.open(fork_db_file, ds, validator);
fork_db_l.open("legacy", fork_db_file, ds, validator);
}

bool savanna_valid { false };
fc::raw::unpack( ds, savanna_valid );
if (savanna_valid) {
fork_db_s.open(fork_db_file, ds, validator);
fork_db_s.open("savanna", fork_db_file, ds, validator);
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ namespace eosio::chain {
void set_to_read_window();
bool is_write_window() const;
void code_block_num_last_used(const digest_type& code_hash, uint8_t vm_type, uint8_t vm_version, uint32_t block_num);
void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys);
void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting = false);

private:
friend class apply_context;
Expand Down
25 changes: 15 additions & 10 deletions libraries/chain/include/eosio/chain/finality/finalizer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,41 +73,44 @@ namespace eosio::chain {
using fsi_map = std::map<bls_public_key, fsi_t>;

private:
const block_timestamp_type t_startup; // nodeos startup time, used for default safety_information
const std::filesystem::path persist_file_path; // where we save the safety data
mutable std::mutex mtx;
mutable fc::datastream<fc::cfile> persist_file; // we want to keep the file open for speed
std::map<bls_public_key, finalizer> finalizers; // the active finalizers for this node, loaded at startup, not mutated afterwards
fsi_map inactive_safety_info; // loaded at startup, not mutated afterwards
fsi_t default_fsi = fsi_t::unset_fsi(); // default provided at spring startup
mutable bool inactive_safety_info_written{false};
bool enable_voting = false;

public:
my_finalizers_t(block_timestamp_type startup_time, const std::filesystem::path& persist_file_path)
: t_startup(startup_time)
, persist_file_path(persist_file_path)
explicit my_finalizers_t(const std::filesystem::path& persist_file_path)
: persist_file_path(persist_file_path)
{}

template<class F> // thread safe
void maybe_vote(const finalizer_policy& fin_pol,
const block_state_ptr& bsp,
const digest_type& digest,
F&& process_vote) {
void maybe_vote(const block_state_ptr& bsp, F&& process_vote) {

if (finalizers.empty())
return;

assert(bsp->active_finalizer_policy);
const auto& fin_pol = *bsp->active_finalizer_policy;

std::vector<vote_message_ptr> votes;
votes.reserve(finalizers.size());

// Possible improvement in the future, look at locking only individual finalizers and releasing the lock for writing the file.
// Would require making sure that only the latest is ever written to the file and that the file access was protected separately.
std::unique_lock g(mtx);

if (!enable_voting && bsp->timestamp() < fc::time_point::now() - fc::seconds(30))
return;
enable_voting = true;

// first accumulate all the votes
for (const auto& f : fin_pol.finalizers) {
if (auto it = finalizers.find(f.public_key); it != finalizers.end()) {
vote_message_ptr vote_msg = it->second.maybe_vote(it->first, bsp, digest);
vote_message_ptr vote_msg = it->second.maybe_vote(it->first, bsp, bsp->strong_digest);
if (vote_msg)
votes.push_back(std::move(vote_msg));
}
Expand All @@ -131,7 +134,9 @@ namespace eosio::chain {
return std::ranges::all_of(std::views::keys(finalizers), std::forward<F>(f));
}

void set_keys(const std::map<std::string, std::string>& finalizer_keys); // only call on startup
/// only call on startup
/// @param enable_immediate_voting if true enable immediate voting on startup (for testing)
void set_keys(const std::map<std::string, std::string>& finalizer_keys, bool enable_immediate_voting);
void set_default_safety_information(const fsi_t& fsi);

// following two member functions could be private, but are used in testing, not thread safe
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/include/eosio/chain/fork_database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ namespace eosio::chain {
explicit fork_database_t();
~fork_database_t();

void open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
void open( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
void close( std::ofstream& out );

bsp_t get_block( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
Expand Down
18 changes: 17 additions & 1 deletion libraries/state_history/abi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -572,13 +572,29 @@ extern const char* const state_history_plugin_abi = R"({
{ "type": "uint32", "name": "account_net_usage_average_window" }
]
},
{
"name": "finalizer_authority", "fields": [
{ "name": "description", "type": "string" },
{ "name": "weight", "type": "uint64" },
{ "name": "public_key", "type": "bytes" }
]
},
{
"name": "finalizer_policy", "fields": [
{ "name": "generation", "type": "uint32" },
{ "name": "threshold", "type": "uint64" },
{ "name": "finalizers", "type": "finalizer_authority[]" }
]
},
{
"name": "finality_data", "fields": [
{ "name": "major_version", "type": "uint32" },
{ "name": "minor_version", "type": "uint32" },
{ "name": "active_finalizer_policy_generation", "type": "uint32" },
{ "name": "final_on_strong_qc_block_num", "type": "uint32" },
{ "name": "action_mroot", "type": "checksum256" },
{ "name": "base_digest", "type": "checksum256" }
{ "name": "base_digest", "type": "checksum256" },
{ "name": "proposed_finalizer_policy", "type": "finalizer_policy?" }
]
}
],
Expand Down
4 changes: 2 additions & 2 deletions libraries/testing/tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1253,7 +1253,7 @@ namespace eosio::testing {
("pop", pop.to_string()));
}

control->set_node_finalizer_keys(local_finalizer_keys);
control->set_node_finalizer_keys(local_finalizer_keys, true);

fc::mutable_variant_object fin_policy_variant;
fin_policy_variant("threshold", input.threshold);
Expand All @@ -1271,7 +1271,7 @@ namespace eosio::testing {
auto [privkey, pubkey, pop] = get_bls_key(name);
local_finalizer_keys[pubkey.to_string()] = privkey.to_string();
}
control->set_node_finalizer_keys(local_finalizer_keys);
control->set_node_finalizer_keys(local_finalizer_keys, true);
}

base_tester::set_finalizers_output_t base_tester::set_active_finalizers(std::span<const account_name> names) {
Expand Down
7 changes: 6 additions & 1 deletion plugins/net_plugin/net_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2393,8 +2393,10 @@ namespace eosio {
("ne", sync_next_expected_num)("id", id.str().substr( 8, 16 )) );
}
auto chain_info = my_impl->get_chain_info();
if( sync_state == lib_catchup || num < chain_info.lib_num )
if( sync_state == lib_catchup || num < chain_info.lib_num ) {
c->send_handshake();
return false;
}
set_state( head_catchup );
{
fc::lock_guard g_conn( c->conn_mtx );
Expand Down Expand Up @@ -2703,6 +2705,9 @@ namespace eosio {
}

void dispatch_manager::bcast_vote_msg( uint32_t exclude_peer, send_buffer_type msg ) {
if (my_impl->sync_master->syncing_from_peer())
return;

my_impl->connections.for_each_block_connection( [exclude_peer, msg{std::move(msg)}]( auto& cp ) {
if( !cp->current() ) return true;
if( cp->connection_id == exclude_peer ) return true;
Expand Down
3 changes: 3 additions & 0 deletions tests/nodeos_chainbase_allocation_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ def isSetProdsBlockNumIrr():

producerNode.kill(signal.SIGTERM)

# wait for all blocks to arrive and be processed by irrNode
time.sleep(3)

# Create the snapshot and rename it to avoid name conflict later on
res = irrNode.createSnapshot()
beforeShutdownSnapshotPath = res["payload"]["snapshot_name"]
Expand Down
Loading

0 comments on commit 10e7728

Please sign in to comment.