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

Gh#4836 signals #4901

Merged
merged 10 commits into from Aug 8, 2018
Merged

Gh#4836 signals #4901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
292bbda
Add implicit and scheduled flags
heifner Jul 27, 2018
04f1cab
Check new implicit and scheduled flags instead of signatures for dete…
heifner Jul 27, 2018
43fad60
- emit accepted_transaction signal for implicit transaction onerror.
heifner Jul 27, 2018
2b19c74
Fix deferred transaction tests
heifner Jul 27, 2018
b6bcb08
Use explicit emit instead of scoped_exit. Attempting to fix unittests.
heifner Jul 28, 2018
d45faeb
Merge branch 'develop' into gh#4836-signals
heifner Aug 3, 2018
357ff35
Merge branch 'develop' into gh#4836-signals
heifner Aug 6, 2018
0c6edfa
Revert unneeded test changes
heifner Aug 7, 2018
31d8b7e
Assert not implicit/scheduled on push_transaction
heifner Aug 7, 2018
9550f29
Consistently emit before squash/undo
heifner Aug 7, 2018
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
58 changes: 39 additions & 19 deletions libraries/chain/controller.cpp
View file
Open in desktop
Expand Up @@ -222,20 +222,20 @@ struct controller_impl {
std::cerr << std::setw(10) << next->block_num() << " of " << end->block_num() <<"\r";
}
}
std::cerr<< "\n";
ilog( "${n} blocks replayed", ("n", head->block_num) );

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 );
}

std::cerr<< "\n";
ilog( "${n} reversible blocks replayed", ("n",rev) );
auto end = fc::time_point::now();
ilog( "replayed ${n} blocks in ${duration} seconds, ${mspb} ms/block",
("n", head->block_num)("duration", (end-start).count()/1000000)
("mspb", ((end-start).count()/1000.0)/head->block_num) );
std::cerr<< "\n";
replaying = false;

} else if( !end ) {
Expand Down Expand Up @@ -495,8 +495,6 @@ struct controller_impl {
trx_context.billed_cpu_time_us, trace->net_usage );
fc::move_append( pending->_actions, move(trx_context.executed) );

emit( self.applied_transaction, trace );

trx_context.squash();
restore.cancel();
return trace;
Expand Down Expand Up @@ -559,19 +557,24 @@ struct controller_impl {
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()) );

signed_transaction dtrx;
fc::raw::unpack(ds,static_cast<transaction&>(dtrx) );
transaction_metadata_ptr trx = std::make_shared<transaction_metadata>( dtrx );
trx->accepted = true;
trx->scheduled = true;

transaction_trace_ptr trace;
if( gtrx.expiration < self.pending_block_time() ) {
auto trace = std::make_shared<transaction_trace>();
trace = std::make_shared<transaction_trace>();
trace->id = gtrx.trx_id;
trace->scheduled = false;
trace->scheduled = true;
trace->receipt = push_receipt( gtrx.trx_id, transaction_receipt::expired, billed_cpu_time_us, 0 ); // expire the transaction
emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );
undo_session.squash();
return trace;
}

signed_transaction dtrx;
fc::raw::unpack(ds,static_cast<transaction&>(dtrx) );

auto reset_in_trx_requiring_checks = fc::make_scoped_exit([old_value=in_trx_requiring_checks,this](){
in_trx_requiring_checks = old_value;
});
Expand All @@ -584,7 +587,7 @@ struct controller_impl {
trx_context.deadline = deadline;
trx_context.explicit_billed_cpu_time = explicit_billed_cpu_time;
trx_context.billed_cpu_time_us = billed_cpu_time_us;
transaction_trace_ptr trace = trx_context.trace;
trace = trx_context.trace;
try {
trx_context.init_for_deferred_trx( gtrx.published );
trx_context.exec();
Expand All @@ -599,11 +602,14 @@ struct controller_impl {

fc::move_append( pending->_actions, move(trx_context.executed) );

emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );

trx_context.squash();
undo_session.squash();

restore.cancel();

return trace;
} catch( const fc::exception& e ) {
cpu_time_to_bill_us = trx_context.update_billed_cpu_time( fc::time_point::now() );
Expand All @@ -622,6 +628,8 @@ struct controller_impl {
error_trace->failed_dtrx_trace = trace;
trace = error_trace;
if( !trace->except_ptr ) {
emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );
undo_session.squash();
return trace;
}
Expand All @@ -648,8 +656,16 @@ struct controller_impl {
block_timestamp_type(self.pending_block_time()).slot ); // Should never fail

trace->receipt = push_receipt(gtrx.trx_id, transaction_receipt::hard_fail, cpu_time_to_bill_us, 0);

emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );

undo_session.squash();
} else {
emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );

undo_session.undo();
}

return trace;
Expand Down Expand Up @@ -679,7 +695,6 @@ struct controller_impl {
*/
transaction_trace_ptr push_transaction( const transaction_metadata_ptr& trx,
fc::time_point deadline,
bool implicit,
uint32_t billed_cpu_time_us,
bool explicit_billed_cpu_time = false )
{
Expand All @@ -696,7 +711,7 @@ struct controller_impl {
trx_context.billed_cpu_time_us = billed_cpu_time_us;
trace = trx_context.trace;
try {
if( implicit ) {
if( trx->implicit ) {
trx_context.init_for_implicit_trx();
trx_context.can_subjectively_fail = false;
} else {
Expand All @@ -712,7 +727,7 @@ struct controller_impl {

trx_context.delay = fc::seconds(trx->trx.delay_sec);

if( !self.skip_auth_check() && !implicit ) {
if( !self.skip_auth_check() && !trx->implicit ) {
authorization.check_authorization(
trx->trx.actions,
trx->recover_keys( chain_id ),
Expand All @@ -729,7 +744,7 @@ struct controller_impl {

auto restore = make_block_restore_point();

if (!implicit) {
if (!trx->implicit) {
transaction_receipt::status_enum s = (trx_context.delay == fc::seconds(0))
? transaction_receipt::executed
: transaction_receipt::delayed;
Expand All @@ -747,8 +762,8 @@ struct controller_impl {

// call the accept signal but only once for this transaction
if (!trx->accepted) {
emit( self.accepted_transaction, trx);
trx->accepted = true;
emit( self.accepted_transaction, trx);
}

emit(self.applied_transaction, trace);
Copy link
Contributor

Choose a reason for hiding this comment

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

We emit the accepted_transaction and applied_transaction signals in push_transaction prior to squashing the undo sessions and canceling the scoped_exit to restore the block receipts, but we emit those signals after squashing the undo sessions in push_scheduled_transaction. It should be consistent.

The question is which one is more appropriate? You seem to have intentionally changed the point at which the existing signals were emitted to happen after. Was there a reason for this?

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 reason was my first stab at this I used a scoped exit to call the emits. However, that failed on gcc. I think I will go with the previous emit before squash/undo.

Expand All @@ -762,7 +777,7 @@ struct controller_impl {
trx_context.squash();
}

if (!implicit) {
if (!trx->implicit) {
unapplied_transactions.erase( trx->signed_id );
}
return trace;
Expand All @@ -775,6 +790,9 @@ struct controller_impl {
unapplied_transactions.erase( trx->signed_id );
}

emit( self.accepted_transaction, trx );
emit( self.applied_transaction, trace );

return trace;
} FC_CAPTURE_AND_RETHROW((trace))
} /// push_transaction
Expand Down Expand Up @@ -827,11 +845,12 @@ struct controller_impl {

try {
auto onbtrx = std::make_shared<transaction_metadata>( get_on_block_transaction() );
onbtrx->implicit = true;
auto reset_in_trx_requiring_checks = fc::make_scoped_exit([old_value=in_trx_requiring_checks,this](){
in_trx_requiring_checks = old_value;
});
in_trx_requiring_checks = true;
push_transaction( onbtrx, fc::time_point::maximum(), true, self.get_global_properties().configuration.min_transaction_cpu_usage, true );
push_transaction( onbtrx, fc::time_point::maximum(), self.get_global_properties().configuration.min_transaction_cpu_usage, true );
} catch( const boost::interprocess::bad_alloc& e ) {
elog( "on block transaction failed due to a bad allocation" );
throw;
Expand Down Expand Up @@ -870,7 +889,7 @@ struct controller_impl {
if( receipt.trx.contains<packed_transaction>() ) {
auto& pt = receipt.trx.get<packed_transaction>();
auto mtrx = std::make_shared<transaction_metadata>(pt);
trace = push_transaction( mtrx, fc::time_point::maximum(), false, receipt.cpu_usage_us, true );
trace = push_transaction( mtrx, fc::time_point::maximum(), receipt.cpu_usage_us, true );
} else if( receipt.trx.contains<transaction_id_type>() ) {
trace = push_scheduled_transaction( receipt.trx.get<transaction_id_type>(), fc::time_point::maximum(), receipt.cpu_usage_us, true );
} else {
Expand Down Expand Up @@ -1332,7 +1351,8 @@ void controller::push_confirmation( const header_confirmation& c ) {

transaction_trace_ptr controller::push_transaction( const transaction_metadata_ptr& trx, fc::time_point deadline, uint32_t billed_cpu_time_us ) {
validate_db_available_size();
return my->push_transaction(trx, deadline, false, billed_cpu_time_us, billed_cpu_time_us > 0 );
EOS_ASSERT( trx && !trx->implicit && !trx->scheduled, transaction_type_exception, "Implicit/Scheduled transaction not allowed" );
return my->push_transaction(trx, deadline, billed_cpu_time_us, billed_cpu_time_us > 0 );
}

transaction_trace_ptr controller::push_scheduled_transaction( const transaction_id_type& trxid, fc::time_point deadline, uint32_t billed_cpu_time_us )
Expand Down
6 changes: 4 additions & 2 deletions libraries/chain/include/eosio/chain/transaction_metadata.hpp
View file
Open in desktop
Expand Up @@ -21,15 +21,17 @@ class transaction_metadata {
packed_transaction packed_trx;
optional<pair<chain_id_type, flat_set<public_key_type>>> signing_keys;
bool accepted = false;
bool implicit = false;
bool scheduled = false;

transaction_metadata( const signed_transaction& t, packed_transaction::compression_type c = packed_transaction::none )
explicit transaction_metadata( const signed_transaction& t, packed_transaction::compression_type c = packed_transaction::none )
:trx(t),packed_trx(t, c) {
id = trx.id();
//raw_packed = fc::raw::pack( static_cast<const transaction&>(trx) );
signed_id = digest_type::hash(packed_trx);
}

transaction_metadata( const packed_transaction& ptrx )
explicit transaction_metadata( const packed_transaction& ptrx )
:trx( ptrx.get_signed_transaction() ), packed_trx(ptrx) {
id = trx.id();
//raw_packed = fc::raw::pack( static_cast<const transaction&>(trx) );
Expand Down
2 changes: 1 addition & 1 deletion plugins/bnet_plugin/bnet_plugin.cpp
View file
Open in desktop
Expand Up @@ -1190,7 +1190,7 @@ namespace eosio {
}

void on_accepted_transaction( transaction_metadata_ptr trx ) {
if( trx->trx.signatures.size() == 0 ) return;
if( trx->implicit || trx->scheduled ) return;
for_each_session( [trx]( auto ses ){ ses->on_accepted_transaction( trx ); } );
}

Expand Down
6 changes: 3 additions & 3 deletions unittests/api_tests.cpp
View file
Open in desktop
Expand Up @@ -1019,7 +1019,7 @@ BOOST_FIXTURE_TEST_CASE(deferred_transaction_tests, TESTER) { try {
produce_block( fc::seconds(2) );

//check that it gets executed afterwards
BOOST_CHECK(trace);
BOOST_REQUIRE(trace);

//confirm printed message
BOOST_TEST(!trace->action_traces.empty());
Expand All @@ -1045,7 +1045,7 @@ BOOST_FIXTURE_TEST_CASE(deferred_transaction_tests, TESTER) { try {
control->push_scheduled_transaction(trx, fc::time_point::maximum());
}
BOOST_CHECK_EQUAL(1, count);
BOOST_CHECK(trace);
BOOST_REQUIRE(trace);
BOOST_CHECK_EQUAL( 1, trace->action_traces.size() );
c.disconnect();
}
Expand Down Expand Up @@ -1098,7 +1098,7 @@ BOOST_FIXTURE_TEST_CASE(deferred_transaction_tests, TESTER) { try {

produce_blocks(10);

{
{
// Trigger a tx which in turn sends a deferred tx with payer != receiver
// Payer is alice in this case, this tx should fail since we don't have the authorization of alice
dtt_action dtt_act1;
Expand Down