Change price queue implementation. Use a regular table (multi_index) …

…instead of std::deque
eosnetworkfoundation · Apr 20, 2024 · 3fd8004 · 3fd8004
1 parent 51f6cdf
commit 3fd8004
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 99 deletions.
diff --git a/include/evm_runtime/tables.hpp b/include/evm_runtime/tables.hpp
@@ -322,11 +322,6 @@ struct consensus_parameter_type {
     }
 };
 
-struct price_time {
-    uint64_t   price;
-    time_point time;
-};
-
 struct [[eosio::table]] [[eosio::contract("evm_contract")]] config
 {
     unsigned_int version; // placeholder for future variant index
@@ -338,9 +333,20 @@ struct [[eosio::table]] [[eosio::contract("evm_contract")]] config
     uint32_t status = 0; // <- bit mask values from status_flags
     binary_extension<evm_version_type> evm_version;
     binary_extension<consensus_parameter_type> consensus_parameter;
-    binary_extension<std::deque<price_time>> base_price_queue;
 
-    EOSLIB_SERIALIZE(config, (version)(chainid)(genesis_time)(ingress_bridge_fee)(gas_price)(miner_cut)(status)(evm_version)(consensus_parameter)(base_price_queue));
+    EOSLIB_SERIALIZE(config, (version)(chainid)(genesis_time)(ingress_bridge_fee)(gas_price)(miner_cut)(status)(evm_version)(consensus_parameter));
+};
+
+struct [[eosio::table]] [[eosio::contract("evm_contract")]] price_queue
+{
+    uint64_t time;
+    uint64_t price;
+
+    uint64_t primary_key()const { return time; }
+
+    EOSLIB_SERIALIZE(price_queue, (time)(price));
 };
 
+typedef eosio::multi_index<"pricequeue"_n, price_queue> price_queue_table;
+
 } //namespace evm_runtime
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -92,5 +92,5 @@ target_compile_options(evm_runtime PUBLIC --no-missing-ricardian-clause)
 if (WITH_LARGE_STACK)
     target_link_options(evm_runtime PUBLIC --stack-size=50000000)
 else()
-    target_link_options(evm_runtime PUBLIC --stack-size=35168)
+    target_link_options(evm_runtime PUBLIC --stack-size=35088)
 endif()
diff --git a/src/config_wrapper.cpp b/src/config_wrapper.cpp
@@ -13,9 +13,6 @@ config_wrapper::config_wrapper(eosio::name self) : _self(self), _config(self, se
         _cached_config.consensus_parameter = consensus_parameter_type();
         // Don't set dirty because action can be read-only.
     }
-    if (!_cached_config.base_price_queue.has_value()) {
-        _cached_config.base_price_queue = std::deque<price_time>();
-    }
 }
 
 config_wrapper::~config_wrapper() {
@@ -81,32 +78,21 @@ void config_wrapper::set_gas_price(uint64_t gas_price) {
 }
 
 void config_wrapper::enqueue_gas_price(uint64_t gas_price) {
-    auto pt = price_time{gas_price, eosio::current_time_point() + eosio::seconds(grace_period_seconds)};
-    // should not happen
-    check(_cached_config.base_price_queue.has_value(), "no queue");
-    auto& q = _cached_config.base_price_queue.value();
-    if(!q.size()) {
-        q.push_back(pt);
-    } else {
-        if(q.back().price == gas_price) {
-            return;
-        }
-        if(q.back().time == pt.time) {
-            q.back() = pt;
-        } else {
-            q.push_back(pt);
-        }
-    }
-    set_dirty();
+    price_queue_table queue(_self, _self.value);
+    auto time = eosio::current_time_point() + eosio::seconds(grace_period_seconds);
+    queue.emplace(_self, [&](auto& el) {
+        el.time = time.elapsed.count();
+        el.price = gas_price;
+    });
 }
 
 void config_wrapper::process_price_queue() {
-    if(!_cached_config.base_price_queue.has_value()) return;
-    auto now = eosio::current_time_point();
-    auto& q = _cached_config.base_price_queue.value();
-    while( q.size() && now >= q.front().time ) {
-        set_gas_price(q.front().price);
-        q.pop_front();
+    auto now = eosio::current_time_point().elapsed.count();
+    price_queue_table queue(_self, _self.value);
+    auto it = queue.begin();
+    while( it != queue.end() && now >= it->time ) {
+        set_gas_price(it->price);
+        it = queue.erase(it);
     }
 }
 

diff --git a/tests/basic_evm_tester.cpp b/tests/basic_evm_tester.cpp
@@ -93,11 +93,6 @@ namespace fc { namespace raw {
          fc::raw::unpack(ds, consensus_parameter);
          tmp.consensus_parameter.emplace(consensus_parameter);
       }
-      if(ds.remaining()) {
-         std::deque<evm_test::price_time> base_price_queue;
-         fc::raw::unpack(ds, base_price_queue);
-         tmp.base_price_queue.emplace(base_price_queue);
-      }
     } FC_RETHROW_EXCEPTIONS(warn, "error unpacking partial_account_table_row") }
 }}
 
@@ -752,6 +747,17 @@ bool basic_evm_tester::scan_account_code(std::function<bool(account_code)> visit
    return true;
 }
 
+bool basic_evm_tester::scan_price_queue(std::function<bool(price_queue)> visitor) const
+{
+   static constexpr eosio::chain::name price_queue_table_name = "pricequeue"_n;
+
+   scan_table<price_queue>(
+      price_queue_table_name, evm_account_name, [&visitor](price_queue&& row) { return visitor(row); }
+   );
+
+   return true;
+}
+
 asset basic_evm_tester::get_eos_balance( const account_name& act ) {
    vector<char> data = get_row_by_account( "eosio.token"_n, act, "accounts"_n, name(native_symbol.to_symbol_code().value) );
    return data.empty() ? asset(0, native_symbol) : fc::raw::unpack<asset>(data);

diff --git a/tests/basic_evm_tester.hpp b/tests/basic_evm_tester.hpp
@@ -40,6 +40,12 @@ inline std::ostream& operator<<(std::ostream& ds, const intx::uint256& num)
 
 } // namespace intx
 
+inline std::ostream& operator<<(std::ostream& ds, const fc::time_point& tp)
+{
+   ds << tp.to_iso_string();
+   return ds;
+}
+
 namespace fc {
 
 void to_variant(const intx::uint256& o, fc::variant& v);
@@ -85,17 +91,6 @@ struct consensus_parameter_type {
    consensus_parameter_data_type current;
    std::optional<pending_consensus_parameter_data_type> pending;
 };
-struct price_time {
-   uint64_t price;
-   fc::time_point time;
-   bool operator==(const price_time& o) const { return price == o.price && time == o.time; }
-};
-
-inline std::ostream& operator<<(std::ostream& ds, const price_time& pt)
-{
-   ds << "{" << pt.price << "," << pt.time.to_iso_string() << "}";
-   return ds;
-}
 struct config_table_row
 {
    unsigned_int version;
@@ -107,7 +102,6 @@ struct config_table_row
    uint32_t status;
    std::optional<evm_version_type> evm_version;
    std::optional<consensus_parameter_type> consensus_parameter;
-   std::optional<std::deque<price_time>> base_price_queue;
 };
 
 struct config2_table_row
@@ -207,9 +201,14 @@ struct account_code {
 
 using bridge_message = std::variant<bridge_message_v0>;
 
-} // namespace evm_test
+struct price_queue {
+   uint64_t time;
+   uint64_t price;
+};
 
+} // namespace evm_test
 
+FC_REFLECT(evm_test::price_queue, (time)(price))
 FC_REFLECT(evm_test::evm_version_type, (pending_version)(cached_version))
 FC_REFLECT(evm_test::evm_version_type::pending, (version)(time))
 FC_REFLECT(evm_test::config2_table_row,(next_account_id))
@@ -228,7 +227,6 @@ FC_REFLECT(evm_test::gcstore, (id)(storage_id));
 FC_REFLECT(evm_test::account_code, (id)(ref_count)(code)(code_hash));
 FC_REFLECT(evm_test::evmtx_v0, (eos_evm_version)(rlptx)(base_fee_per_gas));
 
-FC_REFLECT(evm_test::price_time, (price)(time));
 FC_REFLECT(evm_test::consensus_parameter_type, (current)(pending));
 FC_REFLECT(evm_test::pending_consensus_parameter_data_type, (data)(pending_time));
 FC_REFLECT(evm_test::consensus_parameter_data_v0, (gas_parameter));
@@ -506,6 +504,7 @@ class basic_evm_tester : public evm_validating_tester
    bool scan_gcstore(std::function<bool(gcstore)> visitor) const;
    bool scan_account_code(std::function<bool(account_code)> visitor) const;
    void scan_balances(std::function<bool(evm_test::vault_balance_row)> visitor) const;
+   bool scan_price_queue(std::function<bool(evm_test::price_queue)> visitor) const;
 
    intx::uint128 tx_data_cost(const silkworm::Transaction& txn) const;
 };

diff --git a/tests/gas_fee_tests.cpp b/tests/gas_fee_tests.cpp
@@ -301,7 +301,7 @@ try {
 FC_LOG_AND_RETHROW()
 
 
-BOOST_FIXTURE_TEST_CASE(set_gas_price_v1, gas_fee_evm_tester)
+BOOST_FIXTURE_TEST_CASE(set_gas_price_queue, gas_fee_evm_tester)
 try {
    init();
 
@@ -310,69 +310,63 @@ try {
 
    const auto ten_gwei = 10'000'000'000ull;
 
-   // Queue change of gas_price to 10Gwei
-   setfeeparams({.gas_price = ten_gwei});
-
-   auto conf1 = get_config();
-   BOOST_REQUIRE(conf1.base_price_queue.has_value());
-
-   auto& q = conf1.base_price_queue.value();
-
-   auto gp1 = price_time{
-      .price=ten_gwei,
-      .time=control->pending_block_time()+fc::seconds(price_queue_grace_period)
+   auto get_price_queue = [&]() -> std::vector<price_queue> {
+      std::vector<price_queue> queue;
+      scan_price_queue([&](price_queue&& row) -> bool {
+         queue.push_back(row);
+         return false;
+      });
+      return queue;
    };
 
-   BOOST_CHECK_EQUAL(q.size(), 1);
-   BOOST_CHECK_EQUAL(q.front(), gp1);
-
-   produce_blocks(2);
+   auto trigger_price_queue_processing = [&](){
+      transfer_token("alice"_n, evm_account_name, make_asset(1), evm_account_name.to_string());
+   };
 
-   // different time, same price => no effect
+   // Queue change of gas_price to 10Gwei
    setfeeparams({.gas_price = ten_gwei});
-   conf1 = get_config();;
-   q = conf1.base_price_queue.value();
+   auto t1 = control->pending_block_time()+fc::seconds(price_queue_grace_period);
+
+   auto q = get_price_queue();
    BOOST_CHECK_EQUAL(q.size(), 1);
-   BOOST_CHECK_EQUAL(q.front(), gp1);
+   BOOST_CHECK_EQUAL(q[0].time, t1.time_since_epoch().count());
+   BOOST_CHECK_EQUAL(q[0].price, ten_gwei);
 
-   produce_blocks(2);
+   produce_blocks(100);
 
-   // different time, different price => [ok]
-   setfeeparams({.gas_price = ten_gwei+1});
-   conf1 = get_config();;
-   q = conf1.base_price_queue.value();
-   auto gp2 = price_time{
-      .price=ten_gwei+1,
-      .time=control->pending_block_time()+fc::seconds(price_queue_grace_period)
-   };
+   // Queue change of gas_price to 20Gwei
+   setfeeparams({.gas_price = 2*ten_gwei});
+   auto t2 = control->pending_block_time()+fc::seconds(price_queue_grace_period);
 
+   q = get_price_queue();
    BOOST_CHECK_EQUAL(q.size(), 2);
-   BOOST_CHECK_EQUAL(q.front(), gp1);
-   BOOST_CHECK_EQUAL(q.back(), gp2);
+   BOOST_CHECK_EQUAL(q[0].time, t1.time_since_epoch().count());
+   BOOST_CHECK_EQUAL(q[0].price, ten_gwei);
+   BOOST_CHECK_EQUAL(q[1].time, t2.time_since_epoch().count());
+   BOOST_CHECK_EQUAL(q[1].price, 2*ten_gwei);
 
-   // Process price queue
-   conf1 = get_config();
-   BOOST_CHECK_EQUAL(conf1.gas_price, suggested_gas_price);
-
-   auto trigger_price_queue_processing = [&](){
-      transfer_token("alice"_n, evm_account_name, make_asset(1), evm_account_name.to_string());
-   };
-
-   while(control->pending_block_time() != gp1.time) {
+   while(control->pending_block_time() != t1) {
       produce_blocks(1);
    }
    trigger_price_queue_processing();
-   conf1 = get_config();
-   BOOST_CHECK_EQUAL(conf1.gas_price, gp1.price);
 
-   while(control->pending_block_time() != gp2.time) {
+   auto cfg = get_config();
+   BOOST_CHECK_EQUAL(cfg.gas_price, ten_gwei);
+
+   q = get_price_queue();
+   BOOST_CHECK_EQUAL(q.size(), 1);
+   BOOST_CHECK_EQUAL(q[0].time, t2.time_since_epoch().count());
+   BOOST_CHECK_EQUAL(q[0].price, 2*ten_gwei);
+
+   while(control->pending_block_time() != t2) {
       produce_blocks(1);
    }
    trigger_price_queue_processing();
-   conf1 = get_config();
-   BOOST_CHECK_EQUAL(conf1.gas_price, gp2.price);
 
-   q = conf1.base_price_queue.value();
+   cfg = get_config();
+   BOOST_CHECK_EQUAL(cfg.gas_price, 2*ten_gwei);
+
+   q = get_price_queue();
    BOOST_CHECK_EQUAL(q.size(), 0);
 }
 FC_LOG_AND_RETHROW()