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.

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

Normalize usage of LIB #918

Merged
merged 6 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
34 changes: 14 additions & 20 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,13 @@ struct controller_impl {
return fork_db.size();
}

block_handle fork_db_root()const {
return fork_db.apply<block_handle>(
[&](const auto& forkdb) {
return block_handle{forkdb.root()};
});
}

block_id_type fork_db_root_block_id() const {
assert(fork_db_has_root());
return fork_db.apply<block_id_type>([&](const auto& forkdb) { return forkdb.root()->id(); });
Expand All @@ -1111,11 +1118,6 @@ struct controller_impl {
return fork_db.apply<uint32_t>([&](const auto& forkdb) { return forkdb.root()->block_num(); });
}

block_timestamp_type fork_db_root_timestamp() const {
assert(fork_db_has_root());
return fork_db.apply<block_timestamp_type>([&](const auto& forkdb) { return forkdb.root()->timestamp(); });
}

// --------------- fork_db APIs ----------------------------------------------------------------------
template<typename ForkDB>
uint32_t pop_block(ForkDB& forkdb) {
Expand Down Expand Up @@ -1615,7 +1617,7 @@ struct controller_impl {

bool should_replay = start_block_num <= blog_head->block_num();
if (!should_replay) {
ilog( "no irreversible blocks need to be replayed" );
ilog( "no irreversible blocks need to be replayed from block log" );
}
return should_replay;
}
Expand Down Expand Up @@ -1687,7 +1689,7 @@ struct controller_impl {
}
transition_legacy_branch.clear(); // not needed after replay
auto end = fc::time_point::now();
ilog( "${n} irreversible blocks replayed", ("n", 1 + chain_head.block_num() - start_block_num) );
ilog( "${n} irreversible blocks replayed from block log", ("n", 1 + chain_head.block_num() - start_block_num) );
ilog( "replayed ${n} blocks in ${duration} seconds, ${mspb} ms/block",
("n", chain_head.block_num() + 1 - start_block_num)("duration", (end-start).count()/1000000)
("mspb", ((end-start).count()/1000.0)/(chain_head.block_num()-start_block_num)) );
Expand Down Expand Up @@ -1871,7 +1873,7 @@ struct controller_impl {
"Snapshot is invalid." );
blog.reset( chain_id, chain_head.block_num() + 1 );
}
ilog( "Snapshot loaded, lib: ${lib}", ("lib", chain_head.block_num()) );
ilog( "Snapshot loaded, head: ${h} : ${id}", ("h", chain_head.block_num())("id", chain_head.id()) );

init(startup_t::snapshot);
block_handle_accessor::apply_l<void>(chain_head, [&](auto& head) {
Expand Down Expand Up @@ -5351,20 +5353,12 @@ bool controller::fork_db_has_root() const {
return my->fork_db_has_root();
}

size_t controller::fork_db_size() const {
return my->fork_db_size();
}

uint32_t controller::last_irreversible_block_num() const {
return my->fork_db_root_block_num();
block_handle controller::fork_db_root()const {
return my->fork_db_root();
}

block_id_type controller::last_irreversible_block_id() const {
return my->fork_db_root_block_id();
}

time_point controller::last_irreversible_block_time() const {
return my->fork_db_root_timestamp().to_time_point();
size_t controller::fork_db_size() const {
return my->fork_db_size();
}

const dynamic_global_property_object& controller::get_dynamic_global_properties()const {
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/fork_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ namespace eosio::chain {
block_num_type new_lib = block_header::num_from_id(id);
block_num_type old_lib = block_header::num_from_id(pending_savanna_lib_id);
if (new_lib > old_lib) {
dlog("set pending savanna lib ${bn}: ${id}", ("bn", block_header::num_from_id(id))("id", id));
dlog("set fork db pending savanna lib ${bn}: ${id}", ("bn", block_header::num_from_id(id))("id", id));
pending_savanna_lib_id = id;
return true;
}
Expand Down
15 changes: 6 additions & 9 deletions libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,15 +328,12 @@ namespace eosio::chain {

void set_savanna_lib_id(const block_id_type& id);

bool fork_db_has_root() const;
size_t fork_db_size() const;

// thread-safe, applied LIB, fork db root
uint32_t last_irreversible_block_num() const;
// thread-safe, applied LIB, fork db root
block_id_type last_irreversible_block_id() const;
// thread-safe, applied LIB, fork db root
time_point last_irreversible_block_time() const;
// thread-safe
bool fork_db_has_root() const;
// thread-safe
block_handle fork_db_root()const;
// thread-safe
size_t fork_db_size() const;

// thread-safe, retrieves block according to fork db best branch which can change at any moment
signed_block_ptr fetch_block_by_number( uint32_t block_num )const;
Expand Down
4 changes: 2 additions & 2 deletions libraries/testing/include/eosio/testing/tester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,8 @@ namespace eosio::testing {
block_handle fork_db_head() const { return control->fork_db_head(); }

chain_id_type get_chain_id() const { return control->get_chain_id(); }
block_id_type last_irreversible_block_id() const { return control->last_irreversible_block_id(); }
uint32_t last_irreversible_block_num() const { return control->last_irreversible_block_num(); }
block_id_type last_irreversible_block_id() const { return control->fork_db_root().id(); }
uint32_t last_irreversible_block_num() const { return control->fork_db_root().block_num(); }
bool block_exists(const block_id_type& id) const { return control->block_exists(id); }

signed_block_ptr fetch_block_by_id(const block_id_type& id) const {
Expand Down
6 changes: 3 additions & 3 deletions libraries/testing/tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ namespace eosio::testing {
control->set_async_voting(async_t::no); // vote synchronously so we don't have to wait for votes
control->set_async_aggregation(async_t::no); // aggregate votes synchronously for `_check_for_vote_if_needed`

lib_id = control->fork_db_has_root() ? control->last_irreversible_block_id() : block_id_type{};
lib_id = control->fork_db_has_root() ? control->fork_db_root().id() : block_id_type{};
lib_number = block_header::num_from_id(lib_id);
lib_block = control->fetch_block_by_id(lib_id);
[[maybe_unused]] auto lib_connection = control->irreversible_block().connect([&](const block_signal_params& t) {
Expand Down Expand Up @@ -515,10 +515,10 @@ namespace eosio::testing {
auto head_block_number = control->head().block_num();
auto producer = control->head_active_producers().get_scheduled_producer(block_time);

auto last_produced_block_num = control->last_irreversible_block_num();
auto last_produced_block_num = control->fork_db_root().block_num();
auto itr = last_produced_block.find(producer.producer_name);
if (itr != last_produced_block.end()) {
last_produced_block_num = std::max(control->last_irreversible_block_num(), block_header::num_from_id(itr->second));
last_produced_block_num = std::max(control->fork_db_root().block_num(), block_header::num_from_id(itr->second));
}

unapplied_transactions.add_aborted( control->abort_block() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ namespace eosio::chain::plugin_interface {
namespace methods {
using get_block_by_id = method_decl<chain_plugin_interface, signed_block_ptr(const block_id_type& block_id)>;
using get_head_block_id = method_decl<chain_plugin_interface, block_id_type ()>;

using get_last_irreversible_block_number = method_decl<chain_plugin_interface, uint32_t ()>;
}

namespace incoming {
Expand Down
17 changes: 9 additions & 8 deletions plugins/chain_plugin/account_query_db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,11 @@ namespace eosio::chain_apis {
auto start = fc::time_point::now();
const auto& index = controller.db().get_index<chain::permission_index>().indices().get<by_id>();

// build a initial time to block number map
const auto lib_num = controller.last_irreversible_block_num();
const auto head_num = controller.head().block_num();
// build an initial time to block number map
const auto fork_root_num = controller.fork_db_root().block_num();
const auto head_num = controller.head().block_num();

for (uint32_t block_num = lib_num + 1; block_num <= head_num; block_num++) {
for (uint32_t block_num = fork_root_num + 1; block_num <= head_num; block_num++) {
const auto block_p = controller.fetch_block_by_number(block_num);
EOS_ASSERT(block_p, chain::plugin_exception, "cannot fetch reversible block ${block_num}, required for account_db initialization", ("block_num", block_num));
time_to_block_num.emplace(block_p->timestamp.to_time_point(), block_num);
Expand Down Expand Up @@ -212,11 +212,12 @@ namespace eosio::chain_apis {
}

uint32_t last_updated_time_to_height( const fc::time_point& last_updated) {
const auto lib_num = controller.last_irreversible_block_num();
const auto lib_time = controller.last_irreversible_block_time();
const auto fork_root = controller.fork_db_root();
const auto fork_root_num = fork_root.block_num();
const auto fork_root_time = fork_root.block_time();

uint32_t last_updated_height = lib_num;
if (last_updated > lib_time) {
uint32_t last_updated_height = fork_root_num;
if (last_updated > fork_root_time) {
const auto iter = time_to_block_num.find(last_updated);
EOS_ASSERT(iter != time_to_block_num.end(), chain::plugin_exception, "invalid block time encountered in on-chain accounts ${time}", ("time", last_updated));
last_updated_height = iter->second;
Expand Down
25 changes: 11 additions & 14 deletions plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ class chain_plugin_impl {
// method provider handles
methods::get_block_by_id::method_type::handle get_block_by_id_provider;
methods::get_head_block_id::method_type::handle get_head_block_id_provider;
methods::get_last_irreversible_block_number::method_type::handle get_last_irreversible_block_number_provider;

// scoped connections for chain controller
std::optional<scoped_connection> accepted_block_header_connection;
Expand Down Expand Up @@ -1028,11 +1027,6 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) {
return chain->head().id();
} );

get_last_irreversible_block_number_provider = app().get_method<methods::get_last_irreversible_block_number>().register_provider(
[this]() {
return chain->last_irreversible_block_num();
} );

// relay signals to channels
accepted_block_header_connection = chain->accepted_block_header().connect(
[this]( const block_signal_params& t ) {
Expand Down Expand Up @@ -1290,16 +1284,19 @@ const string read_only::KEYi64 = "i64";
read_only::get_info_results read_only::get_info(const read_only::get_info_params&, const fc::time_point&) const {
const auto& rm = db.get_resource_limits_manager();

auto head_id = db.head().id();
auto lib_id = db.last_irreversible_block_id();
auto fhead_id = db.fork_db_head().id();
auto head = db.head();
auto head_id = head.id();
auto fork_root = db.fork_db_root();
auto fork_head = db.fork_db_head();
Copy link
Contributor

Choose a reason for hiding this comment

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

Now this seems inconsistent. Why do we have both fork_root and fork_db_root at different spots in the code?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like fork_root because it sounds like is the the root of a fork, not of fork_db. If we are going to spell it out, I think it should be fork_db_root everywhere, although I did prefer the shorter froot :-).

auto fork_root_id = fork_root.id();
auto fork_head_id = fork_head.id();

return {
itoh(static_cast<uint32_t>(app().version())),
db.get_chain_id(),
block_header::num_from_id(head_id),
block_header::num_from_id(lib_id),
lib_id,
block_header::num_from_id(fork_root_id),
fork_root_id,
head_id,
db.head().block_time(),
db.head().producer(),
Expand All @@ -1310,13 +1307,13 @@ read_only::get_info_results read_only::get_info(const read_only::get_info_params
//std::bitset<64>(db.get_dynamic_global_properties().recent_slots_filled).to_string(),
//__builtin_popcountll(db.get_dynamic_global_properties().recent_slots_filled) / 64.0,
app().version_string(),
block_header::num_from_id(fhead_id),
fhead_id,
block_header::num_from_id(fork_head_id),
fork_head_id,
app().full_version_string(),
rm.get_total_cpu_weight(),
rm.get_total_net_weight(),
db.earliest_available_block_num(),
db.last_irreversible_block_time()
fork_root.block_time()
};
}

Expand Down
6 changes: 3 additions & 3 deletions plugins/net_plugin/include/eosio/net_plugin/protocol.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ namespace eosio {
fc::sha256 token; ///< digest of time to prove we own the private key of the key above
chain::signature_type sig; ///< signature for the digest
string p2p_address;
uint32_t last_irreversible_block_num = 0;
block_id_type last_irreversible_block_id;
uint32_t fork_root_num = 0;
block_id_type fork_root_id;
uint32_t fork_head_num = 0;
block_id_type fork_head_id;
string os;
Expand Down Expand Up @@ -154,7 +154,7 @@ FC_REFLECT( eosio::chain_size_message,
FC_REFLECT( eosio::handshake_message,
(network_version)(chain_id)(node_id)(key)
(time)(token)(sig)(p2p_address)
(last_irreversible_block_num)(last_irreversible_block_id)
(fork_root_num)(fork_root_id)
(fork_head_num)(fork_head_id)
(os)(agent)(generation) )
FC_REFLECT( eosio::go_away_message, (reason)(node_id) )
Expand Down
Loading