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

IF: Do not use watermarks for savanna #55

Merged
merged 1 commit into from
Apr 19, 2024
Merged

IF: Do not use watermarks for savanna #55

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 27 additions & 26 deletions plugins/producer_plugin/producer_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,9 +639,11 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
}
}

void on_block_header(chain::account_name producer, uint32_t block_num, chain::block_timestamp_type timestamp) {
if (_producers.contains(producer))
_producer_watermarks.consider_new_watermark(producer, block_num, timestamp);
void on_block_header(const signed_block_ptr& block) {
if (!block->is_proper_svnn_block()) {
if (_producers.contains(block->producer))
_producer_watermarks.consider_new_watermark(block->producer, block->block_num(), block->timestamp);
}
}

void on_irreversible_block(const signed_block_ptr& lib) {
Expand Down Expand Up @@ -1341,15 +1343,15 @@ void producer_plugin_impl::plugin_startup() {
chain.set_node_finalizer_keys(_finalizer_keys);

_accepted_block_connection.emplace(chain.accepted_block().connect([this](const block_signal_params& t) {
const auto& [ block, id ] = t;
const auto& [ block, _ ] = t;
on_block(block);
}));
_accepted_block_header_connection.emplace(chain.accepted_block_header().connect([this](const block_signal_params& t) {
const auto& [ block, id ] = t;
on_block_header(block->producer, block->block_num(), block->timestamp);
const auto& [ block, _ ] = t;
on_block_header(block);
}));
_irreversible_block_connection.emplace(chain.irreversible_block().connect([this](const block_signal_params& t) {
const auto& [ block, id ] = t;
const auto& [ block, _ ] = t;
on_irreversible_block(block);
}));

Expand Down Expand Up @@ -1795,8 +1797,6 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
// copy as reference is invalidated by abort_block() below
const producer_authority scheduled_producer = chain.active_producers().get_scheduled_producer(block_time);

const auto current_watermark = _producer_watermarks.get_watermark(scheduled_producer.producer_name);

size_t num_relevant_signatures = 0;
scheduled_producer.for_each_key([&](const public_key_type& key) {
const auto& iter = _signature_providers.find(key);
Expand Down Expand Up @@ -1827,7 +1827,7 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {

if (in_producing_mode()) {
// determine if our watermark excludes us from producing at this point
if (current_watermark) {
if (auto current_watermark = _producer_watermarks.get_watermark(scheduled_producer.producer_name)) {
const block_timestamp_type block_timestamp{block_time};
if (current_watermark->first > head_block_num) {
elog("Not producing block because \"${producer}\" signed a block at a higher block number (${watermark}) than the current "
Expand Down Expand Up @@ -1881,25 +1881,26 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() {
fc_dlog(_log, "Starting block #${n} at ${time} producer ${p}", ("n", pending_block_num)("time", now)("p", scheduled_producer.producer_name));

try {
uint16_t blocks_to_confirm = 0;

auto block_state = chain.head_block_state_legacy(); // null means savanna is active
if (in_producing_mode() && block_state) { // only if savanna not enabled
// determine how many blocks this producer can confirm
// 1) if it is not a producer from this node, assume no confirmations (we will discard this block anyway)
// 2) if it is a producer on this node that has never produced, the conservative approach is to assume no
// confirmations to make sure we don't double sign after a crash
// 3) if it is a producer on this node where this node knows the last block it produced, safely set it -UNLESS-
// 4) the producer on this node's last watermark is higher (meaning on a different fork)
if (current_watermark) {
uint32_t watermark_bn = current_watermark->first;
if (watermark_bn < head_block_num) {
blocks_to_confirm = (uint16_t)(std::min<uint32_t>(std::numeric_limits<uint16_t>::max(), (head_block_num - watermark_bn)));
uint16_t blocks_to_confirm = 0;
if (in_producing_mode()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems more efficient to not grab the head_block_state_legacy() unless producing which is when it is needed.

if (auto block_state = chain.head_block_state_legacy()) { // only if savanna not enabled
// determine how many blocks this producer can confirm
// 1) if it is not a producer from this node, assume no confirmations (we will discard this block anyway)
// 2) if it is a producer on this node that has never produced, the conservative approach is to assume no
// confirmations to make sure we don't double sign after a crash
// 3) if it is a producer on this node where this node knows the last block it produced, safely set it -UNLESS-
// 4) the producer on this node's last watermark is higher (meaning on a different fork)
if (auto current_watermark = _producer_watermarks.get_watermark(scheduled_producer.producer_name)) {
uint32_t watermark_bn = current_watermark->first;
if (watermark_bn < head_block_num) {
blocks_to_confirm = (uint16_t)(std::min<uint32_t>(std::numeric_limits<uint16_t>::max(), (head_block_num - watermark_bn)));
}
}
}

// can not confirm irreversible blocks
blocks_to_confirm = (uint16_t)(std::min<uint32_t>(blocks_to_confirm, (head_block_num - block_state->dpos_irreversible_blocknum)));
// can not confirm irreversible blocks
blocks_to_confirm = (uint16_t)(std::min<uint32_t>(blocks_to_confirm, (head_block_num - block_state->dpos_irreversible_blocknum)));
}
}

auto features_to_activate = chain.get_preactivated_protocol_features();
Expand Down
Loading