Refactor max_serialization_time to be passed as argument instead #4463

andriantolie · 2018-07-03T07:12:25Z

max_serialization_time is moved from the static variable abi_serializer::max_serialization_time to being passed into methods that require it
Pass max_serialization_time as arguments instead of static variable of abi_serializer

heifner · 2018-07-05T19:18:38Z

libraries/chain/include/eosio/chain/controller.hpp

@@ -58,6 +58,7 @@ namespace eosio { namespace chain {

            genesis_state            genesis;
            wasm_interface::vm_type  wasm_runtime = chain::config::default_wasm_runtime;
+            fc::microseconds         abi_serializer_max_time_ms = chain::config::default_abi_serializer_max_time_ms;


We shouldn't store this on controller as it is only slightly better than storing on abi_serializer.

heifner

Working on moving setting out of controller.

Needs reviewed by someone else, I've made extensive changes.

andriantolie · 2018-07-06T03:35:53Z

plugins/mongo_db_plugin/mongo_db_plugin.cpp

@@ -252,7 +252,8 @@ namespace {
           abi = chain::eosio_contract_abi(abi);
        }
        abis.set_abi(abi);
-        auto v = abis.binary_to_variant(abis.get_action_type(msg.name), msg.data);
+        chain_plugin* chain_plug = app().find_plugin<chain_plugin>();
+        auto v = abis.binary_to_variant(abis.get_action_type(msg.name), msg.data, chain_plug.chain().get_abi_serializer_max_time_ms());


get_abi_serializer_max_time_ms of chain_controller is already removed

andriantolie · 2018-07-06T03:36:09Z

plugins/mongo_db_plugin/mongo_db_plugin.cpp

@@ -638,7 +639,8 @@ void mongo_db_plugin_impl::update_account(const chain::action& msg) {
      auto eosio_account = find_account(accounts, msg.account);
      auto abi = fc::json::from_string(bsoncxx::to_json(eosio_account.view()["abi"].get_document())).as<abi_def>();
      abis.set_abi(abi);
-      auto transfer = abis.binary_to_variant(abis.get_action_type(msg.name), msg.data);
+      chain_plugin* chain_plug = app().find_plugin<chain_plugin>();
+      auto transfer = abis.binary_to_variant(abis.get_action_type(msg.name), msg.data, chain_plug.chain().get_abi_serializer_max_time_ms());


get_abi_serializer_max_time_ms of chain_controller is already removed

andriantolie · 2018-07-06T03:36:20Z

plugins/account_history_plugin/account_history_plugin.cpp

@@ -443,7 +443,7 @@ fc::variant account_history_plugin_impl::transaction_to_variant(const packed_tra
   };

   fc::variant pretty_output;
-   abi_serializer::to_variant(ptrx, pretty_output, resolver);
+   abi_serializer::to_variant(ptrx, pretty_output, resolver, chain_plug->chain().get_abi_serializer_max_time_ms());


get_abi_serializer_max_time_ms of chain_controller is already removed. But I guess, we won't need to use account_history_plugin anymore since we have history_plugin now.

andriantolie · 2018-07-06T03:36:39Z

unittests/eosio_system_tester.hpp

@@ -437,7 +437,7 @@ class eosio_system_tester : public TESTER {
      action act;
      act.account = N(eosio.msig);
      act.name = name;
-      act.data = msig_abi_ser.variant_to_binary( action_type_name, data );
+      act.data = msig_abi_ser.variant_to_binary( action_type_name, data, control->get_abi_serializer_max_time_ms() );


get_abi_serializer_max_time_ms of chain_controller is already removed (this piece of code is actually just inside a comment)

andriantolie · 2018-07-06T03:36:47Z

unittests/multisig_tests.cpp

@@ -151,19 +151,19 @@ class eosio_msig_tester : public tester {
         action act;
         act.account = N(eosio.msig);
         act.name = name;
-         act.data = abi_ser.variant_to_binary( action_type_name, data );
+         act.data = abi_ser.variant_to_binary( action_type_name, data, control->get_abi_serializer_max_time_ms() );


get_abi_serializer_max_time_ms of chain_controller is already removed (this piece of code is actually just inside a comment)

jgiszczak · 2018-07-06T17:18:30Z

libraries/chain/abi_serializer.cpp

@@ -193,7 +193,7 @@ namespace eosio { namespace chain {
      return itr->second;
   }

-   void abi_serializer::validate()const {
+   void abi_serializer::validate(const fc::microseconds& max_serialization_time)const {


Called by set_abi() but calculates its own deadline. Should instead accept a deadline.

jgiszczak · 2018-07-06T17:19:17Z

libraries/chain/abi_serializer.cpp

@@ -348,16 +348,16 @@ namespace eosio { namespace chain {

            FC_ASSERT( st.base == type_name(), "support for base class as array not yet implemented" );
            /*if( st.base != type_name() ) {


Commented code.

jgiszczak · 2018-07-06T17:25:47Z

libraries/testing/tester.cpp

@@ -427,7 +426,7 @@ namespace eosio { namespace testing {
                                   const variant_object& data )const { try {
      const auto& acnt = control->get_account(code);
      auto abi = acnt.get_abi();
-      chain::abi_serializer abis(abi);
+      chain::abi_serializer abis(abi, abi_serializer_max_time);
    //  auto a = control->get_account(code).get_abi();


Commented code.

jgiszczak · 2018-07-06T17:27:45Z

plugins/chain_plugin/chain_plugin.cpp

@@ -1185,7 +1193,7 @@ read_only::get_account_results read_only::get_account( const get_account_params&
   //const abi_def abi = get_abi( db, N(eosio) );
   abi_def abi;
   if( abi_serializer::to_abi(code_account.abi, abi) ) {
-      abi_serializer abis( abi );
+      abi_serializer abis( abi, abi_serializer_max_time );
      //get_table_rows_ex<key_value_index, by_scope_primary>(p,abi);


Commented code.

jgiszczak · 2018-07-06T17:30:27Z

plugins/history_plugin/CMakeLists.txt

@@ -1,4 +1,5 @@
-file(GLOB HEADERS "include/eosio/history_plugin/*.hpp")
+file(GLOB HEADERS "include/eosio/history_plugin/*.hpp"
+                  "include/eosio/*.hpp")


Every eosio/ header always? Seems a bit of a blunt instrument...

andriantolie · 2018-07-09T02:15:39Z

PR Updated based on @jgiszczak comments (pass the deadline to validate() and remove commented code).

For @jgiszczak comment regarding eosio/* header inclusion in history_plugin CMakeList.txt, can @heifner please respond to it? Thanks!

…o_variant_with_abi.

…chines

… serializer deadline.

andriantolie requested a review from heifner July 3, 2018 07:12

andriantolie added this to the Version 1.1 milestone Jul 3, 2018

heifner self-assigned this Jul 5, 2018

heifner reviewed Jul 5, 2018

View reviewed changes

heifner previously requested changes Jul 5, 2018

View reviewed changes

heifner added the Needs Revisions label Jul 5, 2018

heifner force-pushed the 4404 branch from 9bb2977 to af567aa Compare July 5, 2018 21:18

heifner removed the Needs Revisions label Jul 5, 2018

andriantolie commented Jul 6, 2018

View reviewed changes

jgiszczak suggested changes Jul 6, 2018

View reviewed changes

Andrianto Lie and others added 15 commits July 9, 2018 12:25

Refactor max_serialization time to be passed as argument instead

ad2d0ed

Add missing header file

0629045

Add missing requires for chain_plugin

fe01e0f

Remove storage of abi_serializer_max_time. Instead pass in value to t…

e1306a9

…o_variant_with_abi.

Remove storage of abi_serializer_max_time.

ff52e99

Modify tests to have a 1s max abi serialization time for slow test ma…

3cbb676

…chines

Use large abi serializer max time for client

aa07f3b

Use command line --abi-serializer-max-time-ms of chain_plugin for abi…

2284d35

… serializer deadline.

Pass abi serialization max time to abi_serializer constructor.

ae430b4

Remove old reference to controller get_abi_serializer_max_time_ms

e82f778

Pass deadline to abi_serializer::validate

da55f4c

Remove commented code

dcc4761

Update location of history_plugin.hpp

854af98

Change location of history_plugin.hpp to match other plugins

8843f5f

Add abi serializer max time to calls to abi serializer

0181388

heifner force-pushed the 4404 branch from f6b7781 to 0181388 Compare July 9, 2018 17:47

Fix setting of abi serialization time

8d16a24

heifner merged commit 51c8cda into release/1.1 Jul 9, 2018

heifner deleted the 4404 branch July 9, 2018 20:40

andriantolie mentioned this pull request Jul 10, 2018

abi_serializer is still using the old definition #4564

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor max_serialization_time to be passed as argument instead #4463

Refactor max_serialization_time to be passed as argument instead #4463

andriantolie commented Jul 3, 2018 •

edited by heifner

Loading

heifner Jul 5, 2018

heifner left a comment

andriantolie Jul 6, 2018

andriantolie Jul 6, 2018

andriantolie Jul 6, 2018

andriantolie Jul 6, 2018

andriantolie Jul 6, 2018

jgiszczak Jul 6, 2018

jgiszczak Jul 6, 2018

jgiszczak Jul 6, 2018

jgiszczak Jul 6, 2018

jgiszczak Jul 6, 2018

andriantolie commented Jul 9, 2018

		@@ -348,16 +348,16 @@ namespace eosio { namespace chain {

		FC_ASSERT( st.base == type_name(), "support for base class as array not yet implemented" );
		/*if( st.base != type_name() ) {

Refactor max_serialization_time to be passed as argument instead #4463

Refactor max_serialization_time to be passed as argument instead #4463

Conversation

andriantolie commented Jul 3, 2018 • edited by heifner Loading

Choose a reason for hiding this comment

heifner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andriantolie commented Jul 9, 2018

andriantolie commented Jul 3, 2018 •

edited by heifner

Loading