From 4eecccee046dca933918a14aa4fc29750a94da07 Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Thu, 3 Apr 2025 13:54:05 -0500 Subject: [PATCH] carrot_impl: add consensus rule for unique output pubkeys in tx Required by Carrot to mitigate burning bugs, described in section 4.3 of the Carrot spec: https://github.com/jeffro256/carrot/blob/master/carrot.md#43-transaction-model Also remove 0-out check in `check_output_types()`, which I added in and technically constitutes a retroactive network split. Co-authored-by: j-berman --- .../cryptonote_format_utils.cpp | 19 +----------- src/cryptonote_core/blockchain.cpp | 17 +++++++++++ src/cryptonote_core/tx_pool.cpp | 9 ------ src/cryptonote_core/tx_verification_utils.cpp | 29 +++++++++++++++++++ src/cryptonote_core/tx_verification_utils.h | 13 +++++++++ 5 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index c7e696f7d..6978ab0d4 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -1193,31 +1193,14 @@ namespace cryptonote if (tx.type == cryptonote::transaction_type::AUDIT || tx.type == cryptonote::transaction_type::STAKE) { CHECK_AND_ASSERT_MES(tx.vout.size() == 1, false, "audit and stake transactions should have 1 output"); } - CHECK_AND_ASSERT_MES(tx.vout.size() > 0, false, "no outputs in transaction"); for (const auto &o: tx.vout) { - if (hf_version > HF_VERSION_CARROT) + if (hf_version >= HF_VERSION_CARROT) { // from v18, require outputs be carrot outputs CHECK_AND_ASSERT_MES(o.target.type() == typeid(txout_to_carrot_v1), false, "wrong variant type: " << o.target.type().name() << ", expected txout_to_carrot_v1 in transaction id=" << get_transaction_hash(tx)); - } - else if (hf_version == HF_VERSION_CARROT) - { - // during v17, require outputs be of type txout_to_tagged_key OR txout_to_carrot_v1 - // to allow grace period before requiring all to be txout_to_carrot_v1 - CHECK_AND_ASSERT_MES( - o.target.type() == typeid(txout_to_carrot_v1) || - o.target.type() == typeid(txout_to_tagged_key) || - o.target.type() == typeid(txout_to_key), - false, "wrong variant type: " << o.target.type().name() - << ", expected txout_to_key or txout_to_tagged_key in transaction id=" << get_transaction_hash(tx)); - - // require all outputs in a tx be of the same type - CHECK_AND_ASSERT_MES(o.target.type() == tx.vout[0].target.type(), false, "non-matching variant types: " - << o.target.type().name() << " and " << tx.vout[0].target.type().name() << ", " - << "expected matching variant types in transaction id=" << get_transaction_hash(tx)); } else { // require outputs be of type txout_to_key OR txout_to_tagged_key // to allow grace period before requiring all to be txout_to_tagged_key diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index bd5aa1273..2f272130d 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -1370,6 +1370,13 @@ 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_CARROT || 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)); + } + return true; } //------------------------------------------------------------------ @@ -3691,6 +3698,16 @@ bool Blockchain::check_tx_type_and_version(const transaction& tx, tx_verificatio 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_CARROT || tx_is_carrot; + if (should_enforce_sorted_outputs) { + if (!are_transaction_output_pubkeys_sorted(tx)) { + tvc.m_invalid_output = true; + return false; + } + } + return true; } //------------------------------------------------------------------ diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index b32db4c1b..46b6edcd7 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -110,15 +110,6 @@ namespace cryptonote return amount * ACCEPT_THRESHOLD; } - uint64_t get_transaction_weight_limit(uint8_t version) - { - // from v2, limit a tx to 50% of the minimum block weight - if (version >= 2) - return get_min_block_weight(version) / 2 - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; - else - return get_min_block_weight(version) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; - } - // external lock must be held for the comparison+set to work properly void set_if_less(std::atomic& next_check, const time_t candidate) noexcept { diff --git a/src/cryptonote_core/tx_verification_utils.cpp b/src/cryptonote_core/tx_verification_utils.cpp index 6961a5f0b..15b14c28f 100644 --- a/src/cryptonote_core/tx_verification_utils.cpp +++ b/src/cryptonote_core/tx_verification_utils.cpp @@ -26,6 +26,9 @@ // STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF // THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#include + +#include "cryptonote_basic/cryptonote_format_utils.h" #include "cryptonote_core/blockchain.h" #include "cryptonote_core/tx_verification_utils.h" #include "ringct/rctSigs.h" @@ -110,6 +113,32 @@ static crypto::hash calc_tx_mixring_hash(const transaction& tx, const rct::ctkey namespace cryptonote { +uint64_t get_transaction_weight_limit(const uint8_t hf_version) +{ + // from v2, limit a tx to 50% of the minimum block weight + if (hf_version >= 2) + return get_min_block_weight(hf_version) / 2 - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; + else + return get_min_block_weight(hf_version) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; +} + +bool are_transaction_output_pubkeys_sorted(const transaction_prefix &tx_prefix) +{ + crypto::public_key last_output_pubkey = crypto::null_pkey; + for (const tx_out &o : tx_prefix.vout) { + crypto::public_key output_pubkey; + if (!get_output_public_key(o, output_pubkey)) { + return false; + } + else if (!(output_pubkey > last_output_pubkey)) { + return false; + } + last_output_pubkey = output_pubkey; + } + + return true; +} + bool ver_rct_non_semantics_simple_cached ( transaction& tx, diff --git a/src/cryptonote_core/tx_verification_utils.h b/src/cryptonote_core/tx_verification_utils.h index c3649f0a8..d4ff0f9cd 100644 --- a/src/cryptonote_core/tx_verification_utils.h +++ b/src/cryptonote_core/tx_verification_utils.h @@ -34,6 +34,19 @@ namespace cryptonote { +/** + * @brief Get the maximum transaction weight for a given hardfork + * + * @param hf_version hard fork version + * @return the maximum unconditional transaction weight + */ +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); + // Modifying this value should not affect consensus. You can adjust it for performance needs static constexpr const size_t RCT_VER_CACHE_SIZE = 8192;