Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Start block state creation early #6167

Merged
merged 17 commits into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions libraries/chain/block_header_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ namespace eosio { namespace chain {
result.blockroot_merkle = blockroot_merkle;
result.blockroot_merkle.append( id );

auto block_mroot = result.blockroot_merkle.get_root();

result.active_schedule = active_schedule;
result.pending_schedule = pending_schedule;
result.dpos_proposed_irreversible_blocknum = dpos_proposed_irreversible_blocknum;
Expand Down Expand Up @@ -143,7 +141,7 @@ namespace eosio { namespace chain {
*
* If the header specifies new_producers then apply them accordingly.
*/
block_header_state block_header_state::next( const signed_block_header& h, bool trust )const {
block_header_state block_header_state::next( const signed_block_header& h, bool skip_validate_signee )const {
EOS_ASSERT( h.timestamp != block_timestamp_type(), block_validate_exception, "", ("h",h) );
EOS_ASSERT( h.header_extensions.size() == 0, block_validate_exception, "no supported extensions" );

Expand Down Expand Up @@ -178,9 +176,8 @@ namespace eosio { namespace chain {
result.id = result.header.id();

// ASSUMPTION FROM controller_impl::apply_block = all untrusted blocks will have their signatures pre-validated here
if( !trust ) {
EOS_ASSERT( result.block_signing_key == result.signee(), wrong_signing_key, "block not signed by expected key",
("result.block_signing_key", result.block_signing_key)("signee", result.signee() ) );
if( !skip_validate_signee ) {
result.verify_signee( result.signee() );
}

return result;
Expand Down Expand Up @@ -236,6 +233,11 @@ namespace eosio { namespace chain {
return fc::crypto::public_key( header.producer_signature, sig_digest(), true );
}

void block_header_state::verify_signee( const public_key_type& signee )const {
EOS_ASSERT( block_signing_key == signee, wrong_signing_key, "block not signed by expected key",
("block_signing_key", block_signing_key)( "signee", signee ) );
}

void block_header_state::add_confirmation( const header_confirmation& conf ) {
for( const auto& c : confirmations )
EOS_ASSERT( c.producer != conf.producer, producer_double_confirm, "block already confirmed by this producer" );
Expand Down
6 changes: 3 additions & 3 deletions libraries/chain/block_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ namespace eosio { namespace chain {
static_cast<block_header&>(*block) = header;
}

block_state::block_state( const block_header_state& prev, signed_block_ptr b, bool trust )
:block_header_state( prev.next( *b, trust )), block( move(b) )
{ }
block_state::block_state( const block_header_state& prev, signed_block_ptr b, bool skip_validate_signee )
:block_header_state( prev.next( *b, skip_validate_signee )), block( move(b) )
{ }



Expand Down
85 changes: 68 additions & 17 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ struct controller_impl {

auto start = fc::time_point::now();
while( auto next = blog.read_block_by_num( head->block_num + 1 ) ) {
self.push_block( next, controller::block_status::irreversible );
replay_push_block( next, controller::block_status::irreversible );
if( next->block_num() % 100 == 0 ) {
std::cerr << std::setw(10) << next->block_num() << " of " << blog_head->block_num() <<"\r";
}
Expand All @@ -332,7 +332,7 @@ struct controller_impl {
int rev = 0;
while( auto obj = reversible_blocks.find<reversible_block_object,by_num>(head->block_num+1) ) {
++rev;
self.push_block( obj->get_block(), controller::block_status::validated );
replay_push_block( obj->get_block(), controller::block_status::validated );
}

ilog( "${n} reversible blocks replayed", ("n",rev) );
Expand Down Expand Up @@ -1178,13 +1178,18 @@ struct controller_impl {
start_block( b->timestamp, b->confirmed, s , producer_block_id);

std::vector<transaction_metadata_ptr> packed_transactions;
packed_transactions.reserve( b->transactions.size() );
for( const auto& receipt : b->transactions ) {
if( receipt.trx.contains<packed_transaction>()) {
auto& pt = receipt.trx.get<packed_transaction>();
auto mtrx = std::make_shared<transaction_metadata>( pt );
if( !self.skip_auth_check() ) {
mtrx->signing_keys_future = async_thread_pool( [this, mtrx]() {
return std::make_pair( this->chain_id, mtrx->trx.get_signature_keys( this->chain_id ) );
std::weak_ptr<transaction_metadata> mtrx_wp = mtrx;
mtrx->signing_keys_future = async_thread_pool( [chain_id = this->chain_id, mtrx_wp]() {
auto mtrx = mtrx_wp.lock();
return mtrx ?
std::make_pair( chain_id, mtrx->trx.get_signature_keys( chain_id ) ) :
std::make_pair( chain_id, decltype( mtrx->trx.get_signature_keys( chain_id ) ){} );
} );
}
packed_transactions.emplace_back( std::move( mtrx ) );
Expand Down Expand Up @@ -1251,19 +1256,38 @@ struct controller_impl {
}
} FC_CAPTURE_AND_RETHROW() } /// apply_block

std::future<block_state_ptr> create_block_state_future( const signed_block_ptr& b ) {
EOS_ASSERT( b, block_validate_exception, "null block" );

void push_block( const signed_block_ptr& b, controller::block_status s ) {
auto id = b->id();

// no reason for a block_state if fork_db already knows about block
auto existing = fork_db.get_block( id );
EOS_ASSERT( !existing, fork_database_exception, "we already know about this block: ${id}", ("id", id) );

auto prev = fork_db.get_block( b->previous );
EOS_ASSERT( prev, unlinkable_block_exception, "unlinkable block ${id}", ("id", id)("previous", b->previous) );

return async_thread_pool( [b, prev]() {
const bool skip_validate_signee = false;
return std::make_shared<block_state>( *prev, move( b ), skip_validate_signee );
} );
}

void push_block( std::future<block_state_ptr>& block_state_future ) {
controller::block_status s = controller::block_status::complete;
EOS_ASSERT(!pending, block_validate_exception, "it is not valid to push a block when there is a pending block");

auto reset_prod_light_validation = fc::make_scoped_exit([old_value=trusted_producer_light_validation, this]() {
trusted_producer_light_validation = old_value;
});
try {
EOS_ASSERT( b, block_validate_exception, "trying to push empty block" );
EOS_ASSERT( s != controller::block_status::incomplete, block_validate_exception, "invalid block status for a completed block" );
block_state_ptr new_header_state = block_state_future.get();
auto& b = new_header_state->block;
emit( self.pre_accepted_block, b );
bool trust = !conf.force_all_checks && (s == controller::block_status::irreversible || s == controller::block_status::validated);
auto new_header_state = fork_db.add( b, trust );

fork_db.add( new_header_state );
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling fork_database::add( block_state_ptr ) here instead of the original fork_database::add( signed_block_ptr bool ) skips some checks that were done in the original version.

create_block_state_future replicates those fork_database::add( signed_block_ptr, bool) checks, but it does so at a different time (and therefore potentially with different state of the fork database) than the call to fork_db.add here.

One of the two necessary checks, checking for duplicate blocks, is already handled in fork_database::add( block_state_ptr ) with the assertion that inserted.second is true.

The other check that is necessary to again here (so it could be done in fork_database::add( block_state_ptr )) is to ensure that the block still links to an existing block in the fork database. Without that check, it is possible to add a block into the fork database that does not link. For example, a user of controller could in theory:

  1. Call create_block_state_future on two different blocks A and B where both currently link to existing blocks in the fork database and where B builds off of the current head.
  2. Then calls push_block with the future of block A which causes: a fork switch, LIB advancement, and purging of orphaned branches which may include the block that the block B builds off of.
  3. Then calls push_block with the future of block B.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current code does not have more than one create_block_state_future in play at a time. It is single threaded from producer plugin on_incoming_block to push_block. The create_block_state_future creation of block_state which includes digest, merkle, and signee verification can run at the same time as abort_block but the rest of the order is all single threaded.

Now if we updated the code so that more than one create_block_state_future could be in flight at a time then that could certainly be a problem. That would take some work as it does verify previous is in the fork_db. Could create a version of create_block_state_future that takes the previous block so that a whole chain of blocks could be processed at once and then verify in fork_db add that the previous is there. However, that would require changing net_plugin to receive more than one block at a time or queue up blocks which would increase latency which is what this is attempting to reduce.

That said the extra check doesn't hurt and should be quick, so I'll go ahead and add it.


if (conf.trusted_producers.count(b->producer)) {
trusted_producer_light_validation = true;
};
Expand All @@ -1273,6 +1297,29 @@ struct controller_impl {
maybe_switch_forks( s );
}

} FC_LOG_AND_RETHROW( )
}

void replay_push_block( const signed_block_ptr& b, controller::block_status s ) {
self.validate_db_available_size();
self.validate_reversible_available_size();

EOS_ASSERT(!pending, block_validate_exception, "it is not valid to push a block when there is a pending block");

try {
EOS_ASSERT( b, block_validate_exception, "trying to push empty block" );
EOS_ASSERT( (s == controller::block_status::irreversible || s == controller::block_status::validated),
block_validate_exception, "invalid block status for replay" );
emit( self.pre_accepted_block, b );
const bool skip_validate_signee = !conf.force_all_checks;
auto new_header_state = fork_db.add( b, skip_validate_signee );

emit( self.accepted_block_header, new_header_state );

if ( read_mode != db_read_mode::IRREVERSIBLE ) {
maybe_switch_forks( s );
}

// on replay irreversible is not emitted by fork database, so emit it explicitly here
if( s == controller::block_status::irreversible )
emit( self.irreversible_block, new_header_state );
Expand All @@ -1299,13 +1346,13 @@ struct controller_impl {
auto branches = fork_db.fetch_branch_from( new_head->id, head->id );

for( auto itr = branches.second.begin(); itr != branches.second.end(); ++itr ) {
fork_db.mark_in_current_chain( *itr , false );
fork_db.mark_in_current_chain( *itr, false );
pop_block();
}
EOS_ASSERT( self.head_block_id() == branches.second.back()->header.previous, fork_database_exception,
"loss of sync between fork_db and chainbase during fork switch" ); // _should_ never fail
"loss of sync between fork_db and chainbase during fork switch" ); // _should_ never fail

for( auto ritr = branches.first.rbegin(); ritr != branches.first.rend(); ++ritr) {
for( auto ritr = branches.first.rbegin(); ritr != branches.first.rend(); ++ritr ) {
optional<fc::exception> except;
try {
apply_block( (*ritr)->block, (*ritr)->validated ? controller::block_status::validated : controller::block_status::complete );
Expand All @@ -1315,7 +1362,7 @@ struct controller_impl {
}
catch (const fc::exception& e) { except = e; }
if (except) {
elog("exception thrown while switching forks ${e}", ("e",except->to_detail_string()));
elog("exception thrown while switching forks ${e}", ("e", except->to_detail_string()));

// ritr currently points to the block that threw
// if we mark it invalid it will automatically remove all forks built off it.
Expand All @@ -1325,11 +1372,11 @@ struct controller_impl {
// ritr base is a forward itr to the last block successfully applied
auto applied_itr = ritr.base();
for( auto itr = applied_itr; itr != branches.first.end(); ++itr ) {
fork_db.mark_in_current_chain( *itr , false );
fork_db.mark_in_current_chain( *itr, false );
pop_block();
}
EOS_ASSERT( self.head_block_id() == branches.second.back()->header.previous, fork_database_exception,
"loss of sync between fork_db and chainbase during fork switch reversal" ); // _should_ never fail
"loss of sync between fork_db and chainbase during fork switch reversal" ); // _should_ never fail

// re-apply good blocks
for( auto ritr = branches.second.rbegin(); ritr != branches.second.rend(); ++ritr ) {
Expand Down Expand Up @@ -1646,10 +1693,14 @@ void controller::abort_block() {
my->abort_block();
}

void controller::push_block( const signed_block_ptr& b, block_status s ) {
std::future<block_state_ptr> controller::create_block_state_future( const signed_block_ptr& b ) {
return my->create_block_state_future( b );
}

void controller::push_block( std::future<block_state_ptr>& block_state_future ) {
validate_db_available_size();
validate_reversible_available_size();
my->push_block( b, s );
my->push_block( block_state_future );
}

transaction_trace_ptr controller::push_transaction( const transaction_metadata_ptr& trx, fc::time_point deadline, uint32_t billed_cpu_time_us ) {
Expand Down
7 changes: 5 additions & 2 deletions libraries/chain/fork_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ namespace eosio { namespace chain {
}

block_state_ptr fork_database::add( block_state_ptr n ) {
EOS_ASSERT( n, fork_database_exception, "attempt to add null block state" );
EOS_ASSERT( my->head, fork_db_block_not_found, "no head block set" );

auto inserted = my->index.insert(n);
EOS_ASSERT( inserted.second, fork_database_exception, "duplicate block added?" );

Expand All @@ -139,7 +142,7 @@ namespace eosio { namespace chain {
return n;
}

block_state_ptr fork_database::add( signed_block_ptr b, bool trust ) {
block_state_ptr fork_database::add( signed_block_ptr b, bool skip_validate_signee ) {
EOS_ASSERT( b, fork_database_exception, "attempt to add null block" );
EOS_ASSERT( my->head, fork_db_block_not_found, "no head block set" );

Expand All @@ -150,7 +153,7 @@ namespace eosio { namespace chain {
auto prior = by_id_idx.find( b->previous );
EOS_ASSERT( prior != by_id_idx.end(), unlinkable_block_exception, "unlinkable block", ("id", string(b->id()))("previous", string(b->previous)) );

auto result = std::make_shared<block_state>( **prior, move(b), trust );
auto result = std::make_shared<block_state>( **prior, move(b), skip_validate_signee );
EOS_ASSERT( result, fork_database_exception , "fail to add new block state" );
return add(result);
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/include/eosio/chain/block_header_state.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ struct block_header_state {
public_key_type block_signing_key;
vector<uint8_t> confirm_count;
vector<header_confirmation> confirmations;
std::shared_future<public_key_type> block_signing_key_future;

block_header_state next( const signed_block_header& h, bool trust = false )const;
block_header_state generate_next( block_timestamp_type when )const;
Expand All @@ -53,6 +52,7 @@ struct block_header_state {
digest_type sig_digest()const;
void sign( const std::function<signature_type(const digest_type&)>& signer );
public_key_type signee()const;
void verify_signee(const public_key_type& signee)const;
};


Expand Down
4 changes: 2 additions & 2 deletions libraries/chain/include/eosio/chain/block_state.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
namespace eosio { namespace chain {

struct block_state : public block_header_state {
block_state( const block_header_state& cur ):block_header_state(cur){}
block_state( const block_header_state& prev, signed_block_ptr b, bool trust = false );
explicit block_state( const block_header_state& cur ):block_header_state(cur){}
block_state( const block_header_state& prev, signed_block_ptr b, bool skip_validate_signee );
block_state( const block_header_state& prev, block_timestamp_type when );
block_state() = default;

Expand Down
3 changes: 2 additions & 1 deletion libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ namespace eosio { namespace chain {
void commit_block();
void pop_block();

void push_block( const signed_block_ptr& b, block_status s = block_status::complete );
std::future<block_state_ptr> create_block_state_future( const signed_block_ptr& b );
void push_block( std::future<block_state_ptr>& block_state_future );

const chainbase::database& db()const;

Expand Down
4 changes: 2 additions & 2 deletions libraries/chain/include/eosio/chain/fork_database.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ namespace eosio { namespace chain {
*/
void set( block_state_ptr s );

/** this method will attempt to append the block to an exsting
/** this method will attempt to append the block to an existing
* block_state and will return a pointer to the new block state or
* throw on error.
*/
block_state_ptr add( signed_block_ptr b, bool trust = false );
block_state_ptr add( signed_block_ptr b, bool skip_validate_signee );
block_state_ptr add( block_state_ptr next_block );
void remove( const block_id_type& id );

Expand Down
11 changes: 6 additions & 5 deletions libraries/testing/include/eosio/testing/tester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@ namespace eosio { namespace testing {

signed_block_ptr produce_block( fc::microseconds skip_time = fc::milliseconds(config::block_interval_ms), uint32_t skip_flag = 0 /*skip_missed_block_penalty*/ )override {
auto sb = _produce_block(skip_time, false, skip_flag | 2);
validating_node->push_block( sb );
auto bs = validating_node->create_block_state_future( sb );
validating_node->push_block( bs );

return sb;
}
Expand All @@ -383,15 +384,15 @@ namespace eosio { namespace testing {
}

void validate_push_block(const signed_block_ptr& sb) {
validating_node->push_block( sb );
auto bs = validating_node->create_block_state_future( sb );
validating_node->push_block( bs );
}

signed_block_ptr produce_empty_block( fc::microseconds skip_time = fc::milliseconds(config::block_interval_ms), uint32_t skip_flag = 0 /*skip_missed_block_penalty*/ )override {
control->abort_block();
auto sb = _produce_block(skip_time, true, skip_flag | 2);
validating_node->push_block( sb );


auto bs = validating_node->create_block_state_future( sb );
validating_node->push_block( bs );

return sb;
}
Expand Down
6 changes: 4 additions & 2 deletions libraries/testing/tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ namespace eosio { namespace testing {
}

signed_block_ptr base_tester::push_block(signed_block_ptr b) {
auto bs = control->create_block_state_future(b);
control->abort_block();
control->push_block(b);
control->push_block(bs);

auto itr = last_produced_block.find(b->producer);
if (itr == last_produced_block.end() || block_header::num_from_id(b->id()) > block_header::num_from_id(itr->second)) {
Expand Down Expand Up @@ -795,8 +796,9 @@ namespace eosio { namespace testing {
for( int i = 1; i <= a.control->head_block_num(); ++i ) {
auto block = a.control->fetch_block_by_number(i);
if( block ) { //&& !b.control->is_known_block(block->id()) ) {
auto bs = b.control->create_block_state_future( block );
b.control->abort_block();
b.control->push_block(block); //, eosio::chain::validation_steps::created_block);
b.control->push_block(bs); //, eosio::chain::validation_steps::created_block);
}
}
};
Expand Down
12 changes: 8 additions & 4 deletions plugins/producer_plugin/producer_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,18 +285,22 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
};

void on_incoming_block(const signed_block_ptr& block) {
fc_dlog(_log, "received incoming block ${id}", ("id", block->id()));
auto id = block->id();

EOS_ASSERT( block->timestamp < (fc::time_point::now() + fc::seconds(7)), block_from_the_future, "received a block from the future, ignoring it" );
fc_dlog(_log, "received incoming block ${id}", ("id", id));

EOS_ASSERT( block->timestamp < (fc::time_point::now() + fc::seconds( 7 )), block_from_the_future,
"received a block from the future, ignoring it: ${id}", ("id", id) );

chain::controller& chain = app().get_plugin<chain_plugin>().chain();

/* de-dupe here... no point in aborting block if we already know the block */
auto id = block->id();
auto existing = chain.fetch_block_by_id( id );
if( existing ) { return; }

// start processing of block
auto bsf = chain.create_block_state_future( block );

// abort the pending block
chain.abort_block();

Expand All @@ -308,7 +312,7 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
// push the new block
bool except = false;
try {
chain.push_block(block);
chain.push_block( bsf );
} catch ( const guard_exception& e ) {
app().get_plugin<chain_plugin>().handle_guard_exception(e);
return;
Expand Down