Gh#4836 signals #4901
Gh#4836 signals #4901
Conversation
- emit accepted_transaction signal for all scheduled transactions. - emit applied_transaction signal after onerror processing of scheduled transactions. - emit applied_transaction signal for expired scheduled transactions.
libraries/chain/controller.cpp
Outdated
| @@ -623,6 +635,8 @@ struct controller_impl { | |||
| trace = error_trace; | |||
| if( !trace->except_ptr ) { | |||
| undo_session.squash(); | |||
| emit( self.accepted_transaction, trx ); | |||
| emit( self.applied_transaction, trace ); | |||
There was a problem hiding this comment.
apply_onerror would already emit an accepted_transaction signal for onerror transaction followed by an applied_transaction signal for the onerror transaction's trace (but without the inner deferred transaction trace).
These two lines would mean emitting the signals again in the case of soft_fail. Except that the accepted_transaction signal is emitted with the original transaction (not the on error) and the applied_transaction is emitted with the finalized, correct trace.
There was a problem hiding this comment.
Seems to me the correct thing here is to not emit in apply_onerror and instead handle all the exit conditions in push_scheduled_transaction.
libraries/chain/controller.cpp
Outdated
| @@ -1332,7 +1347,7 @@ 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 ); | |||
| return my->push_transaction(trx, deadline, billed_cpu_time_us, billed_cpu_time_us > 0 ); | |||
There was a problem hiding this comment.
Assert that trx->implicit is false so that it does not become possible for outside callers to push a transaction that is treated as an implicit transaction by controller.
unittests/api_tests.cpp
Outdated
| @@ -1084,7 +1084,7 @@ BOOST_FIXTURE_TEST_CASE(deferred_transaction_tests, TESTER) { try { | |||
| auto c = control->applied_transaction.connect([&]( const transaction_trace_ptr& t) { if (t && t->scheduled) { trace = t; } } ); | |||
| CALL_TEST_FUNCTION(*this, "test_transaction", "send_deferred_transaction", {}); | |||
| CALL_TEST_FUNCTION(*this, "test_transaction", "cancel_deferred_transaction_success", {}); | |||
| produce_block( fc::seconds(2) ); | |||
| produce_block( fc::seconds(3) ); | |||
There was a problem hiding this comment.
Why was this change necessary?
There was a problem hiding this comment.
It was not necessary, I made it while debugging another issue.
| trx->accepted = true; | ||
| emit( self.accepted_transaction, trx); | ||
| } | ||
|
|
||
| emit(self.applied_transaction, trace); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Resolves #4836
Added new emit of signals:
Added new implicit and scheduled flags to transaction_metadata so bnet and other plugins can explicitly distinguish transaction types. Updated the bnet_plugin to use the new flags.