wallet: Improve Fee Priority Code for Clarity

src/simplewallet/simplewallet.cpp

@@ -43,6 +43,7 @@
 #include <iostream>
 #include <sstream>
 #include <fstream>
+#include <string_view>
 #include <ctype.h>
 #include <boost/lexical_cast.hpp>
 #include <boost/program_options.hpp>
@@ -71,6 +72,7 @@
 #include "ringct/rctSigs.h"
 #include "multisig/multisig.h"
 #include "wallet/wallet_args.h"
+#include "wallet/fee_priority.h"
 #include "version.h"
 #include <stdexcept>
 #include "wallet/message_store.h"
@@ -92,6 +94,7 @@ using namespace cryptonote;
 using boost::lexical_cast;
 namespace po = boost::program_options;
 typedef cryptonote::simple_wallet sw;
+using tools::fee_priority;
 
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "wallet.simplewallet"
@@ -164,7 +167,7 @@ static std::string get_human_readable_timespan(uint64_t seconds);
 
 namespace
 {
-  const std::array<const char* const, 5> allowed_priority_strings = {{"default", "unimportant", "normal", "elevated", "priority"}};
+  constexpr std::array<std::string_view, 5> allowed_priority_strings = tools::fee_priority_utilities::fee_priority_strings;
   const auto arg_wallet_file = wallet_args::arg_wallet_file();
   const command_line::arg_descriptor<std::string> arg_generate_new_wallet = {"generate-new-wallet", sw::tr("Generate new wallet and save it to <arg>"), ""};
   const command_line::arg_descriptor<std::string> arg_generate_from_device = {"generate-from-device", sw::tr("Generate new wallet from device and save it to <arg>"), ""};
@@ -763,17 +766,13 @@ namespace
   }
 }
 
-bool parse_priority(const std::string& arg, uint32_t& priority)
+bool parse_priority(const std::string& arg, fee_priority& priority)
 {
-  auto priority_pos = std::find(
+  const auto priority_optional = tools::fee_priority_utilities::from_string(arg);
-    allowed_priority_strings.begin(),
+  if (!priority_optional.has_value())
-    allowed_priority_strings.end(),
+    return false;
-    arg);
+  priority = priority_optional.value();
-  if(priority_pos != allowed_priority_strings.end()) {
+  return true;
-    priority = std::distance(allowed_priority_strings.begin(), priority_pos);
-    return true;
-  }
-  return false;
 }
 
 std::string join_priority_strings(const char *delimiter)
@@ -1032,8 +1031,10 @@ bool simple_wallet::print_fee_info(const std::vector<std::string> &args/* = std:
   message_writer() << (boost::format(tr("Current fee is %s %s per %s")) % print_money(base_fee) % cryptonote::get_unit(cryptonote::get_default_decimal_point()) % base).str();
 
   std::vector<uint64_t> fees;
-  for (uint32_t priority = 1; priority <= 4; ++priority)
+  for (const auto priority : tools::fee_priority_utilities::enums)
   {
+    if (priority == fee_priority::Default)
+      continue;
     uint64_t mult = m_wallet->get_fee_multiplier(priority);
     fees.push_back(base_fee * typical_size * mult);
   }
@@ -1054,23 +1055,29 @@ bool simple_wallet::print_fee_info(const std::vector<std::string> &args/* = std:
     return true;
   }
 
-  for (uint32_t priority = 1; priority <= 4; ++priority)
+  for (const auto priority : tools::fee_priority_utilities::enums)
   {
-    uint64_t nblocks_low = blocks[priority - 1].first;
+    if (priority == tools::fee_priority::Default)
-    uint64_t nblocks_high = blocks[priority - 1].second;
+      continue;
+
+    const auto lower_priority = tools::fee_priority_utilities::decrease(priority);
+    const auto lower_priority_index = tools::fee_priority_utilities::as_integral(lower_priority);
+    const auto current_priority_index = tools::fee_priority_utilities::as_integral(priority);
+    uint64_t nblocks_low = blocks[lower_priority_index].first;
+    uint64_t nblocks_high = blocks[lower_priority_index].second;
     if (nblocks_low > 0)
     {
       std::string msg;
-      if (priority == m_wallet->get_default_priority() || (m_wallet->get_default_priority() == 0 && priority == 2))
+      if (priority == m_wallet->get_default_priority() || (m_wallet->get_default_priority() == fee_priority::Default && priority == fee_priority::Normal))
         msg = tr(" (current)");
       uint64_t minutes_low = nblocks_low * DIFFICULTY_TARGET_V2 / 60, minutes_high = nblocks_high * DIFFICULTY_TARGET_V2 / 60;
       if (nblocks_high == nblocks_low)
-        message_writer() << (boost::format(tr("%u block (%u minutes) backlog at priority %u%s")) % nblocks_low % minutes_low % priority % msg).str();
+        message_writer() << (boost::format(tr("%u block (%u minutes) backlog at priority %u%s")) % nblocks_low % minutes_low % current_priority_index % msg).str();
       else
-        message_writer() << (boost::format(tr("%u to %u block (%u to %u minutes) backlog at priority %u")) % nblocks_low % nblocks_high % minutes_low % minutes_high % priority).str();
+        message_writer() << (boost::format(tr("%u to %u block (%u to %u minutes) backlog at priority %u")) % nblocks_low % nblocks_high % minutes_low % minutes_high % current_priority_index).str();
     }
     else
-      message_writer() << tr("No backlog at priority ") << priority;
+      message_writer() << tr("No backlog at priority ") << current_priority_index;
   }
   return true;
 }
@@ -2463,7 +2470,7 @@ bool simple_wallet::set_default_priority(const std::vector<std::string> &args/*
       if (!found)
       {
         priority = boost::lexical_cast<int>(args[1]);
-        if (priority < 1 || priority > 4)
+        if (priority < tools::fee_priority_utilities::as_integral(fee_priority::Unimportant) || priority > tools::fee_priority_utilities::as_integral(fee_priority::Priority))
         {
           fail_msg_writer() << tr("priority must be either 0, 1, 2, 3, or 4, or one of: ") << join_priority_strings(", ");
           return true;
@@ -2474,7 +2481,7 @@ bool simple_wallet::set_default_priority(const std::vector<std::string> &args/*
     const auto pwd_container = get_and_verify_password();
     if (pwd_container)
     {
-      m_wallet->set_default_priority(priority);
+      m_wallet->set_default_priority(tools::fee_priority_utilities::from_integral(priority));
       m_wallet->rewrite(m_wallet_file, pwd_container->password());
     }
     return true;
@@ -3711,9 +3718,9 @@ bool simple_wallet::set_variable(const std::vector<std::string> &args)
     if (m_use_english_language_names)
       seed_language = crypto::ElectrumWords::get_english_name_for(seed_language);
     std::string priority_string = "invalid";
-    uint32_t priority = m_wallet->get_default_priority();
+    const fee_priority priority = m_wallet->get_default_priority();
-    if (priority < allowed_priority_strings.size())
+    priority_string = tools::fee_priority_utilities::to_string(priority);
-      priority_string = allowed_priority_strings[priority];
+    const auto priority_index = tools::fee_priority_utilities::as_integral(priority);
     std::string ask_password_string = "invalid";
     switch (m_wallet->ask_password())
     {
@@ -3735,7 +3742,7 @@ bool simple_wallet::set_variable(const std::vector<std::string> &args)
     success_msg_writer() << "default-ring-size = " << (m_wallet->default_mixin() ? m_wallet->default_mixin() + 1 : 0);
     success_msg_writer() << "auto-refresh = " << m_wallet->auto_refresh();
     success_msg_writer() << "refresh-type = " << get_refresh_type_name(m_wallet->get_refresh_type());
-    success_msg_writer() << "priority = " << priority<< " (" << priority_string << ")";
+    success_msg_writer() << "priority = " << priority_index << " (" << priority_string << ")";
     success_msg_writer() << "ask-password = " << m_wallet->ask_password() << " (" << ask_password_string << ")";
     success_msg_writer() << "unit = " << cryptonote::get_unit(cryptonote::get_default_decimal_point());
     success_msg_writer() << "max-reorg-depth = " << m_wallet->max_reorg_depth();
@@ -6440,7 +6447,7 @@ bool simple_wallet::transfer_main(const std::vector<std::string> &args_, bool ca
     local_args.erase(local_args.begin());
   }
 
-  uint32_t priority = m_wallet->get_default_priority();
+  fee_priority priority = m_wallet->get_default_priority();
   if (local_args.size() > 0 && parse_priority(local_args[0], priority))
     local_args.erase(local_args.begin());
 
@@ -7005,7 +7012,7 @@ bool simple_wallet::sweep_main(uint32_t account, uint64_t below, const std::vect
     local_args.erase(local_args.begin());
   }
 
-  uint32_t priority = 0;
+  fee_priority priority = fee_priority::Default;
   if (local_args.size() > 0 && parse_priority(local_args[0], priority))
     local_args.erase(local_args.begin());
 
@@ -7249,7 +7256,7 @@ bool simple_wallet::sweep_single(const std::vector<std::string> &args_)
 
   std::vector<std::string> local_args = args_;
 
-  uint32_t priority = 0;
+  fee_priority priority = fee_priority::Default;
   if (local_args.size() > 0 && parse_priority(local_args[0], priority))
     local_args.erase(local_args.begin());
 

src/wallet/api/wallet.cpp

@@ -1623,7 +1623,7 @@ PendingTransaction *WalletImpl::createTransactionMultDest(const std::vector<stri
       
     cryptonote::address_parse_info info;
 
-    uint32_t adjusted_priority = m_wallet->adjust_priority(static_cast<uint32_t>(priority));
+    const auto adjusted_priority = m_wallet->adjust_priority(static_cast<uint32_t>(priority));
 
     PendingTransactionImpl * transaction = new PendingTransactionImpl(*this);
 
@@ -1896,7 +1896,7 @@ uint64_t WalletImpl::estimateTransactionFee(const std::vector<std::pair<std::str
         m_wallet->use_fork_rules(HF_VERSION_CLSAG, 0),
         m_wallet->use_fork_rules(HF_VERSION_BULLETPROOF_PLUS, 0),
         m_wallet->use_fork_rules(HF_VERSION_VIEW_TAGS, 0),
-        m_wallet->get_base_fee(priority),
+        m_wallet->get_base_fee(static_cast<uint32_t>(priority)),
         m_wallet->get_fee_quantization_mask());
 }
 

src/wallet/fee_algorithm.h

@@ -0,0 +1,21 @@
+#pragma once
+
+namespace tools
+{
+    enum class fee_algorithm : int
+    {
+        Unset = -1,
+        PreHardforkV3, /* Original */
+        HardforkV3,
+        HardforkV5,
+        HardforkV8,
+    };
+
+    namespace fee_algorithm_utilities
+    {
+        static int as_integral(const fee_algorithm algorithm)
+        {
+            return static_cast<int>(algorithm);
+        }
+    }
+}

src/wallet/fee_priority.cpp

@@ -0,0 +1,9 @@
+#include "fee_priority.h"
+
+namespace tools
+{
+    std::ostream& operator<<(std::ostream& os, const fee_priority priority)
+    {
+        return os << fee_priority_utilities::to_string(priority);
+    }
+}

src/wallet/fee_priority.h

@@ -0,0 +1,118 @@
+#pragma once
+
+#include <stdint.h>
+#include <string>
+#include <string_view>
+#include <array>
+#include <algorithm>
+#include <iterator>
+#include <optional>
+#include <iosfwd>
+
+namespace tools
+{
+    enum class fee_priority : uint32_t
+    {
+        // If adding or removing an enumeration, ensure to update enums and fee_priority_strings in the class below.
+        Default = 0,
+        Unimportant, /* Low */
+        Normal, /* Medium */
+        Elevated, /* High */
+        Priority, /* Very High */
+    };
+
+    std::ostream& operator<<(std::ostream& os, const fee_priority priority);
+
+    namespace fee_priority_utilities
+    {
+        using EnumStringsType = std::array<std::string_view, 5>;
+        using EnumsType = std::array<fee_priority, 5>;
+
+        inline constexpr EnumStringsType fee_priority_strings = { { "default", "unimportant", "normal", "elevated", "priority" } };
+        inline constexpr EnumsType enums = { { fee_priority::Default, fee_priority::Unimportant, fee_priority::Normal, fee_priority::Elevated, fee_priority::Priority } };
+
+        static fee_priority decrease(const fee_priority priority)
+        {
+            if (priority == fee_priority::Default)
+            {
+                return fee_priority::Default;
+            }
+            else
+            {
+                const uint32_t integralValue = static_cast<uint32_t>(priority);
+                const auto decrementedIntegralValue = integralValue - 1u;
+                return static_cast<fee_priority>(decrementedIntegralValue);
+            }
+        }
+
+        static constexpr uint32_t as_integral(const fee_priority priority)
+        {
+            return static_cast<uint32_t>(priority);
+        }
+
+        static constexpr fee_priority from_integral(const uint32_t priority)
+        {
+            if (priority >= as_integral(fee_priority::Priority))
+            {
+                return fee_priority::Priority;
+            }
+
+            return static_cast<fee_priority>(priority);
+        }
+
+        static bool is_valid(const uint32_t priority)
+        {
+            return priority <= as_integral(fee_priority::Priority);
+        }
+
+        static fee_priority clamp(const fee_priority priority)
+        {
+            const auto highest = as_integral(fee_priority::Priority);
+            const auto lowest = as_integral(fee_priority::Default);
+            const auto current = as_integral(priority);
+
+            if (current < lowest)
+            {
+                return fee_priority::Default;
+            }
+            else if (current > highest)
+            {
+                return fee_priority::Priority;
+            }
+            else
+            {
+                return priority;
+            }
+        }
+
+        static fee_priority clamp_modified(const fee_priority priority)
+        {
+            /* Map Default to an actionable priority. */
+            if (priority == fee_priority::Default)
+            {
+                return fee_priority::Unimportant;
+            }
+            else
+            {
+                return clamp(priority);
+            }
+        }
+
+        static std::string_view to_string(const fee_priority priority)
+        {
+            const auto integralValue = as_integral(clamp(priority));
+            return fee_priority_strings.at(integralValue);
+        }
+
+        static std::optional<fee_priority> from_string(const std::string& str)
+        {
+            const auto strIterator = std::find(fee_priority_strings.begin(), fee_priority_strings.end(), str);
+            if (strIterator == fee_priority_strings.end())
+                return std::nullopt;
+
+            const auto distance = std::distance(fee_priority_strings.begin(), strIterator);
+            return enums.at(distance);
+        }
+
+    }
+}

src/wallet/wallet2.cpp

@@ -40,6 +40,7 @@
 #include <boost/algorithm/string/split.hpp>
 #include <boost/algorithm/string/join.hpp>
 #include <boost/algorithm/string/predicate.hpp>
+#include <boost/numeric/conversion/cast.hpp>
 #include <boost/archive/binary_iarchive.hpp>
 #include <boost/asio/ip/address.hpp>
 #include <boost/filesystem/operations.hpp>
@@ -1198,7 +1199,7 @@ wallet2::wallet2(network_type nettype, uint64_t kdf_rounds, bool unattended, std
   m_print_ring_members(false),
   m_store_tx_info(true),
   m_default_mixin(0),
-  m_default_priority(0),
+  m_default_priority(fee_priority::Default),
   m_refresh_type(RefreshOptimizeCoinbase),
   m_auto_refresh(true),
   m_first_refresh_done(false),
@@ -4606,7 +4607,7 @@ boost::optional<wallet2::keys_file_data> wallet2::get_keys_file_data(const crypt
   value2.SetUint(m_default_mixin);
   json.AddMember("default_mixin", value2, json.GetAllocator());
 
-  value2.SetUint(m_default_priority);
+  value2.SetUint(boost::numeric_cast<unsigned>(fee_priority_utilities::as_integral(m_default_priority)));
   json.AddMember("default_priority", value2, json.GetAllocator());
 
   value2.SetInt(m_auto_refresh ? 1 :0);
@@ -4938,7 +4939,7 @@ bool wallet2::load_keys_buf(const std::string& keys_buf, const epee::wipeable_st
     m_print_ring_members = false;
     m_store_tx_info = true;
     m_default_mixin = 0;
-    m_default_priority = 0;
+    m_default_priority = fee_priority::Default;
     m_auto_refresh = true;
     m_refresh_type = RefreshType::RefreshDefault;
     m_refresh_from_block_height = 0;
@@ -5071,15 +5072,15 @@ bool wallet2::load_keys_buf(const std::string& keys_buf, const epee::wipeable_st
     GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, default_priority, unsigned int, Uint, false, 0);
     if (field_default_priority_found)
     {
-      m_default_priority = field_default_priority;
+      m_default_priority = fee_priority_utilities::from_integral(boost::numeric_cast<uint32_t>(field_default_priority));
     }
     else
     {
       GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, default_fee_multiplier, unsigned int, Uint, false, 0);
       if (field_default_fee_multiplier_found)
-        m_default_priority = field_default_fee_multiplier;
+        m_default_priority = fee_priority_utilities::from_integral(boost::numeric_cast<uint32_t>(field_default_fee_multiplier));
       else
-        m_default_priority = 0;
+        m_default_priority = fee_priority::Default;
     }
     GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, auto_refresh, int, Int, false, true);
     m_auto_refresh = field_auto_refresh;
@@ -8470,42 +8471,45 @@ uint64_t wallet2::estimate_fee(bool use_per_byte_fee, bool use_rct, int n_inputs
   }
 }
 
-uint64_t wallet2::get_fee_multiplier(uint32_t priority, int fee_algorithm)
+uint64_t wallet2::get_fee_multiplier(fee_priority priority, fee_algorithm fee_algorithm)
 {
-  static const struct
+  struct fee_step
   {
-    size_t count;
+    fee_priority maximum_priority; // Determines the maximum priority available for said fee algorithm.
-    uint64_t multipliers[4];
+    uint64_t fee_multipliers[4]; // Determines the fee multiplier applied at various priorities for said fee algorithm.
-  }
-  multipliers[] =
-  {
-    { 3, {1, 2, 3} },
-    { 3, {1, 20, 166} },
-    { 4, {1, 4, 20, 166} },
-    { 4, {1, 5, 25, 1000} },
   };
 
-  if (fee_algorithm == -1)
+  static constexpr fee_step fee_steps[] =
+  {
+    { fee_priority::Elevated, {1, 2, 3} },
+    { fee_priority::Elevated, {1, 20, 166} },
+    { fee_priority::Priority, {1, 4, 20, 166} },
+    { fee_priority::Priority, {1, 5, 25, 1000} },
+  };
+
+  if (fee_algorithm == fee_algorithm::Unset)
     fee_algorithm = get_fee_algorithm();
 
   // 0 -> default (here, x1 till fee algorithm 2, x4 from it)
-  if (priority == 0)
+  if (priority == fee_priority::Default)
     priority = m_default_priority;
-  if (priority == 0)
+  if (priority == fee_priority::Default)
   {
-    if (fee_algorithm >= 2)
+    if (fee_algorithm >= fee_algorithm::HardforkV5)
-      priority = 2;
+      priority = fee_priority::Normal;
     else
-      priority = 1;
+      priority = fee_priority::Unimportant;
   }
 
-  THROW_WALLET_EXCEPTION_IF(fee_algorithm < 0 || fee_algorithm > 3, error::invalid_priority);
+  const auto fee_algorithm_index = fee_algorithm_utilities::as_integral(fee_algorithm);
+  THROW_WALLET_EXCEPTION_IF(fee_algorithm_index < 0 || fee_algorithm_index > static_cast<int>(std::size(fee_steps)), error::invalid_priority);
 
   // 1 to 3/4 are allowed as priorities
-  const uint32_t max_priority = multipliers[fee_algorithm].count;
+  const fee_priority max_priority = fee_steps[fee_algorithm_index].maximum_priority;
-  if (priority >= 1 && priority <= max_priority)
+  if (priority >= fee_priority::Unimportant && priority <= max_priority)
   {
-    return multipliers[fee_algorithm].multipliers[priority-1];
+    const fee_priority adjusted_priority = fee_priority_utilities::decrease(priority);
+    return fee_steps[fee_algorithm_index].fee_multipliers[fee_priority_utilities::as_integral(adjusted_priority)];
   }
 
   THROW_WALLET_EXCEPTION_IF (false, error::invalid_priority);
@@ -8533,16 +8537,18 @@ uint64_t wallet2::get_base_fee()
 }
 //----------------------------------------------------------------------------------------------------
 uint64_t wallet2::get_base_fee(uint32_t priority)
+{
+  return get_base_fee(fee_priority_utilities::from_integral(priority));
+}
+//----------------------------------------------------------------------------------------------------
+uint64_t wallet2::get_base_fee(fee_priority priority)
 {
   const bool use_2021_scaling = use_fork_rules(HF_VERSION_2021_SCALING, -30 * 1);
   if (use_2021_scaling)
   {
     // clamp and map to 0..3 indices, mapping 0 (default, but should not end up here) to 0, and 1..4 to 0..3
-    if (priority == 0)
+    priority = fee_priority_utilities::clamp_modified(priority);
-      priority = 1;
+    priority = fee_priority_utilities::decrease(priority);
-    else if (priority > 4)
-      priority = 4;
-    --priority;
 
     std::vector<uint64_t> fees;
     boost::optional<std::string> result = m_node_rpc_proxy.get_dynamic_base_fee_estimate_2021_scaling(FEE_ESTIMATE_GRACE_BLOCKS, fees);
@@ -8551,12 +8557,14 @@ uint64_t wallet2::get_base_fee(uint32_t priority)
       MERROR("Failed to determine base fee, using default");
       return FEE_PER_BYTE;
     }
-    if (priority >= fees.size())
+
+    const auto priority_index = fee_priority_utilities::as_integral(priority);
+    if (priority_index >= fees.size())
     {
-      MERROR("Failed to determine base fee for priority " << priority << ", using default");
+      MERROR("Failed to determine base fee for priority " << priority_index << ", using default");
       return FEE_PER_BYTE;
     }
-    return fees[priority];
+    return fees[priority_index];
   }
   else
   {
@@ -8579,16 +8587,16 @@ uint64_t wallet2::get_fee_quantization_mask()
   return fee_quantization_mask;
 }
 //----------------------------------------------------------------------------------------------------
-int wallet2::get_fee_algorithm()
+fee_algorithm wallet2::get_fee_algorithm()
 {
   // changes at v3, v5, v8
   if (use_fork_rules(HF_VERSION_PER_BYTE_FEE, 0))
-    return 3;
+    return fee_algorithm::HardforkV8;
   if (use_fork_rules(5, 0))
-    return 2;
+    return fee_algorithm::HardforkV5;
   if (use_fork_rules(3, -30 * 14))
-   return 1;
+   return fee_algorithm::HardforkV3;
-  return 0;
+  return fee_algorithm::PreHardforkV3;
 }
 //------------------------------------------------------------------------------------------------------------------------------
 uint64_t wallet2::get_min_ring_size()
@@ -8632,15 +8640,20 @@ uint64_t wallet2::adjust_mixin(uint64_t mixin)
   return mixin;
 }
 //----------------------------------------------------------------------------------------------------
-uint32_t wallet2::adjust_priority(uint32_t priority)
+fee_priority wallet2::adjust_priority(uint32_t priority)
 {
-  if (priority == 0 && m_default_priority == 0 && auto_low_priority())
+  return adjust_priority(fee_priority_utilities::from_integral(priority));
+}
+//----------------------------------------------------------------------------------------------------
+fee_priority wallet2::adjust_priority(fee_priority priority)
+{
+  if (priority == fee_priority::Default && m_default_priority == fee_priority::Default && auto_low_priority())
   {
     try
     {
       // check if there's a backlog in the tx pool
       const bool use_per_byte_fee = use_fork_rules(HF_VERSION_PER_BYTE_FEE, 0);
-      const uint64_t base_fee = get_base_fee(1);
+      const uint64_t base_fee = get_base_fee(fee_priority::Unimportant);
       const double fee_level = base_fee * (use_per_byte_fee ? 1 : (12/(double)13 / (double)1024));
       const std::vector<std::pair<uint64_t, uint64_t>> blocks = estimate_backlog({std::make_pair(fee_level, fee_level)});
       if (blocks.size() != 1)
@@ -8651,7 +8664,7 @@ uint32_t wallet2::adjust_priority(uint32_t priority)
       else if (blocks[0].first > 0)
       {
         MINFO("We don't use the low priority because there's a backlog in the tx pool.");
-        return 2;
+        return fee_priority::Normal;
       }
 
       // get the current full reward zone
@@ -8696,10 +8709,10 @@ uint32_t wallet2::adjust_priority(uint32_t priority)
       if (P > 80)
       {
         MINFO("We don't use the low priority because recent blocks are quite full.");
-        return 2;
+        return fee_priority::Normal;
       }
       MINFO("We'll use the low priority because probably it's safe to do so.");
-      return 1;
+      return fee_priority::Unimportant;
     }
     catch (const std::exception &e)
     {
@@ -10420,7 +10433,7 @@ static uint32_t get_count_above(const std::vector<wallet2::transfer_details> &tr
 // This system allows for sending (almost) the entire balance, since it does
 // not generate spurious change in all txes, thus decreasing the instantaneous
 // usable balance.
-std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryptonote::tx_destination_entry> dsts, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, const unique_index_container& subtract_fee_from_outputs)
+std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryptonote::tx_destination_entry> dsts, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, const unique_index_container& subtract_fee_from_outputs)
 {
   //ensure device is let in NONE mode in any case
   hw::device &hwdev = m_account.get_device();
@@ -11188,7 +11201,7 @@ bool wallet2::sanity_check(const std::vector<wallet2::pending_tx> &ptx_vector, c
   return true;
 }
 
-std::vector<wallet2::pending_tx> wallet2::create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices)
+std::vector<wallet2::pending_tx> wallet2::create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices)
 {
   std::vector<size_t> unused_transfers_indices;
   std::vector<size_t> unused_dust_indices;
@@ -11261,7 +11274,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_all(uint64_t below
   return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, priority, extra);
 }
 
-std::vector<wallet2::pending_tx> wallet2::create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra)
+std::vector<wallet2::pending_tx> wallet2::create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra)
 {
   std::vector<size_t> unused_transfers_indices;
   std::vector<size_t> unused_dust_indices;
@@ -11282,7 +11295,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_single(const crypt
   return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, priority, extra);
 }
 
-std::vector<wallet2::pending_tx> wallet2::create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector<size_t> unused_transfers_indices, std::vector<size_t> unused_dust_indices, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra)
+std::vector<wallet2::pending_tx> wallet2::create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector<size_t> unused_transfers_indices, std::vector<size_t> unused_dust_indices, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra)
 {
   //ensure device is let in NONE mode in any case
   hw::device &hwdev = m_account.get_device();
@@ -11751,7 +11764,7 @@ std::vector<wallet2::pending_tx> wallet2::create_unmixable_sweep_transactions()
   const bool hf1_rules = use_fork_rules(2, 10); // first hard fork has version 2
   tx_dust_policy dust_policy(hf1_rules ? 0 : ::config::DEFAULT_DUST_THRESHOLD);
 
-  const uint64_t base_fee  = get_base_fee(1);
+  const uint64_t base_fee  = get_base_fee(fee_priority::Unimportant);
 
   // may throw
   std::vector<size_t> unmixable_outputs = select_available_unmixable_outputs();
@@ -11772,7 +11785,7 @@ std::vector<wallet2::pending_tx> wallet2::create_unmixable_sweep_transactions()
       unmixable_transfer_outputs.push_back(n);
   }
 
-  return create_transactions_from(m_account_public_address, false, 1, unmixable_transfer_outputs, unmixable_dust_outputs, 0 /*fake_outs_count */, 1 /*priority */, std::vector<uint8_t>());
+  return create_transactions_from(m_account_public_address, false, 1, unmixable_transfer_outputs, unmixable_dust_outputs, 0 /*fake_outs_count */, fee_priority::Unimportant, std::vector<uint8_t>());
 }
 //----------------------------------------------------------------------------------------------------
 void wallet2::discard_unmixable_outputs()

src/wallet/wallet2.h

@@ -71,6 +71,8 @@
 #include "common/password.h"
 #include "node_rpc_proxy.h"
 #include "message_store.h"
+#include "fee_priority.h"
+#include "fee_algorithm.h"
 
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "wallet.wallet2"
@@ -1190,10 +1192,10 @@ private:
     bool parse_unsigned_tx_from_str(const std::string &unsigned_tx_st, unsigned_tx_set &exported_txs) const;
     bool load_tx(const std::string &signed_filename, std::vector<tools::wallet2::pending_tx> &ptx, std::function<bool(const signed_tx_set&)> accept_func = NULL);
     bool parse_tx_from_str(const std::string &signed_tx_st, std::vector<tools::wallet2::pending_tx> &ptx, std::function<bool(const signed_tx_set &)> accept_func);
-    std::vector<wallet2::pending_tx> create_transactions_2(std::vector<cryptonote::tx_destination_entry> dsts, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, const unique_index_container& subtract_fee_from_outputs = {});     // pass subaddr_indices by value on purpose
+    std::vector<wallet2::pending_tx> create_transactions_2(std::vector<cryptonote::tx_destination_entry> dsts, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, const unique_index_container& subtract_fee_from_outputs = {});     // pass subaddr_indices by value on purpose
-    std::vector<wallet2::pending_tx> create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices);
+    std::vector<wallet2::pending_tx> create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices);
-    std::vector<wallet2::pending_tx> create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra);
+    std::vector<wallet2::pending_tx> create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra);
-    std::vector<wallet2::pending_tx> create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector<size_t> unused_transfers_indices, std::vector<size_t> unused_dust_indices, const size_t fake_outs_count, uint32_t priority, const std::vector<uint8_t>& extra);
+    std::vector<wallet2::pending_tx> create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector<size_t> unused_transfers_indices, std::vector<size_t> unused_dust_indices, const size_t fake_outs_count, fee_priority priority, const std::vector<uint8_t>& extra);
     bool sanity_check(const std::vector<wallet2::pending_tx> &ptx_vector, const std::vector<cryptonote::tx_destination_entry>& dsts, const unique_index_container& subtract_fee_from_outputs = {}) const;
     void cold_tx_aux_import(const std::vector<pending_tx>& ptx, const std::vector<std::string>& tx_device_aux);
     void cold_sign_tx(const std::vector<pending_tx>& ptx_vector, signed_tx_set &exported_txs, std::vector<cryptonote::address_parse_info> &dsts_info, std::vector<std::string> & tx_device_aux);
@@ -1432,8 +1434,8 @@ private:
     void store_tx_info(bool store) { m_store_tx_info = store; }
     uint32_t default_mixin() const { return m_default_mixin; }
     void default_mixin(uint32_t m) { m_default_mixin = m; }
-    uint32_t get_default_priority() const { return m_default_priority; }
+    fee_priority get_default_priority() const { return m_default_priority; }
-    void set_default_priority(uint32_t p) { m_default_priority = p; }
+    void set_default_priority(fee_priority p) { m_default_priority = p; }
     bool auto_refresh() const { return m_auto_refresh; }
     void auto_refresh(bool r) { m_auto_refresh = r; }
     AskPasswordType ask_password() const { return m_ask_password; }
@@ -1539,7 +1541,7 @@ private:
     uint8_t get_current_hard_fork();
     void get_hard_fork_info(uint8_t version, uint64_t &earliest_height);
     bool use_fork_rules(uint8_t version, int64_t early_blocks = 0);
-    int get_fee_algorithm();
+    fee_algorithm get_fee_algorithm();
 
     std::string get_wallet_file() const;
     std::string get_keys_file() const;
@@ -1652,15 +1654,22 @@ private:
     std::vector<std::pair<uint64_t, uint64_t>> estimate_backlog(uint64_t min_tx_weight, uint64_t max_tx_weight, const std::vector<uint64_t> &fees);
 
     static uint64_t estimate_fee(bool use_per_byte_fee, bool use_rct, int n_inputs, int mixin, int n_outputs, size_t extra_size, bool bulletproof, bool clsag, bool bulletproof_plus, bool use_view_tags, uint64_t base_fee, uint64_t fee_quantization_mask);
-    uint64_t get_fee_multiplier(uint32_t priority, int fee_algorithm = -1);
+    uint64_t get_fee_multiplier(fee_priority priority, fee_algorithm fee_algorithm = fee_algorithm::Unset);
-    uint64_t get_base_fee(uint32_t priority);
+    uint64_t get_base_fee(fee_priority priority);
     uint64_t get_base_fee();
     uint64_t get_fee_quantization_mask();
     uint64_t get_min_ring_size();
     uint64_t get_max_ring_size();
     uint64_t adjust_mixin(uint64_t mixin);
 
-    uint32_t adjust_priority(uint32_t priority);
+    fee_priority adjust_priority(fee_priority priority);
+
+    /*
+      The overloads taking in an integer is kept for backwards compatibility, as this is called from wallet.cpp
+      after casting from a type (PendingTransaction::FeePriority) which I don't want to touch.
+    */
+    fee_priority adjust_priority(uint32_t priority);
+    uint64_t get_base_fee(uint32_t);
 
     bool is_unattended() const { return m_unattended; }
 
@@ -1959,7 +1968,7 @@ private:
     bool m_print_ring_members;
     bool m_store_tx_info; /*!< request txkey to be returned in RPC, and store in the wallet cache file */
     uint32_t m_default_mixin;
-    uint32_t m_default_priority;
+    fee_priority m_default_priority;
     RefreshType m_refresh_type;
     bool m_auto_refresh;
     bool m_first_refresh_done;

src/wallet/wallet_rpc_server.cpp

@@ -55,6 +55,7 @@ using namespace epee;
 #include "rpc/rpc_args.h"
 #include "rpc/core_rpc_server_commands_defs.h"
 #include "daemonizer/daemonizer.h"
+#include "fee_priority.h"
 
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "wallet.rpc"
@@ -1170,6 +1171,12 @@ namespace tools
       er.message = "Transaction cannot have non-zero unlock time";
       return false;
     }
+    else if (!fee_priority_utilities::is_valid(req.priority))
+    {
+      er.code = WALLET_RPC_ERROR_CODE_INVALID_FEE_PRIORITY;
+      er.message = "Invalid priority value. Must be between 0 and 4.";
+      return false;
+    }
 
     CHECK_MULTISIG_ENABLED();
 
@@ -1182,7 +1189,7 @@ namespace tools
     try
     {
       uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0);
-      uint32_t priority = m_wallet->adjust_priority(req.priority);
+      const fee_priority priority = m_wallet->adjust_priority(fee_priority_utilities::from_integral(req.priority));
       std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_2(dsts, mixin, priority, extra, req.account_index, req.subaddr_indices, req.subtract_fee_from_outputs);
 
       if (ptx_vector.empty())
@@ -1230,6 +1237,12 @@ namespace tools
       er.message = "Transaction cannot have non-zero unlock time";
       return false;
     }
+    else if (!fee_priority_utilities::is_valid(req.priority))
+    {
+      er.code = WALLET_RPC_ERROR_CODE_INVALID_FEE_PRIORITY;
+      er.message = "Invalid priority value. Must be between 0 and 4.";
+      return false;
+    }
 
     CHECK_MULTISIG_ENABLED();
 
@@ -1242,7 +1255,7 @@ namespace tools
     try
     {
       uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0);
-      uint32_t priority = m_wallet->adjust_priority(req.priority);
+      const fee_priority priority = m_wallet->adjust_priority(fee_priority_utilities::from_integral(req.priority));
       LOG_PRINT_L2("on_transfer_split calling create_transactions_2");
       std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_2(dsts, mixin, priority, extra, req.account_index, req.subaddr_indices);
       LOG_PRINT_L2("on_transfer_split called create_transactions_2");
@@ -1675,6 +1688,12 @@ namespace tools
       er.message = "Transaction cannot have non-zero unlock time";
       return false;
     }
+    else if (!fee_priority_utilities::is_valid(req.priority))
+    {
+      er.code = WALLET_RPC_ERROR_CODE_INVALID_FEE_PRIORITY;
+      er.message = "Invalid priority value. Must be between 0 and 4.";
+      return false;
+    }
 
     CHECK_MULTISIG_ENABLED();
 
@@ -1709,7 +1728,7 @@ namespace tools
     try
     {
       uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0);
-      uint32_t priority = m_wallet->adjust_priority(req.priority);
+      const fee_priority priority = m_wallet->adjust_priority(fee_priority_utilities::from_integral(req.priority));
       std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_all(req.below_amount, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, priority, extra, req.account_index, subaddr_indices);
 
       return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.amounts_by_dest_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay,
@@ -1741,6 +1760,12 @@ namespace tools
       er.message = "Transaction cannot have non-zero unlock time";
       return false;
     }
+    else if (!fee_priority_utilities::is_valid(req.priority))
+    {
+      er.code = WALLET_RPC_ERROR_CODE_INVALID_FEE_PRIORITY;
+      er.message = "Invalid priority value. Must be between 0 and 4.";
+      return false;
+    }
 
     if (req.outputs < 1)
     {
@@ -1772,7 +1797,7 @@ namespace tools
     try
     {
       uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0);
-      uint32_t priority = m_wallet->adjust_priority(req.priority);
+      const fee_priority priority = m_wallet->adjust_priority(fee_priority_utilities::from_integral(req.priority));
       std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_single(ki, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, priority, extra);
 
       if (ptx_vector.empty())
@@ -4785,14 +4810,14 @@ namespace tools
     if (!m_wallet) return not_open(er);
     try
     {
-      uint32_t priority = m_wallet->adjust_priority(0);
+      const fee_priority priority = m_wallet->adjust_priority(fee_priority::Default);
-      if (priority == 0)
+      if (priority == fee_priority::Default)
       {
         er.code = WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR;
         er.message = "Failed to get adjusted fee priority";
         return false;
       }
-      res.priority = priority;
+      res.priority = fee_priority_utilities::as_integral(priority);
     }
     catch (const std::exception& e)
     {

src/wallet/wallet_rpc_server_error_codes.h

@@ -83,3 +83,4 @@
 #define WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME    -50
 #define WALLET_RPC_ERROR_CODE_IS_BACKGROUND_WALLET   -51
 #define WALLET_RPC_ERROR_CODE_IS_BACKGROUND_SYNCING  -52
+#define WALLET_RPC_ERROR_CODE_INVALID_FEE_PRIORITY   -53

tests/functional_tests/transactions_flow_test.cpp

@@ -85,7 +85,7 @@ bool do_send_money(tools::wallet2& w1, tools::wallet2& w2, size_t mix_in_factor,
   try
   {
     std::vector<tools::wallet2::pending_tx> ptx;
-    ptx = w1.create_transactions_2(dsts, mix_in_factor, 0, std::vector<uint8_t>(), 0, {});
+    ptx = w1.create_transactions_2(dsts, mix_in_factor, tools::fee_priority::Default, std::vector<uint8_t>(), 0, {});
     for (auto &p: ptx)
       w1.commit_tx(p);
     return true;