From 9094766556c0003bc6e59b15dab5b6c2b82bf088 Mon Sep 17 00:00:00 2001 From: Todd Fleming Date: Mon, 23 Jul 2018 17:11:16 -0400 Subject: [PATCH 1/4] extended_symbol compare --- contracts/eosiolib/symbol.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/eosiolib/symbol.hpp b/contracts/eosiolib/symbol.hpp index 597679ebaf2..8e63a0f18e5 100644 --- a/contracts/eosiolib/symbol.hpp +++ b/contracts/eosiolib/symbol.hpp @@ -215,6 +215,10 @@ namespace eosio { return std::tie( a.value, a.contract ) != std::tie( b.value, b.contract ); } + friend bool operator < ( const extended_symbol& a, const extended_symbol& b ) { + return std::tie( a.value, a.contract ) < std::tie( b.value, b.contract ); + } + EOSLIB_SERIALIZE( extended_symbol, (value)(contract) ) }; From f978fd91831acc7944c667cddbe27fd1d8bb8fae Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Tue, 24 Jul 2018 16:55:25 -0400 Subject: [PATCH 2/4] Resolve stalling producer schedule upgrade due to speculation The producer_plugin was calculating the "next time to produce" for a producer taking into account the effects of the current speculative block on the producer schedule. As that block is speculative, it may not happen. In the case where it did not happen this calculated value would be wrong potentially causing a producer to sleep through its rightful time to produce. If all of the producers that could provide the last confirming block for a pending->active upgrade were in this state the upgrade stalled out. In normal operation, you can predict wake up assuming the block is _not_ going to arrive because when the block arrives we recalculate. --- plugins/producer_plugin/producer_plugin.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 51d6d5d1b2f..d4be760a06c 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -769,9 +769,11 @@ producer_plugin::greylist_params producer_plugin::get_greylist() const { optional producer_plugin_impl::calculate_next_block_time(const account_name& producer_name) const { chain::controller& chain = app().get_plugin().chain(); + const auto& hbs = chain.head_block_state(); + const auto& active_schedule = hbs->active_schedule.producers; + const auto& pbs = chain.pending_block_state(); - const auto& active_schedule = pbs->active_schedule.producers; - const auto& hbt = pbs->header.timestamp; + const auto& pbt = pbs->header.timestamp; // determine if this producer is in the active schedule and if so, where auto itr = std::find_if(active_schedule.begin(), active_schedule.end(), [&](const auto& asp){ return asp.producer_name == producer_name; }); @@ -798,7 +800,7 @@ optional producer_plugin_impl::calculate_next_block_time(const a } // this producers next opportuity to produce is the next time its slot arrives after or at the calculated minimum - uint32_t minimum_slot = hbt.slot + minimum_offset; + uint32_t minimum_slot = pbt.slot + minimum_offset; size_t minimum_slot_producer_index = (minimum_slot % (active_schedule.size() * config::producer_repetitions)) / config::producer_repetitions; if ( producer_index == minimum_slot_producer_index ) { // this is the producer for the minimum slot, go with that From 78aa3d1bde73f19db8275297fd9053bea0d0c7c6 Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Tue, 24 Jul 2018 17:45:50 -0400 Subject: [PATCH 3/4] Consolidated Security Fixes for 1.1.1 - Fix Bug when deferred transaction cancels itself - Limit range of tracked block_status notices - Guard against a malformed notice message accessing the back of an empty vector. Co-authored-by: Bart Wyatt Co-authored-by: Kevin Heifner --- libraries/chain/controller.cpp | 56 ++++++++++--------- .../chain/generated_transaction_object.hpp | 28 ++++++++++ plugins/bnet_plugin/bnet_plugin.cpp | 14 +++-- plugins/net_plugin/net_plugin.cpp | 36 +++--------- 4 files changed, 74 insertions(+), 60 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index bb14017f138..408f694f71f 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -464,14 +464,14 @@ struct controller_impl { return fc::make_scoped_exit( std::move(callback) ); } - transaction_trace_ptr apply_onerror( const generated_transaction_object& gto, + transaction_trace_ptr apply_onerror( const generated_transaction& gtrx, fc::time_point deadline, fc::time_point start, uint32_t billed_cpu_time_us) { signed_transaction etrx; // Deliver onerror action containing the failed deferred transaction directly back to the sender. - etrx.actions.emplace_back( vector{{gto.sender, config::active_name}}, - onerror( gto.sender_id, gto.packed_trx.data(), gto.packed_trx.size() ) ); + etrx.actions.emplace_back( vector{{gtrx.sender, config::active_name}}, + onerror( gtrx.sender_id, gtrx.packed_trx.data(), gtrx.packed_trx.size() ) ); etrx.expiration = self.pending_block_time() + fc::microseconds(999'999); // Round up to avoid appearing expired etrx.set_reference_block( self.head_block_id() ); @@ -481,13 +481,13 @@ struct controller_impl { transaction_trace_ptr trace = trx_context.trace; try { trx_context.init_for_implicit_trx(); - trx_context.published = gto.published; + trx_context.published = gtrx.published; trx_context.trace->action_traces.emplace_back(); - trx_context.dispatch_action( trx_context.trace->action_traces.back(), etrx.actions.back(), gto.sender ); + trx_context.dispatch_action( trx_context.trace->action_traces.back(), etrx.actions.back(), gtrx.sender ); trx_context.finalize(); // Automatically rounds up network and CPU usage in trace and bills payers if successful auto restore = make_block_restore_point(); - trace->receipt = push_receipt( gto.trx_id, transaction_receipt::soft_fail, + trace->receipt = push_receipt( gtrx.trx_id, transaction_receipt::soft_fail, trx_context.billed_cpu_time_us, trace->net_usage ); fc::move_append( pending->_actions, move(trx_context.executed) ); @@ -539,19 +539,27 @@ struct controller_impl { transaction_trace_ptr push_scheduled_transaction( const generated_transaction_object& gto, fc::time_point deadline, uint32_t billed_cpu_time_us ) { try { auto undo_session = db.start_undo_session(true); - fc::datastream ds( gto.packed_trx.data(), gto.packed_trx.size() ); + auto gtrx = generated_transaction(gto); - auto remove_retained_state = fc::make_scoped_exit([&, this](){ - remove_scheduled_transaction(gto); - }); + // remove the generated transaction object after making a copy + // this will ensure that anything which affects the GTO multi-index-container will not invalidate + // data we need to successfully retire this transaction. + // + // IF the transaction FAILs in a subjective way, `undo_session` should expire without being squashed + // resulting in the GTO being restored and available for a future block to retire. + remove_scheduled_transaction(gto); + + fc::datastream ds( gtrx.packed_trx.data(), gtrx.packed_trx.size() ); + + EOS_ASSERT( gtrx.delay_until <= self.pending_block_time(), transaction_exception, "this transaction isn't ready", + ("gtrx.delay_until",gtrx.delay_until)("pbt",self.pending_block_time()) ); - EOS_ASSERT( gto.delay_until <= self.pending_block_time(), transaction_exception, "this transaction isn't ready", - ("gto.delay_until",gto.delay_until)("pbt",self.pending_block_time()) ); - if( gto.expiration < self.pending_block_time() ) { + + if( gtrx.expiration < self.pending_block_time() ) { auto trace = std::make_shared(); - trace->id = gto.trx_id; + trace->id = gtrx.trx_id; trace->scheduled = false; - trace->receipt = push_receipt( gto.trx_id, transaction_receipt::expired, billed_cpu_time_us, 0 ); // expire the transaction + trace->receipt = push_receipt( gtrx.trx_id, transaction_receipt::expired, billed_cpu_time_us, 0 ); // expire the transaction undo_session.squash(); return trace; } @@ -564,20 +572,20 @@ struct controller_impl { }); in_trx_requiring_checks = true; - transaction_context trx_context( self, dtrx, gto.trx_id ); + transaction_context trx_context( self, dtrx, gtrx.trx_id ); trx_context.deadline = deadline; trx_context.billed_cpu_time_us = billed_cpu_time_us; transaction_trace_ptr trace = trx_context.trace; flat_set bill_to_accounts; try { - trx_context.init_for_deferred_trx( gto.published ); + trx_context.init_for_deferred_trx( gtrx.published ); bill_to_accounts = trx_context.bill_to_accounts; trx_context.exec(); trx_context.finalize(); // Automatically rounds up network and CPU usage in trace and bills payers if successful auto restore = make_block_restore_point(); - trace->receipt = push_receipt( gto.trx_id, + trace->receipt = push_receipt( gtrx.trx_id, transaction_receipt::executed, trx_context.billed_cpu_time_us, trace->net_usage ); @@ -599,10 +607,10 @@ struct controller_impl { // Only soft or hard failure logic below: - if( gto.sender != account_name() && !failure_is_subjective(*trace->except)) { + if( gtrx.sender != account_name() && !failure_is_subjective(*trace->except)) { // Attempt error handling for the generated transaction. dlog("${detail}", ("detail", trace->except->to_detail_string())); - auto error_trace = apply_onerror( gto, deadline, trx_context.start, trx_context.billed_cpu_time_us ); + auto error_trace = apply_onerror( gtrx, deadline, trx_context.start, trx_context.billed_cpu_time_us ); error_trace->failed_dtrx_trace = trace; trace = error_trace; if( !trace->except_ptr ) { @@ -618,12 +626,8 @@ struct controller_impl { resource_limits.add_transaction_usage( bill_to_accounts, trx_context.billed_cpu_time_us, 0, block_timestamp_type(self.pending_block_time()).slot ); // Should never fail - if (failure_is_subjective(*trace->except)) { - // this is a subjective failure, don't remove the retained state so it can be - // retried at a later time and don't include any artifact of the transaction in the pending block - remove_retained_state.cancel(); - } else { - trace->receipt = push_receipt(gto.trx_id, transaction_receipt::hard_fail, trx_context.billed_cpu_time_us, 0); + if (!failure_is_subjective(*trace->except)) { + trace->receipt = push_receipt(gtrx.trx_id, transaction_receipt::hard_fail, trx_context.billed_cpu_time_us, 0); emit( self.applied_transaction, trace ); undo_session.squash(); } diff --git a/libraries/chain/include/eosio/chain/generated_transaction_object.hpp b/libraries/chain/include/eosio/chain/generated_transaction_object.hpp index da808e6d122..64c16de4dc6 100644 --- a/libraries/chain/include/eosio/chain/generated_transaction_object.hpp +++ b/libraries/chain/include/eosio/chain/generated_transaction_object.hpp @@ -77,6 +77,34 @@ namespace eosio { namespace chain { > >; + class generated_transaction + { + public: + generated_transaction(const generated_transaction_object& gto) + :trx_id(gto.trx_id) + ,sender(gto.sender) + ,sender_id(gto.sender_id) + ,payer(gto.payer) + ,delay_until(gto.delay_until) + ,expiration(gto.expiration) + ,published(gto.published) + ,packed_trx(gto.packed_trx.begin(), gto.packed_trx.end()) + {} + + generated_transaction(const generated_transaction& gt) = default; + generated_transaction(generated_transaction&& gt) = default; + + transaction_id_type trx_id; + account_name sender; + uint128_t sender_id; + account_name payer; + time_point delay_until; /// this generated transaction will not be applied until the specified time + time_point expiration; /// this generated transaction will not be applied after this time + time_point published; + vector packed_trx; + + }; + namespace config { template<> struct billable_size { diff --git a/plugins/bnet_plugin/bnet_plugin.cpp b/plugins/bnet_plugin/bnet_plugin.cpp index edb1c110494..17d12c421d2 100644 --- a/plugins/bnet_plugin/bnet_plugin.cpp +++ b/plugins/bnet_plugin/bnet_plugin.cpp @@ -253,12 +253,9 @@ namespace eosio { > > transaction_status_index; - auto get_status( block_id_type id ) { - return _block_status.find( id ); - } - block_status_index _block_status; transaction_status_index _transaction_status; + const uint32_t _max_block_status_range = 2048; // limit tracked block_status known_by_peer public_key_type _local_peer_id; uint32_t _local_lib = 0; @@ -650,13 +647,20 @@ namespace eosio { void mark_block_known_by_peer( block_id_type id) { auto itr = _block_status.find(id); if( itr == _block_status.end() ) { - _block_status.insert( block_status(id, true, false) ); + // optimization to avoid sending blocks to nodes that already know about them + // to avoid unbounded memory growth limit number tracked + auto min_block_num = std::min( _local_lib, _last_sent_block_num ); + auto max_block_num = min_block_num + _max_block_status_range; + auto block_num = block_header::num_from_id(id); + if(block_num > min_block_num && block_num < max_block_num) + _block_status.insert( block_status(id, true, false) ); } else { _block_status.modify( itr, [&]( auto& item ) { item.known_by_peer = true; }); } } + void mark_block_recv_from_peer( block_id_type id ) { auto itr = _block_status.find(id); if( itr == _block_status.end() ) { diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 1729525db4e..a77f4084681 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -84,22 +84,6 @@ namespace eosio { uint16_t requests = 0; /// the number of "in flight" requests for this txn }; - struct update_entry { - const packed_transaction &txn; - update_entry(const packed_transaction &msg) : txn(msg) {} - - void operator() (node_transaction_state& nts) { - nts.packed_txn = txn; - net_message msg(txn); - uint32_t packsiz = fc::raw::pack_size(msg); - uint32_t bufsiz = packsiz + sizeof(packsiz); - nts.serialized_txn.resize(bufsiz); - fc::datastream ds( nts.serialized_txn.data(), bufsiz ); - ds.write( reinterpret_cast(&packsiz), sizeof(packsiz) ); - fc::raw::pack( ds, msg ); - } - }; - struct update_in_flight { int32_t incr; update_in_flight (int32_t delta) : incr (delta) {} @@ -582,7 +566,6 @@ namespace eosio { void blk_send(const vector &txn_lis); void stop_send(); - void enqueue( transaction_id_type id ); void enqueue( const net_message &msg, bool trigger_send = true ); void cancel_sync(go_away_reason); void flush_queues(); @@ -666,13 +649,12 @@ namespace eosio { connection_ptr source; stages state; - deque _blocks; - chain_plugin * chain_plug; + chain_plugin* chain_plug; constexpr auto stage_str(stages s ); public: - sync_manager(uint32_t span); + explicit sync_manager(uint32_t span); void set_state(stages s); bool sync_required(); void send_handshakes(); @@ -690,13 +672,8 @@ namespace eosio { class dispatch_manager { public: - uint32_t just_send_it_max; + uint32_t just_send_it_max = 0; - struct block_request { - block_id_type id; - bool local_retry; - }; - vector req_blks; vector req_trx; struct block_origin { @@ -1688,6 +1665,7 @@ namespace eosio { if (c && c->last_req && c->last_req->req_blocks.mode != none && + !c->last_req->req_blocks.ids.empty() && c->last_req->req_blocks.ids.back() == id) { c->last_req.reset(); } @@ -1797,6 +1775,7 @@ namespace eosio { if (c && c->last_req && c->last_req->req_trx.mode != none && + !c->last_req->req_trx.ids.empty() && c->last_req->req_trx.ids.back() == id) { c->last_req.reset(); } @@ -1868,7 +1847,6 @@ namespace eosio { if (!b) { send_req = true; req.req_blocks.ids.push_back( blkid ); - req_blks.push_back( {blkid, generated} ); entry.requested_time = fc::time_point::now(); } c->add_peer_block(entry); @@ -1894,11 +1872,11 @@ namespace eosio { transaction_id_type tid; block_id_type bid; bool is_txn = false; - if( c->last_req->req_trx.mode == normal ) { + if( c->last_req->req_trx.mode == normal && !c->last_req->req_trx.ids.empty() ) { is_txn = true; tid = c->last_req->req_trx.ids.back(); } - else if( c->last_req->req_blocks.mode == normal ) { + else if( c->last_req->req_blocks.mode == normal && !c->last_req->req_blocks.ids.empty() ) { bid = c->last_req->req_blocks.ids.back(); } else { From da55177c0fc618e49ff4c8acd953617c4072ef7c Mon Sep 17 00:00:00 2001 From: Bart Wyatt Date: Tue, 24 Jul 2018 18:34:53 -0400 Subject: [PATCH 4/4] bump version to 1.1.1 --- CMakeLists.txt | 2 +- Docker/README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e81d7e0a635..a10fdfcb3be 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,7 +20,7 @@ set( CXX_STANDARD_REQUIRED ON) set(VERSION_MAJOR 1) set(VERSION_MINOR 1) -set(VERSION_PATCH 0) +set(VERSION_PATCH 1) set( CLI_CLIENT_EXECUTABLE_NAME cleos ) set( GUI_CLIENT_EXECUTABLE_NAME eosio ) diff --git a/Docker/README.md b/Docker/README.md index ad5d63a7b73..9eb3874adf5 100644 --- a/Docker/README.md +++ b/Docker/README.md @@ -20,10 +20,10 @@ cd eos/Docker docker build . -t eosio/eos ``` -The above will build off the most recent commit to the master branch by default. If you would like to target a specific branch/tag, you may use a build argument. For example, if you wished to generate a docker image based off of the v1.1.0 tag, you could do the following: +The above will build off the most recent commit to the master branch by default. If you would like to target a specific branch/tag, you may use a build argument. For example, if you wished to generate a docker image based off of the v1.1.1 tag, you could do the following: ```bash -docker build -t eosio/eos:v1.1.0 --build-arg branch=v1.1.0 . +docker build -t eosio/eos:v1.1.1 --build-arg branch=v1.1.1 . ``` By default, the symbol in eosio.system is set to SYS. You can override this using the symbol argument while building the docker image.