Verbose VMExitCode (#81)

turuslan · web-flow · commit 5874a360ac72 · 2020-02-03T09:45:22.000+03:00
Signed-off-by: turuslan &lt;turuslan.devbox@gmail.com&gt;
diff --git a/core/vm/actor/account_actor.cpp b/core/vm/actor/account_actor.cpp
@@ -5,11 +5,13 @@
 
 #include "vm/actor/account_actor.hpp"
 
+#include "vm/exit_code/exit_code.hpp"
+
 namespace fc::vm::actor {
   outcome::result<Actor> AccountActor::create(
       const std::shared_ptr<StateTree> &state_tree, const Address &address) {
     if (!address.isKeyType()) {
-      return CREATE_WRONG_ADDRESS_TYPE;
+      return VMExitCode::ACCOUNT_ACTOR_CREATE_WRONG_ADDRESS_TYPE;
     }
     Actor actor{kAccountCodeCid, ActorSubstateCID{kEmptyObjectCid}, 0, 0};
     if (address.getProtocol() == Protocol::BLS) {
@@ -28,11 +30,11 @@ namespace fc::vm::actor {
     }
     auto maybe_actor = state_tree->get(address);
     if (!maybe_actor) {
-      return RESOLVE_NOT_FOUND;
+      return VMExitCode::ACCOUNT_ACTOR_RESOLVE_NOT_FOUND;
     }
     auto actor = maybe_actor.value();
     if (actor.code != kAccountCodeCid) {
-      return RESOLVE_NOT_ACCOUNT_ACTOR;
+      return VMExitCode::ACCOUNT_ACTOR_RESOLVE_NOT_ACCOUNT_ACTOR;
     }
     OUTCOME_TRY(account_actor_state,
                 state_tree->getStore()->getCbor<AccountActorState>(actor.head));
diff --git a/core/vm/actor/account_actor.hpp b/core/vm/actor/account_actor.hpp
@@ -7,7 +7,6 @@
 #define CPP_FILECOIN_CORE_VM_ACTOR_ACCOUNT_ACTOR_HPP
 
 #include "primitives/address/address_codec.hpp"
-#include "vm/exit_code/exit_code.hpp"
 #include "vm/state/state_tree.hpp"
 
 namespace fc::vm::actor {
@@ -36,10 +35,6 @@ namespace fc::vm::actor {
 
   /// Account actors represent actors without code
   struct AccountActor {
-    static constexpr VMExitCode CREATE_WRONG_ADDRESS_TYPE{1};
-    static constexpr VMExitCode RESOLVE_NOT_FOUND{1};
-    static constexpr VMExitCode RESOLVE_NOT_ACCOUNT_ACTOR{1};
-
     /// Create account actor from BLS or Secp256k1 address
     static outcome::result<Actor> create(
         const std::shared_ptr<StateTree> &state_tree, const Address &address);
diff --git a/core/vm/actor/actor_method.hpp b/core/vm/actor/actor_method.hpp
@@ -26,26 +26,22 @@ namespace fc::vm::actor {
   /// Reserved method number for constructor
   constexpr MethodNumber kConstructorMethodNumber{1};
 
-  constexpr VMExitCode DECODE_ACTOR_PARAMS_ERROR{1};
-
   /// Decode actor params, raises appropriate error
   template <typename T>
   outcome::result<T> decodeActorParams(gsl::span<const uint8_t> params_bytes) {
     auto maybe_params = codec::cbor::decode<T>(params_bytes);
     if (!maybe_params) {
-      return DECODE_ACTOR_PARAMS_ERROR;
+      return VMExitCode::DECODE_ACTOR_PARAMS_ERROR;
     }
     return maybe_params;
   }
 
-  constexpr VMExitCode ENCODE_ACTOR_PARAMS_ERROR{1};
-
   /// Encode actor params, raises appropriate error
   template <typename T>
   outcome::result<std::vector<uint8_t>> encodeActorParams(const T &params) {
     auto maybe_bytes = codec::cbor::encode(params);
     if (!maybe_bytes) {
-      return ENCODE_ACTOR_PARAMS_ERROR;
+      return VMExitCode::ENCODE_ACTOR_PARAMS_ERROR;
     }
     return maybe_bytes;
   }
diff --git a/core/vm/actor/cron_actor.cpp b/core/vm/actor/cron_actor.cpp
@@ -17,7 +17,7 @@ namespace fc::vm::actor::cron_actor {
                                               Runtime &runtime,
                                               const MethodParams &params) {
     if ((runtime.getMessage().get().from != kCronAddress)) {
-      return cron_actor::WRONG_CALL;
+      return VMExitCode::CRON_ACTOR_WRONG_CALL;
     }
 
     for (const auto &entry : entries) {
diff --git a/core/vm/actor/cron_actor.hpp b/core/vm/actor/cron_actor.hpp
@@ -16,7 +16,6 @@ namespace fc::vm::actor::cron_actor {
   };
 
   constexpr MethodNumber kEpochTickMethodNumber{2};
-  constexpr VMExitCode WRONG_CALL{1};
 
   /**
    * @brief EpochTick executes built-in periodic actions, run at every Epoch.
diff --git a/core/vm/actor/impl/invoker_impl.cpp b/core/vm/actor/impl/invoker_impl.cpp
@@ -23,16 +23,16 @@ namespace fc::vm::actor {
       MethodNumber method,
       const MethodParams &params) {
     if (actor.code == actor::kAccountCodeCid) {
-      return CANT_INVOKE_ACCOUNT_ACTOR;
+      return VMExitCode::INVOKER_CANT_INVOKE_ACCOUNT_ACTOR;
     }
     auto maybe_builtin_actor = builtin_.find(actor.code);
     if (maybe_builtin_actor == builtin_.end()) {
-      return NO_CODE_OR_METHOD;
+      return VMExitCode::INVOKER_NO_CODE_OR_METHOD;
     }
     auto builtin_actor = maybe_builtin_actor->second;
     auto maybe_builtin_method = builtin_actor.find(method);
     if (maybe_builtin_method == builtin_actor.end()) {
-      return NO_CODE_OR_METHOD;
+      return VMExitCode::INVOKER_NO_CODE_OR_METHOD;
     }
     return maybe_builtin_method->second(actor, runtime, params);
   }
diff --git a/core/vm/actor/impl/invoker_impl.hpp b/core/vm/actor/impl/invoker_impl.hpp
@@ -17,9 +17,6 @@ namespace fc::vm::actor {
   /// Finds and loads actor code, invokes actor methods
   class InvokerImpl : public Invoker {
    public:
-    static constexpr VMExitCode CANT_INVOKE_ACCOUNT_ACTOR{254};
-    static constexpr VMExitCode NO_CODE_OR_METHOD{255};
-
     InvokerImpl();
     ~InvokerImpl() override = default;
     outcome::result<InvocationOutput> invoke(
diff --git a/core/vm/actor/init_actor.cpp b/core/vm/actor/init_actor.cpp
@@ -26,10 +26,10 @@ namespace fc::vm::actor::init_actor {
                                          const MethodParams &params) {
     OUTCOME_TRY(exec_params, decodeActorParams<ExecParams>(params));
     if (!isBuiltinActor(exec_params.code)) {
-      return init_actor::NOT_BUILTIN_ACTOR;
+      return VMExitCode::INIT_ACTOR_NOT_BUILTIN_ACTOR;
     }
     if (isSingletonActor(exec_params.code)) {
-      return init_actor::SINGLETON_ACTOR;
+      return VMExitCode::INIT_ACTOR_SINGLETON_ACTOR;
     }
     OUTCOME_TRY(runtime.chargeGas(runtime::kInitActorExecCost));
     auto &message = runtime.getMessage().get();
diff --git a/core/vm/actor/init_actor.hpp b/core/vm/actor/init_actor.hpp
@@ -38,8 +38,6 @@ namespace fc::vm::actor::init_actor {
   }
 
   constexpr MethodNumber kExecMethodNumber{2};
-  constexpr VMExitCode NOT_BUILTIN_ACTOR{1};
-  constexpr VMExitCode SINGLETON_ACTOR{1};
 
   struct ExecParams {
     CodeId code;
diff --git a/core/vm/actor/storage_power_actor.cpp b/core/vm/actor/storage_power_actor.cpp
@@ -6,6 +6,7 @@
 #include "vm/actor/storage_power_actor.hpp"
 
 #include "power/impl/power_table_impl.hpp"
+#include "vm/exit_code/exit_code.hpp"
 #include "vm/indices/indices.hpp"
 
 namespace fc::vm::actor {
@@ -21,7 +22,9 @@ namespace fc::vm::actor {
       const crypto::randomness::Randomness &randomness) {
     std::vector<primitives::address::Address> selected_miners;
 
-    if (power_table_->getSize() < challenge_count) return OUT_OF_BOUND;
+    if (power_table_->getSize() < challenge_count) {
+      return VMExitCode::STORAGE_POWER_ACTOR_OUT_OF_BOUND;
+    }
 
     OUTCOME_TRY(all_miners, power_table_->getMiners());
 
@@ -181,7 +184,9 @@ namespace fc::vm::actor {
   outcome::result<void> StoragePowerActor::addMiner(
       const primitives::address::Address &miner_addr) {
     auto check = power_table_->getMinerPower(miner_addr);
-    if (!check.has_error()) return ALREADY_EXISTS;
+    if (!check.has_error()) {
+      return VMExitCode::STORAGE_POWER_ACTOR_ALREADY_EXISTS;
+    }
 
     OUTCOME_TRY(power_table_->setMinerPower(miner_addr, 0));
     OUTCOME_TRY(nominal_power_->setMinerPower(miner_addr, 0));
diff --git a/core/vm/actor/storage_power_actor.hpp b/core/vm/actor/storage_power_actor.hpp
@@ -10,7 +10,6 @@
 #include "crypto/randomness/randomness_types.hpp"
 #include "power/power_table.hpp"
 #include "vm/actor/util.hpp"
-#include "vm/exit_code/exit_code.hpp"
 #include "vm/indices/indices.hpp"
 
 namespace fc::vm::actor {
@@ -40,9 +39,6 @@ namespace fc::vm::actor {
     // From spec: 3
     static const size_t kMinMinerSizeTarg;
 
-    static constexpr VMExitCode OUT_OF_BOUND{1};
-    static constexpr VMExitCode ALREADY_EXISTS{2};
-
     StoragePowerActor(std::shared_ptr<Indices> indices,
                       std::shared_ptr<crypto::randomness::RandomnessProvider>
                           randomness_provider);
diff --git a/core/vm/exit_code/exit_code.hpp b/core/vm/exit_code/exit_code.hpp
@@ -12,13 +12,47 @@
 
 namespace fc::vm {
   /**
-   * VM exit code enum for outcome errors. Meaning of exit codes may be
-   * different across actors.
+   * VM exit code enum for outcome errors. Mapping to ret code in range 1-255 is
+   * specified in `getRetCode`.
    */
-  enum class VMExitCode : uint8_t {};
+  enum class VMExitCode {
+    _ = 1,
+    DECODE_ACTOR_PARAMS_ERROR,
+    ENCODE_ACTOR_PARAMS_ERROR,
+
+    INVOKER_CANT_INVOKE_ACCOUNT_ACTOR,
+    INVOKER_NO_CODE_OR_METHOD,
+
+    ACCOUNT_ACTOR_CREATE_WRONG_ADDRESS_TYPE,
+    ACCOUNT_ACTOR_RESOLVE_NOT_FOUND,
+    ACCOUNT_ACTOR_RESOLVE_NOT_ACCOUNT_ACTOR,
+
+    STORAGE_POWER_ACTOR_OUT_OF_BOUND,
+    STORAGE_POWER_ACTOR_ALREADY_EXISTS,
+
+    INIT_ACTOR_NOT_BUILTIN_ACTOR,
+    INIT_ACTOR_SINGLETON_ACTOR,
+
+    CRON_ACTOR_WRONG_CALL,
+  };
 
   /// Distinguish VMExitCode errors from other errors
   bool isVMExitCode(const std::error_code &error);
+
+  /// Get ret code from VMExitCode
+  uint8_t getRetCode(VMExitCode error);
+
+  /// Get ret code from VMExitCode error
+  outcome::result<uint8_t> getRetCode(const std::error_code &error);
+
+  /// Get ret code from outcome
+  template <typename T>
+  outcome::result<uint8_t> getRetCode(const outcome::result<T> &result) {
+    if (result) {
+      return 0;
+    }
+    return getRetCode(result.error());
+  }
 }  // namespace fc::vm
 
 OUTCOME_HPP_DECLARE_ERROR(fc::vm, VMExitCode);
diff --git a/core/vm/exit_code/impl/exit_code.cpp b/core/vm/exit_code/impl/exit_code.cpp
@@ -7,6 +7,8 @@
 
 #include <sstream>
 
+#include <boost/assert.hpp>
+
 #include "common/enum.hpp"
 
 OUTCOME_CPP_DEFINE_CATEGORY(fc::vm, VMExitCode, e) {
@@ -17,6 +19,48 @@ namespace fc::vm {
   bool isVMExitCode(const std::error_code &error) {
     return error.category() == __libp2p::Category<VMExitCode>::get();
   }
+
+  uint8_t getRetCode(VMExitCode error) {
+    using E = VMExitCode;
+    switch (error) {
+      case E::_:
+        break;
+
+      case E::DECODE_ACTOR_PARAMS_ERROR:
+      case E::ENCODE_ACTOR_PARAMS_ERROR:
+        return 1;
+
+      case E::INVOKER_CANT_INVOKE_ACCOUNT_ACTOR:
+        return 254;
+      case E::INVOKER_NO_CODE_OR_METHOD:
+        return 255;
+
+      case E::ACCOUNT_ACTOR_CREATE_WRONG_ADDRESS_TYPE:
+      case E::ACCOUNT_ACTOR_RESOLVE_NOT_FOUND:
+      case E::ACCOUNT_ACTOR_RESOLVE_NOT_ACCOUNT_ACTOR:
+        return 1;
+
+      // TODO(turuslan): FIL-128 StoragePowerActor
+      case E::STORAGE_POWER_ACTOR_OUT_OF_BOUND:
+      case E::STORAGE_POWER_ACTOR_ALREADY_EXISTS:
+        break;
+
+      case E::INIT_ACTOR_NOT_BUILTIN_ACTOR:
+      case E::INIT_ACTOR_SINGLETON_ACTOR:
+        return 1;
+
+      case E::CRON_ACTOR_WRONG_CALL:
+        return 1;
+    }
+    BOOST_ASSERT_MSG(false, "Ret code mapping missing");
+  }
+
+  outcome::result<uint8_t> getRetCode(const std::error_code &error) {
+    if (!isVMExitCode(error)) {
+      return error;
+    }
+    return getRetCode(VMExitCode(error.value()));
+  }
 }  // namespace fc::vm
 
 using fc::common::to_int;
diff --git a/test/core/vm/actor/account_actor_test.cpp b/test/core/vm/actor/account_actor_test.cpp
@@ -8,6 +8,7 @@
 #include "testutil/init_actor.hpp"
 
 using fc::primitives::address::Address;
+using fc::vm::VMExitCode;
 using fc::vm::actor::AccountActor;
 using fc::vm::actor::AccountActorState;
 using fc::vm::state::StateTree;
@@ -37,13 +38,13 @@ TEST(InitActorTest, CreateResolve) {
       0,
       0};
 
-  EXPECT_OUTCOME_ERROR(AccountActor::CREATE_WRONG_ADDRESS_TYPE,
+  EXPECT_OUTCOME_ERROR(VMExitCode::ACCOUNT_ACTOR_CREATE_WRONG_ADDRESS_TYPE,
                        AccountActor::create(state_tree, addr1));
 
-  EXPECT_OUTCOME_ERROR(AccountActor::RESOLVE_NOT_FOUND,
+  EXPECT_OUTCOME_ERROR(VMExitCode::ACCOUNT_ACTOR_RESOLVE_NOT_FOUND,
                        AccountActor::resolveToKeyAddress(state_tree, addr1));
   EXPECT_OUTCOME_TRUE_1(state_tree->set(addr1, actor));
-  EXPECT_OUTCOME_ERROR(AccountActor::RESOLVE_NOT_ACCOUNT_ACTOR,
+  EXPECT_OUTCOME_ERROR(VMExitCode::ACCOUNT_ACTOR_RESOLVE_NOT_ACCOUNT_ACTOR,
                        AccountActor::resolveToKeyAddress(state_tree, addr1));
 
   actor.code = fc::vm::actor::kAccountCodeCid;
diff --git a/test/core/vm/actor/cron_actor_test.cpp b/test/core/vm/actor/cron_actor_test.cpp
@@ -29,7 +29,7 @@ TEST(CronActorTest, WrongSender) {
   EXPECT_CALL(runtime, getMessage())
       .WillOnce(testing::Return(message_wrong_sender));
   EXPECT_OUTCOME_FALSE(err, actor::cron_actor::epochTick(actor, runtime, {}));
-  ASSERT_EQ(err, actor::cron_actor::WRONG_CALL);
+  ASSERT_EQ(err, VMExitCode::CRON_ACTOR_WRONG_CALL);
 }
 
 /**
diff --git a/test/core/vm/actor/init_actor_test.cpp b/test/core/vm/actor/init_actor_test.cpp
@@ -18,6 +18,7 @@ namespace InitActor = fc::vm::actor::init_actor;
 
 using fc::primitives::BigInt;
 using fc::primitives::address::Address;
+using fc::vm::VMExitCode;
 using fc::vm::actor::CodeId;
 using fc::vm::actor::InvocationOutput;
 using fc::vm::actor::kInitAddress;
@@ -78,10 +79,10 @@ TEST(InitActorExecText, ExecError) {
   MockRuntime runtime;
 
   EXPECT_OUTCOME_ERROR(
-      InitActor::NOT_BUILTIN_ACTOR,
+      VMExitCode::INIT_ACTOR_NOT_BUILTIN_ACTOR,
       InitActor::exec({}, runtime, execParams("010001020000"_cid, {})));
   EXPECT_OUTCOME_ERROR(
-      InitActor::SINGLETON_ACTOR,
+      VMExitCode::INIT_ACTOR_SINGLETON_ACTOR,
       InitActor::exec(
           {}, runtime, execParams(fc::vm::actor::kInitCodeCid, {})));
 }
diff --git a/test/core/vm/actor/invoker_test.cpp b/test/core/vm/actor/invoker_test.cpp
@@ -24,17 +24,17 @@ TEST(InvokerTest, InvokeCron) {
   MockRuntime runtime;
 
   EXPECT_OUTCOME_ERROR(
-      InvokerImpl::CANT_INVOKE_ACCOUNT_ACTOR,
+      VMExitCode::INVOKER_CANT_INVOKE_ACCOUNT_ACTOR,
       invoker.invoke({kAccountCodeCid}, runtime, MethodNumber{0}, {}));
   EXPECT_OUTCOME_ERROR(
-      InvokerImpl::NO_CODE_OR_METHOD,
+      VMExitCode::INVOKER_NO_CODE_OR_METHOD,
       invoker.invoke({CodeId{kEmptyObjectCid}}, runtime, MethodNumber{0}, {}));
   EXPECT_OUTCOME_ERROR(
-      InvokerImpl::NO_CODE_OR_METHOD,
+      VMExitCode::INVOKER_NO_CODE_OR_METHOD,
       invoker.invoke({kCronCodeCid}, runtime, MethodNumber{1000}, {}));
   EXPECT_CALL(runtime, getMessage()).WillOnce(testing::Return(message));
   EXPECT_OUTCOME_ERROR(
-      cron_actor::WRONG_CALL,
+      VMExitCode::CRON_ACTOR_WRONG_CALL,
       invoker.invoke(
           {kCronCodeCid}, runtime, cron_actor::kEpochTickMethodNumber, {}));
 }
@@ -44,7 +44,7 @@ TEST(InvokerTest, DecodeActorParams) {
   using fc::vm::actor::decodeActorParams;
 
   // 80 is cbor empty list, not int
-  EXPECT_OUTCOME_ERROR(fc::vm::actor::DECODE_ACTOR_PARAMS_ERROR,
+  EXPECT_OUTCOME_ERROR(VMExitCode::DECODE_ACTOR_PARAMS_ERROR,
                        decodeActorParams<int>("80"_unhex));
   EXPECT_OUTCOME_EQ(decodeActorParams<int>("03"_unhex), 3);
 }
@@ -53,7 +53,7 @@ TEST(InvokerTest, DecodeActorParams) {
 TEST(InvokerTest, EncodeActorParams) {
   using fc::vm::actor::encodeActorParams;
 
-  EXPECT_OUTCOME_ERROR(fc::vm::actor::ENCODE_ACTOR_PARAMS_ERROR,
+  EXPECT_OUTCOME_ERROR(VMExitCode::ENCODE_ACTOR_PARAMS_ERROR,
                        encodeActorParams(fc::CID()));
   EXPECT_OUTCOME_EQ(encodeActorParams(3), "03"_unhex);
 }
diff --git a/test/core/vm/actor/storage_power_actor_test.cpp b/test/core/vm/actor/storage_power_actor_test.cpp

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ namespace fc::vm::actor::cron_actor {`
`17`	`17`	`Runtime &runtime,`
`18`	`18`	`const MethodParams &params) {`
`19`	`19`	`if ((runtime.getMessage().get().from != kCronAddress)) {`
`20`		`- return cron_actor::WRONG_CALL;`
	`20`	`+ return VMExitCode::CRON_ACTOR_WRONG_CALL;`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`for (const auto &entry : entries) {`
Original file line number	Diff line number	Diff line change
`@@ -23,16 +23,16 @@ namespace fc::vm::actor {`
`23`	`23`	`MethodNumber method,`
`24`	`24`	`const MethodParams &params) {`
`25`	`25`	`if (actor.code == actor::kAccountCodeCid) {`
`26`		`- return CANT_INVOKE_ACCOUNT_ACTOR;`
	`26`	`+ return VMExitCode::INVOKER_CANT_INVOKE_ACCOUNT_ACTOR;`
`27`	`27`	`}`
`28`	`28`	`auto maybe_builtin_actor = builtin_.find(actor.code);`
`29`	`29`	`if (maybe_builtin_actor == builtin_.end()) {`
`30`		`- return NO_CODE_OR_METHOD;`
	`30`	`+ return VMExitCode::INVOKER_NO_CODE_OR_METHOD;`
`31`	`31`	`}`
`32`	`32`	`auto builtin_actor = maybe_builtin_actor->second;`
`33`	`33`	`auto maybe_builtin_method = builtin_actor.find(method);`
`34`	`34`	`if (maybe_builtin_method == builtin_actor.end()) {`
`35`		`- return NO_CODE_OR_METHOD;`
	`35`	`+ return VMExitCode::INVOKER_NO_CODE_OR_METHOD;`
`36`	`36`	`}`
`37`	`37`	`return maybe_builtin_method->second(actor, runtime, params);`
`38`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,10 +26,10 @@ namespace fc::vm::actor::init_actor {`
`26`	`26`	`const MethodParams &params) {`
`27`	`27`	`OUTCOME_TRY(exec_params, decodeActorParams<ExecParams>(params));`
`28`	`28`	`if (!isBuiltinActor(exec_params.code)) {`
`29`		`- return init_actor::NOT_BUILTIN_ACTOR;`
	`29`	`+ return VMExitCode::INIT_ACTOR_NOT_BUILTIN_ACTOR;`
`30`	`30`	`}`
`31`	`31`	`if (isSingletonActor(exec_params.code)) {`
`32`		`- return init_actor::SINGLETON_ACTOR;`
	`32`	`+ return VMExitCode::INIT_ACTOR_SINGLETON_ACTOR;`
`33`	`33`	`}`
`34`	`34`	`OUTCOME_TRY(runtime.chargeGas(runtime::kInitActorExecCost));`
`35`	`35`	`auto &message = runtime.getMessage().get();`
Original file line number	Diff line number	Diff line change
`@@ -38,8 +38,6 @@ namespace fc::vm::actor::init_actor {`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`constexpr MethodNumber kExecMethodNumber{2};`
`41`		`- constexpr VMExitCode NOT_BUILTIN_ACTOR{1};`
`42`		`- constexpr VMExitCode SINGLETON_ACTOR{1};`
`43`	`41`
`44`	`42`	`struct ExecParams {`
`45`	`43`	`CodeId code;`