cryptonote_core: ban large miner tx extras after v17 and simplify output order checking

Author: jeffro256

src/cryptonote_core/blockchain.cpp

@@ -1333,11 +1333,12 @@ bool Blockchain::prevalidate_miner_transaction(const block& b, uint64_t height,
 
   CHECK_AND_ASSERT_MES(check_output_types(b.miner_tx, hf_version), false, "miner transaction has invalid output type(s) in block " << get_block_hash(b));
 
-  // from carrot or v18, require output pubkeys be sorted in strictly increasing lexicographical order
-  const bool tx_is_carrot = !b.miner_tx.vout.empty() && b.miner_tx.vout.at(0).target.type() == typeid(txout_to_carrot_v1);
-  const bool should_enforce_sorted_outputs = hf_version > HF_VERSION_FCMP_PLUS_PLUS || tx_is_carrot;
-  if (should_enforce_sorted_outputs) {
-    CHECK_AND_ASSERT_MES(are_transaction_output_pubkeys_sorted(b.miner_tx), false, "miner transaction outputs are not sorted in block " << get_block_hash(b));
+  CHECK_AND_ASSERT_MES(check_transaction_output_pubkeys_order(b.miner_tx, hf_version),
+    false, "FCMP++ miner transaction has unsorted outputs in block " << get_block_hash(b));
+
+  // from v17, require tx.extra size be within limit
+  if (hf_version >= HF_VERSION_REJECT_LARGE_EXTRA) {
+    CHECK_AND_ASSERT_MES(b.miner_tx.extra.size() <= MAX_TX_EXTRA_SIZE, "false", "miner transaction extra too big");
   }
 
   return true;
@@ -3414,16 +3415,6 @@ bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context
     return false;
   }
 
-  // from carrot or v18, require output pubkeys be sorted in strictly increasing lexicographical order
-  const bool tx_is_carrot = !tx.vout.empty() && tx.vout.at(0).target.type() == typeid(txout_to_carrot_v1);
-  const bool should_enforce_sorted_outputs = hf_version > HF_VERSION_FCMP_PLUS_PLUS || tx_is_carrot;
-  if (should_enforce_sorted_outputs) {
-    if (!are_transaction_output_pubkeys_sorted(tx)) {
-      tvc.m_invalid_output = true;
-      return false;
-    }
-  }
-
   return true;
 }
 //------------------------------------------------------------------

src/cryptonote_core/tx_verification_utils.cpp

@@ -28,6 +28,7 @@
 
 #include <boost/iterator/transform_iterator.hpp>
 
+#include "carrot_impl/format_utils.h"
 #include "cryptonote_basic/cryptonote_format_utils.h"
 #include "cryptonote_core/blockchain.h"
 #include "cryptonote_core/cryptonote_core.h"
@@ -284,6 +285,14 @@ static bool ver_non_input_consensus_templated(TxForwardIt tx_begin,
             return false;
         }
 
+        // Rule 10
+        if (!check_transaction_output_pubkeys_order(tx, hf_version))
+        {
+            tvc.m_verifivation_failed = true;
+            tvc.m_invalid_output = true;
+            return false;
+        }
+
         // Rule 5
         if (!core::check_tx_semantic(tx, tvc, hf_version))
             return false;
@@ -564,10 +573,10 @@ uint64_t get_transaction_weight_limit(const uint8_t hf_version)
         return get_min_block_weight(hf_version) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE;
 }
 
-bool are_transaction_output_pubkeys_sorted(const transaction_prefix &tx_prefix)
+bool are_transaction_output_pubkeys_sorted(const std::vector<tx_out> &vout)
 {
     crypto::public_key last_output_pubkey = crypto::null_pkey;
-    for (const tx_out &o : tx_prefix.vout) {
+    for (const tx_out &o : vout) {
       crypto::public_key output_pubkey;
       if (!get_output_public_key(o, output_pubkey)) {
         return false;
@@ -581,6 +590,13 @@ bool are_transaction_output_pubkeys_sorted(const transaction_prefix &tx_prefix)
     return true;
 }
 
+bool check_transaction_output_pubkeys_order(const transaction_prefix &tx_prefix, const std::uint8_t hf_version)
+{
+    if (hf_version > HF_VERSION_FCMP_PLUS_PLUS || carrot::is_carrot_transaction_v1(tx_prefix))
+        return are_transaction_output_pubkeys_sorted(tx_prefix.vout);
+    return true;
+}
+
 size_t get_minimum_transaction_version(uint8_t hf_version, bool has_unmixable_ring)
 {
     if (hf_version >= HF_VERSION_REJECT_UNMIXABLE_V1)

src/cryptonote_core/tx_verification_utils.h

@@ -78,7 +78,17 @@ uint64_t get_transaction_weight_limit(uint8_t hf_version);
 /**
  * @brief Check whether transaction's output pubkeys are sorted in strictly increasing lexicographical order
  */
-bool are_transaction_output_pubkeys_sorted(const transaction_prefix &tx_prefix);
+bool are_transaction_output_pubkeys_sorted(const std::vector<tx_out> &vout);
+
+/**
+ * @brief Check whether transaction's output pubkeys are sorted, iff required by fork rule
+ * @param tx_prefix
+ * @param hf_version hard fork version
+ * @see are_transaction_output_pubkeys_sorted()
+ *
+ * Output pubkeys must be sorted after FCMP++ grace period or if a Carrot tx during FCMP++ grace period.
+ */
+bool check_transaction_output_pubkeys_order(const transaction_prefix &tx_prefix, std::uint8_t hf_version);
 
 /**
  * @brief Get the minimum allowed transaction version
@@ -189,6 +199,7 @@ bool batch_ver_fcmp_pp_consensus
  *     7. Passes ver_mixed_rct_semantics() [Uses batch RingCT verification when applicable]
  *     8. Check unlock time is 0 from hardfork v17
  *     9. Check extra size <= MAX_TX_EXTRA_SIZE from hardfork v17
+ *     10. Passes check_transaction_output_pubkeys_order()
  *
  * For pool_supplement input:
  * We assume the structure of the pool supplement is already correct: for each value entry, the