Address several style/documentation/aesthetic issues

Cleanup re [[fallthrough]] in switch statements (Fix #220)
  - src/3pc/agent/impl.cpp
  - src/3pc/broker/impl.cpp
  - src/uhs/twophase/coordinator/format.cpp
src/3pc/agent/impl.cpp
  - do_commit(): remove unused code block in lambda callback passed to m_broker->commit()
src/3pc/agent/runners/interface.hpp
& src/3pc/agent/runners/lua/impl.hpp
& src/3pc/agent/runners/evm/impl.hpp
  - runner::interface::run(): add [[nodiscard]] and comments
src/3pc/agent/runners/evm/init_addresses.hpp
  - rename init_addresses → init_addresses_for_testing
src/3pc/agent/runners/evm/serialization.hpp
  - fix broken doc comments
src/3pc/directory/impl.cpp
& src/3pc/directory/impl.hpp:
  - key_location(): note that using modulo operation creates a (very) small bias from a uniform distribution
src/util/rpc/http/kqueue_event_handler.cpp
  - init(): fix if statement containing assignment
tests/unit/3pc/agent/runners/evm/evm_test.cpp
  - Direct reader to origins of bytecode

Signed-off-by: Alexander Jung <[email protected]>

temp

Signed-off-by: Alexander Jung <[email protected]>

Author: AlexRamRam

src/3pc/agent/impl.cpp

@@ -40,7 +40,6 @@ namespace cbdc::threepc::agent {
         switch(m_state) {
             // In these states we can start again from the beginning
             case state::init:
-                [[fallthrough]];
             case state::begin_sent:
             case state::begin_failed:
                 break;
@@ -69,7 +68,6 @@ namespace cbdc::threepc::agent {
 
             // Rollback first so we can start fresh
             case state::function_get_sent:
-                [[fallthrough]];
             case state::function_get_failed:
             case state::function_failed:
             case state::function_started:
@@ -87,7 +85,6 @@ namespace cbdc::threepc::agent {
 
             // End states, cannot re-run exec
             case state::function_get_error:
-                [[fallthrough]];
             case state::commit_error:
             case state::function_exception:
             case state::finish_complete:
@@ -375,16 +372,7 @@ namespace cbdc::threepc::agent {
             m_ticket_number.value(),
             payload,
             [this](broker::interface::commit_return_type commit_res) {
-                if(commit_res.has_value()) {
-                    if(std::holds_alternative<
-                           runtime_locking_shard::shard_error>(
-                           commit_res.value())) {
-                        auto err
-                            = std::get<runtime_locking_shard::shard_error>(
-                                commit_res.value());
-                    }
-                }
-                handle_commit(commit_res);
+                handle_commit(std::move(commit_res));
             });
         if(!maybe_success) {
             m_state = state::commit_failed;
@@ -491,34 +479,40 @@ namespace cbdc::threepc::agent {
             // No results should be reported in these states, fatal bugs
             case state::init:
                 m_log->fatal("Result reported in initial state");
+                // System terminated by fatal()
             case state::begin_sent:
                 m_log->fatal("Result reported in begin_sent state");
+                // System terminated by fatal()
             case state::function_get_sent:
                 m_log->fatal("Result reported in function_get_sent state");
+                // System terminated by fatal()
             case state::commit_sent:
                 m_log->fatal("Result reported in commit_sent state");
+                // System terminated by fatal()
             case state::finish_sent:
                 m_log->fatal("Result reported in finish_sent state");
+                // System terminated by fatal()
             case state::function_started:
                 m_log->fatal("Result reported in function_started state");
+                // System terminated by fatal()
             case state::rollback_sent:
                 m_log->fatal("Result reported in rollback_sent state");
+                // System terminated by fatal()
             case state::rollback_complete:
                 if(!std::holds_alternative<error_code>(m_result.value())
                    || std::get<error_code>(m_result.value())
                           != error_code::retry) {
                     m_log->fatal("Result reported in rollback_complete state "
                                  "when result is not retry");
                 }
-                [[fallthrough]];
+                break;
 
             // Failure due to transient problems, should retry
             case state::begin_failed:
                 // Couldn't get a ticket number, no need to rollback
                 break;
 
             case state::function_get_failed:
-                [[fallthrough]];
             case state::function_failed:
             case state::commit_failed:
                 do_rollback(false);
@@ -534,7 +528,6 @@ namespace cbdc::threepc::agent {
 
             // Failure due to permanent error, abort completely
             case state::function_get_error:
-                [[fallthrough]];
             case state::commit_error:
             case state::function_exception:
                 do_rollback(true);

src/3pc/agent/runners/evm/impl.hpp

@@ -68,8 +68,9 @@ namespace cbdc::threepc::agent::runner {
         auto operator=(evm_runner&&) -> evm_runner& = delete;
 
         /// Begin executing the transaction asynchronously.
-        /// \return true if execution was initiated successfully.
-        auto run() -> bool override;
+        /// \return true if execution was initiated successfully
+        /// returns false if an internal system error has occurred
+        [[nodiscard]] auto run() -> bool override;
 
         /// Initial lock type for the agent to request when retrieving the
         /// function key.

src/3pc/agent/runners/evm/init_addresses.hpp

@@ -10,8 +10,8 @@
 #include <vector>
 
 namespace cbdc::threepc::agent {
-    /// List of initial addresses to mint accounts for.
-    const std::vector<std::string> init_addresses
+    /// List of initial addresses to mint accounts for testing
+    const std::vector<std::string> init_addresses_for_testing
         = {std::string("01a151cc5ed14d110cc0e6b64360913de9f453f1"),
            std::string("13333383a6c55d9b699c4d57e3b0c85759f0efca"),
            std::string("9269d13036f74f5e7c1560b2daad6181fb71e682"),

src/3pc/agent/runners/evm/serialization.hpp

@@ -39,8 +39,8 @@ namespace cbdc::threepc::agent::runner {
     /// \param buf buffer containing the transaction to decode
     /// \param logger logger to output any parsing errors to
     /// \param chain_id the expected chain ID for the transaction. If the
-    // transaction contains a different chain ID this method will return
-    // std::nullopt
+    /// transaction contains a different chain ID this method will return
+    /// std::nullopt
     /// \return the evm_tx that was decoded or std::nullopt if no valid
     ///         value could be decoded
     auto tx_decode(const cbdc::buffer& buf,
@@ -60,8 +60,8 @@ namespace cbdc::threepc::agent::runner {
     /// Converts a given Json::Value to an evm_tx
     /// \param json Json::Value containing the transaction to decode
     /// \param chain_id the expected chain ID for the transaction. If the
-    // transaction contains a different chain ID this method will return
-    // std::nullopt
+    /// transaction contains a different chain ID this method will return
+    /// std::nullopt
     /// \return the evm_tx that was decoded or std::nullopt if no valid
     ///         value could be decoded
     auto tx_from_json(const Json::Value& json,
@@ -72,8 +72,8 @@ namespace cbdc::threepc::agent::runner {
     /// Converts a given Json::Value to an evm_dryrun_tx
     /// \param json Json::Value containing the transaction to decode
     /// \param chain_id the expected chain ID for the transaction. If the
-    // transaction contains a different chain ID this method will return
-    // std::nullopt
+    /// transaction contains a different chain ID this method will return
+    /// std::nullopt
     /// \return the evm_dryrun_tx that was decoded or std::nullopt if no valid
     ///         value could be decoded
     auto dryrun_tx_from_json(const Json::Value& json,

src/3pc/agent/runners/evm/util.cpp

@@ -112,7 +112,8 @@ namespace cbdc::threepc::agent::runner {
 
         auto successes = std::vector<std::future<bool>>();
 
-        for(const auto& init_addr_hex : cbdc::threepc::agent::init_addresses) {
+        for(const auto& init_addr_hex :
+            cbdc::threepc::agent::init_addresses_for_testing) {
             log->info("Seeding address ", init_addr_hex);
             auto init_addr = cbdc::buffer::from_hex(init_addr_hex).value();
             auto seed_success = std::make_shared<std::promise<bool>>();

src/3pc/agent/runners/interface.hpp

@@ -113,8 +113,8 @@ namespace cbdc::threepc::agent::runner {
 
         /// Begins function execution. Retrieves the function bytecode using a
         /// read lock and executes it with the given parameter.
-        /// \return true.
-        virtual auto run() -> bool = 0;
+        /// \return true unless a internal system error has occurred
+        [[nodiscard]] virtual auto run() -> bool = 0;
 
         friend class lua_runner;
         friend class evm_runner;

src/3pc/agent/runners/lua/impl.hpp

@@ -34,8 +34,8 @@ namespace cbdc::threepc::agent::runner {
 
         /// Begins function execution. Retrieves the function bytecode using a
         /// read lock and executes it with the given parameter.
-        /// \return true.
-        auto run() -> bool override;
+        /// \return true unless a internal system error has occurred
+        [[nodiscard]] auto run() -> bool override;
 
         /// Lock type to acquire when requesting the function code.
         static constexpr auto initial_lock_type = broker::lock_type::read;

src/3pc/broker/impl.cpp

@@ -961,7 +961,6 @@ namespace cbdc::threepc::broker {
             for(auto& [sidx, t_state] : ticket->m_shard_states) {
                 switch(t_state.m_state) {
                     case shard_state_type::begun:
-                        [[fallthrough]];
                     case shard_state_type::prepared:
                     case shard_state_type::wounded:
                         break;

src/3pc/directory/impl.cpp

@@ -12,6 +12,8 @@ namespace cbdc::threepc::directory {
                             key_location_callback_type result_callback)
         -> bool {
         auto key_hash = m_siphash(key);
+        // NOTE: using modulo creates a small bias from a true
+        // uniform distribution
         auto shard = key_hash % m_n_shards;
         result_callback(shard);
         return true;

src/3pc/directory/impl.hpp

@@ -9,8 +9,7 @@
 #include "interface.hpp"
 
 namespace cbdc::threepc::directory {
-    /// Implementation of a directory. Uses siphash to uniformly map keys to
-    /// shard IDs based on a fixed number of shards. Thread-safe.
+    /// Implementation of a directory which map keys to shard IDs. Thread-safe.
     class impl : public interface {
       public:
         /// Constructor.

src/uhs/twophase/coordinator/format.cpp

@@ -46,7 +46,6 @@ namespace cbdc {
             // Discard, done and get don't have a payload
             case coordinator::state_machine::command::discard:
             case coordinator::state_machine::command::done:
-                [[fallthrough]];
             case coordinator::state_machine::command::get: {
                 break;
             }

src/util/rpc/http/kqueue_event_handler.cpp

@@ -16,10 +16,8 @@ namespace cbdc::rpc {
     }
 
     auto kqueue_event_handler::init() -> bool {
-        if((m_kq = kqueue()) == -1) {
-            return false;
-        }
-        return true;
+        m_kq = kqueue();
+        return m_kq != -1;
     }
 
     void kqueue_event_handler::set_timeout(long timeout_ms) {

tests/unit/3pc/agent/runners/evm/evm_test.cpp

@@ -312,6 +312,8 @@ TEST_F(evm_test, simple_send) {
 }
 
 TEST_F(evm_test, contract_deploy) {
+    // See tools/bench/3pc/evm/contracts for the source Solidity contract and
+    // other details on the generation of the following bytecode
     auto bytecode
         = cbdc::buffer::from_hex(
               "608060405234801561001057600080fd5b5061002d61002261003260201b602"