From 463a9db59d7fcfbe195990b581c51bc0abef3a9d Mon Sep 17 00:00:00 2001 From: j-berman Date: Thu, 20 Aug 2026 17:57:03 -0700 Subject: [PATCH] FCMP++: cleaner/safer get_outs_by_last_locked_block --- .../blockchain_import.cpp | 6 +- .../cryptonote_format_utils.cpp | 58 ++++++++++++++----- .../cryptonote_format_utils.h | 10 +++- src/cryptonote_core/blockchain.cpp | 15 +++-- src/cryptonote_core/blockchain.h | 4 +- src/cryptonote_core/tx_verification_utils.cpp | 56 +++++++++--------- src/cryptonote_core/tx_verification_utils.h | 6 +- src/wallet/wallet2.cpp | 4 +- tests/core_tests/fcmp_pp.cpp | 4 +- 9 files changed, 98 insertions(+), 65 deletions(-) diff --git a/src/blockchain_utilities/blockchain_import.cpp b/src/blockchain_utilities/blockchain_import.cpp index b00e407b56b..24f59a0499a 100644 --- a/src/blockchain_utilities/blockchain_import.cpp +++ b/src/blockchain_utilities/blockchain_import.cpp @@ -490,13 +490,15 @@ int import_from_file(cryptonote::core& core, const std::string& import_file_path const uint64_t first_unified_id = core.get_blockchain_storage().get_db().num_outputs(); std::unordered_map transparent_amount_commitments; - const auto tx_refs = cryptonote::collect_transparent_amount_commitments(b.miner_tx, txs, transparent_amount_commitments); + cryptonote::collect_transparent_amount_commitments(b.miner_tx, txs, transparent_amount_commitments); + OutsByLastLockedBlockMeta new_locked_outs = cryptonote::get_outs_by_last_locked_block(b.miner_tx, txs, transparent_amount_commitments, first_unified_id, h); try { uint64_t long_term_block_weight = core.get_blockchain_storage().get_next_long_term_block_weight(block_weight); const uint64_t new_height = core.get_blockchain_storage().get_db().add_block(std::make_pair(b, block_to_blob(b)), block_weight, long_term_block_weight, cumulative_difficulty, coins_generated, txs, transparent_amount_commitments); - cryptonote::handle_fcmp_tree(&core.get_blockchain_storage().get_db(), new_height - 1, first_unified_id, tx_refs, transparent_amount_commitments); + CHECK_AND_ASSERT_THROW_MES(h == (new_height-1), "Unexpected height"); + cryptonote::handle_fcmp_tree(&core.get_blockchain_storage().get_db(), new_height-1, first_unified_id, transparent_amount_commitments, std::move(new_locked_outs)); } catch (const std::exception& e) { diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index a0e04dca140..7bc67c0081f 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -156,6 +156,29 @@ namespace cryptonote return tx.vout.size(); } //--------------------------------------------------------------- + static OutsByLastLockedBlockMeta get_outs_by_last_locked_block( + const std::vector> &txs, + const std::unordered_map &transparent_amount_commitments, + const uint64_t first_unified_id, + const uint64_t block_idx) + { + OutsByLastLockedBlockMeta outs; + outs.next_unified_id = first_unified_id; + + for (const auto &tx : txs) + { + outs.next_unified_id += set_tx_outs_by_last_locked_block( + tx.get(), + transparent_amount_commitments, + outs.next_unified_id, + block_idx, + outs.outs_by_last_locked_block, + outs.timelocked_outputs); + } + + return outs; + } + //--------------------------------------------------------------- } namespace cryptonote @@ -1986,26 +2009,33 @@ namespace cryptonote } //--------------------------------------------------------------- OutsByLastLockedBlockMeta get_outs_by_last_locked_block( - const std::vector> &txs, + const cryptonote::transaction &miner_tx, + const std::vector &txs, const std::unordered_map &transparent_amount_commitments, const uint64_t first_unified_id, const uint64_t block_idx) { - OutsByLastLockedBlockMeta outs_by_last_locked_block_meta_out; - outs_by_last_locked_block_meta_out.next_unified_id = first_unified_id; - + std::vector> tx_refs; + tx_refs.reserve(1 + txs.size()); + tx_refs.push_back(std::cref(miner_tx)); for (const auto &tx : txs) - { - outs_by_last_locked_block_meta_out.next_unified_id += set_tx_outs_by_last_locked_block( - tx.get(), - transparent_amount_commitments, - outs_by_last_locked_block_meta_out.next_unified_id, - block_idx, - outs_by_last_locked_block_meta_out.outs_by_last_locked_block, - outs_by_last_locked_block_meta_out.timelocked_outputs); - } + tx_refs.push_back(std::cref(tx)); + return get_outs_by_last_locked_block(tx_refs, transparent_amount_commitments, first_unified_id, block_idx); + } - return outs_by_last_locked_block_meta_out; + OutsByLastLockedBlockMeta get_outs_by_last_locked_block( + const cryptonote::transaction &miner_tx, + const std::vector> &tx_pairs, + const std::unordered_map &transparent_amount_commitments, + const uint64_t first_unified_id, + const uint64_t block_idx) + { + std::vector> tx_refs; + tx_refs.reserve(1 + tx_pairs.size()); + tx_refs.push_back(std::cref(miner_tx)); + for (const auto &tx : tx_pairs) + tx_refs.push_back(std::cref(tx.first)); + return get_outs_by_last_locked_block(tx_refs, transparent_amount_commitments, first_unified_id, block_idx); } //--------------------------------------------------------------- fcmp_pp::OutputPair to_output_pair(const cryptonote::txout_target_v &tx_out, const rct::key &commitment) diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index b4956bb54a3..b4fcfc12cfc 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -295,7 +295,15 @@ namespace cryptonote }; OutsByLastLockedBlockMeta get_outs_by_last_locked_block( - const std::vector> &txs, + const cryptonote::transaction &miner_tx, + const std::vector &txs, + const std::unordered_map &transparent_amount_commitments, + const uint64_t first_unified_id, + const uint64_t block_idx); + + OutsByLastLockedBlockMeta get_outs_by_last_locked_block( + const cryptonote::transaction &miner_tx, + const std::vector> &tx_pairs, const std::unordered_map &transparent_amount_commitments, const uint64_t first_unified_id, const uint64_t block_idx); diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 7082acd054b..ad8691ac18c 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2846,11 +2846,8 @@ static bool batch_verify_fcmp_pp_txs(const BlockchainDB *db, return true; } //------------------------------------------------------------------ -void cryptonote::handle_fcmp_tree(BlockchainDB *db, const uint64_t block_idx, const uint64_t first_unified_id, const std::vector> &tx_refs, const std::unordered_map &transparent_amount_commitments) +void cryptonote::handle_fcmp_tree(BlockchainDB *db, const uint64_t block_idx, const uint64_t first_unified_id, const std::unordered_map &transparent_amount_commitments, OutsByLastLockedBlockMeta &&new_locked_outs) { - // Collect outs by last locked block to add to the db - OutsByLastLockedBlockMeta new_locked_outs = cryptonote::get_outs_by_last_locked_block(tx_refs, transparent_amount_commitments, first_unified_id, block_idx); - // Get the outputs with default last locked block const uint64_t default_last_locked_block = cryptonote::get_default_last_locked_block_index(block_idx); auto new_default_locked_outs_it = new_locked_outs.outs_by_last_locked_block.find(default_last_locked_block); @@ -4856,7 +4853,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& TIME_MEASURE_START(tac); // Collect all remaining transparent amount commitments - const auto tx_refs = collect_transparent_amount_commitments(bl.miner_tx, txs, transparent_amount_commitments); + collect_transparent_amount_commitments(bl.miner_tx, txs, transparent_amount_commitments); TIME_MEASURE_FINISH(tac); @@ -4890,7 +4887,9 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& if(precomputed) block_processing_time += m_fake_pow_calc_time; + // Collect new locked outputs we're adding to the db const uint64_t first_unified_id = m_db->num_outputs(); + OutsByLastLockedBlockMeta new_locked_outs = cryptonote::get_outs_by_last_locked_block(bl.miner_tx, txs, transparent_amount_commitments, first_unified_id, blockchain_height); rtxn_guard.stop(); TIME_MEASURE_START(addblock); @@ -4926,9 +4925,9 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& LOG_ERROR("Blocks that failed verification should not reach here"); } - if (new_height == 0) + if (new_height == 0 || (new_height-1) != blockchain_height) { - LOG_ERROR("handle_block_to_main_chain: unexpected new_height == 0"); + LOG_ERROR("handle_block_to_main_chain: unexpected new_height: " << new_height << " , expected: " << blockchain_height+1); bvc.m_verifivation_failed = true; return false; } @@ -4945,7 +4944,7 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash& TIME_MEASURE_START(advance_tree); - try { handle_fcmp_tree(m_db, new_height-1, first_unified_id, tx_refs, transparent_amount_commitments); } + try { handle_fcmp_tree(m_db, new_height-1, first_unified_id, transparent_amount_commitments, std::move(new_locked_outs)); } catch (const std::exception& e) { LOG_ERROR("Failed to advance tree at block with hash: " << id << ", what = " << e.what()); diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index e7b8d892da7..2ea52f4b32b 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -75,8 +75,8 @@ namespace cryptonote void handle_fcmp_tree(BlockchainDB *db, uint64_t block_idx, uint64_t first_unified_id, - const std::vector> &tx_refs, - const std::unordered_map &transparent_amount_commitments); + const std::unordered_map &transparent_amount_commitments, + OutsByLastLockedBlockMeta &&new_locked_outs); /** Declares ways in which the BlockchainDB backend should be told to sync * diff --git a/src/cryptonote_core/tx_verification_utils.cpp b/src/cryptonote_core/tx_verification_utils.cpp index a16969f2ea3..53669cb58f5 100644 --- a/src/cryptonote_core/tx_verification_utils.cpp +++ b/src/cryptonote_core/tx_verification_utils.cpp @@ -472,6 +472,28 @@ static bool collect_fcmp_pp_tx_verify_input(cryptonote::transaction &tx, return true; } +static void collect_transparent_amount_commitments_static( + const std::vector> &tx_refs, + std::unordered_map &transparent_amount_commitments_inout) +{ + // Note: we do not clear transparent_amount_commitments_inout because it may be a rolling cache + + for (const auto &tx_ref : tx_refs) + { + const auto &tx = tx_ref.get(); + + // We only need commitments for transparent amounts, which are tx version 1 || coinbase txs + if (tx.version > 1 && !cryptonote::is_coinbase(tx)) + continue; + for (const auto &tx_out : tx.vout) + { + const uint64_t amount = tx_out.amount; + if (transparent_amount_commitments_inout.find(amount) == transparent_amount_commitments_inout.end()) + transparent_amount_commitments_inout[amount] = rct::zeroCommitVartime(amount); + } + } +} + //////////////////////////////////////////////////////////////////////////////////////////////////// namespace cryptonote @@ -504,28 +526,6 @@ bool collect_points_for_torsion_check(const transaction& tx, } void collect_transparent_amount_commitments( - const std::vector> &txs, - std::unordered_map &transparent_amount_commitments_inout) -{ - // Note: we do not clear transparent_amount_commitments_inout because it may be a rolling cache - - for (const auto &tx_ref : txs) - { - const auto &tx = tx_ref.get(); - - // We only need commitments for transparent amounts, which are tx version 1 || coinbase txs - if (tx.version > 1 && !cryptonote::is_coinbase(tx)) - continue; - for (const auto &tx_out : tx.vout) - { - const uint64_t amount = tx_out.amount; - if (transparent_amount_commitments_inout.find(amount) == transparent_amount_commitments_inout.end()) - transparent_amount_commitments_inout[amount] = rct::zeroCommitVartime(amount); - } - } -} - -std::vector> collect_transparent_amount_commitments( const transaction &miner_tx, const std::vector> &tx_pairs, std::unordered_map &transparent_amount_commitments_inout) @@ -535,11 +535,10 @@ std::vector> collect_transparent_amoun tx_refs.push_back(std::cref(miner_tx)); for (const auto &tx : tx_pairs) tx_refs.push_back(std::cref(tx.first)); - collect_transparent_amount_commitments(tx_refs, transparent_amount_commitments_inout); - return tx_refs; + collect_transparent_amount_commitments_static(tx_refs, transparent_amount_commitments_inout); } -std::vector> collect_transparent_amount_commitments( +void collect_transparent_amount_commitments( const transaction &miner_tx, const std::vector &txs, std::unordered_map &transparent_amount_commitments_inout) @@ -549,8 +548,7 @@ std::vector> collect_transparent_amoun tx_refs.push_back(std::cref(miner_tx)); for (const auto &tx : txs) tx_refs.push_back(std::cref(tx)); - collect_transparent_amount_commitments(tx_refs, transparent_amount_commitments_inout); - return tx_refs; + collect_transparent_amount_commitments_static(tx_refs, transparent_amount_commitments_inout); } void collect_transparent_amount_commitments( @@ -561,7 +559,7 @@ void collect_transparent_amount_commitments( tx_refs.reserve(txs_by_txid.size()); for (const auto &tx_pair : txs_by_txid) tx_refs.push_back(std::cref(tx_pair.second.first)); - collect_transparent_amount_commitments(tx_refs, transparent_amount_commitments_inout); + collect_transparent_amount_commitments_static(tx_refs, transparent_amount_commitments_inout); } uint64_t get_non_coinbase_tx_weight_limit(const uint8_t hf_version) @@ -925,7 +923,7 @@ bool ver_non_input_consensus(const transaction& tx, tx_verification_context& tvc { // Get tx's transparent amount commitments std::unordered_map transparent_amount_commitments; - collect_transparent_amount_commitments({std::cref(tx)}, transparent_amount_commitments); + collect_transparent_amount_commitments_static({std::cref(tx)}, transparent_amount_commitments); return ver_non_input_consensus_templated(&tx, &tx + 1, transparent_amount_commitments, tvc, hf_version); } diff --git a/src/cryptonote_core/tx_verification_utils.h b/src/cryptonote_core/tx_verification_utils.h index f515a42448f..c9fc28c3230 100644 --- a/src/cryptonote_core/tx_verification_utils.h +++ b/src/cryptonote_core/tx_verification_utils.h @@ -49,15 +49,11 @@ bool collect_points_for_torsion_check(const transaction& tx, std::vector &pubkeys_and_commitments_inout); void collect_transparent_amount_commitments( - const std::vector> &txs, - std::unordered_map &transparent_amount_commitments_inout); - -std::vector> collect_transparent_amount_commitments( const transaction &miner_tx, const std::vector> &tx_pairs, std::unordered_map &transparent_amount_commitments_inout); -std::vector> collect_transparent_amount_commitments( +void collect_transparent_amount_commitments( const transaction &miner_tx, const std::vector &txs, std::unordered_map &transparent_amount_commitments_inout); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index e818afe530b..1544d421eeb 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -3205,8 +3205,8 @@ static void prepare_tree_state_change(const TreeSyncStartParams &tree_sync_start new_block_hashes_out.push_back(parsed_blocks[i].block.hash); // Slow: collect transparent amount commitments - const auto tx_refs = cryptonote::collect_transparent_amount_commitments(parsed_blocks[i].block.miner_tx, parsed_blocks[i].txes, transparent_amount_commitments); - auto res = cryptonote::get_outs_by_last_locked_block(tx_refs, transparent_amount_commitments, first_unified_id, created_block_idx); + cryptonote::collect_transparent_amount_commitments(parsed_blocks[i].block.miner_tx, parsed_blocks[i].txes, transparent_amount_commitments); + auto res = cryptonote::get_outs_by_last_locked_block(parsed_blocks[i].block.miner_tx, parsed_blocks[i].txes, transparent_amount_commitments, first_unified_id, created_block_idx); outs_by_last_locked_blocks.emplace_back(std::move(res.outs_by_last_locked_block)); first_unified_id = res.next_unified_id; diff --git a/tests/core_tests/fcmp_pp.cpp b/tests/core_tests/fcmp_pp.cpp index dea446c011a..dbde8aa1aed 100644 --- a/tests/core_tests/fcmp_pp.cpp +++ b/tests/core_tests/fcmp_pp.cpp @@ -323,8 +323,8 @@ bool gen_fcmp_pp_tx_validation_base::generate_with(std::vector blk_idx == pre_rct_tx_block_idx ? std::vector{pre_rct_tx} : blk_idx == bpp_block_idx ? std::vector{bpp_tx} : std::vector{}; - const auto tx_refs = cryptonote::collect_transparent_amount_commitments(blk.miner_tx, txs, transparent_amount_commitments); - auto outs_meta = cryptonote::get_outs_by_last_locked_block(tx_refs, transparent_amount_commitments, first_unified_id, blk_idx); + cryptonote::collect_transparent_amount_commitments(blk.miner_tx, txs, transparent_amount_commitments); + auto outs_meta = cryptonote::get_outs_by_last_locked_block(blk.miner_tx, txs, transparent_amount_commitments, first_unified_id, blk_idx); outs_by_last_locked_blocks.emplace_back(std::move(outs_meta.outs_by_last_locked_block)); first_unified_id = outs_meta.next_unified_id; }