From b055eb3f61e8e51858125b7f17e914df23211893 Mon Sep 17 00:00:00 2001 From: j-berman Date: Tue, 10 Sep 2024 18:11:06 -0700 Subject: [PATCH] fcmp++: output may not be present in locked outputs table on remove - If the output is invalid/unspendable, upon unlock it will be deleted from the locked outputs table and then won't be used to grow the tree. Upon reorg/pop blocks, the invalid output won't be re-added to the locked outputs table upon trimming the tree. Thus, it's possible for an invalid/unspendable output to not be present in the locked outputs table upon remove. --- src/blockchain_db/lmdb/db_lmdb.cpp | 26 +++++++++++++++++--------- src/fcmp_pp/curve_trees.cpp | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index ac16cb341..608d6d998 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -1259,10 +1259,16 @@ void BlockchainLMDB::remove_output(const uint64_t amount, const uint64_t& out_in throw1(DB_ERROR(lmdb_error("Error adding removal of output tx to db transaction", result).c_str())); } - // Remove output from locked outputs table. We expect the output to be in the - // locked outputs table because remove_output is called when removing the - // top block from the chain, and all outputs from the top block are expected - // to be locked until they are at least 10 blocks old (10 is the lower bound). + // Remove output from locked outputs table if present. We expect all valid + // outputs to be in the locked outputs table because remove_output is called + // when removing the top block from the chain, and all outputs from the top + // block are expected to be locked until they are at least 10 blocks old (10 + // is the lower bound). An output might not be in the locked outputs table if + // it is invalid, then gets removed from the locked outputs table upon growing + // the tree. + // TODO: test case where we add an invalid output to the chain, grow the tree + // in the block in which that output unlocks, pop blocks to remove that output + // from the chain, then progress the chain again. CURSOR(locked_outputs); const uint64_t unlock_block = cryptonote::get_unlock_block_index(ok->data.unlock_time, ok->data.height); @@ -1273,16 +1279,18 @@ void BlockchainLMDB::remove_output(const uint64_t amount, const uint64_t& out_in result = mdb_cursor_get(m_cur_locked_outputs, &k_block_id, &v_output, MDB_GET_BOTH); if (result == MDB_NOTFOUND) { - throw0(DB_ERROR("Unexpected: output not found in m_cur_locked_outputs")); + // We expect this output is invalid } else if (result) { throw1(DB_ERROR(lmdb_error("Error adding removal of locked output to db transaction", result).c_str())); } - - result = mdb_cursor_del(m_cur_locked_outputs, 0); - if (result) - throw0(DB_ERROR(lmdb_error(std::string("Error deleting locked output index ").append(boost::lexical_cast(out_index).append(": ")).c_str(), result).c_str())); + else + { + result = mdb_cursor_del(m_cur_locked_outputs, 0); + if (result) + throw0(DB_ERROR(lmdb_error(std::string("Error deleting locked output index ").append(boost::lexical_cast(out_index).append(": ")).c_str(), result).c_str())); + } result = mdb_cursor_del(m_cur_output_txs, 0); if (result) diff --git a/src/fcmp_pp/curve_trees.cpp b/src/fcmp_pp/curve_trees.cpp index 634f3f8c2..a1640b5e7 100644 --- a/src/fcmp_pp/curve_trees.cpp +++ b/src/fcmp_pp/curve_trees.cpp @@ -191,7 +191,7 @@ static LayerExtension hash_children_chunks(const std::unique_ptr &curve, // See how many children we need to fill up the existing last chunk std::size_t chunk_size = std::min(new_child_scalars.size(), chunk_width - start_offset); - CHECK_AND_ASSERT_THROW_MES(new_child_scalars.size() >= chunk_size, "unexpected size of new child scalars"); + CHECK_AND_ASSERT_THROW_MES(new_child_scalars.size() >= chunk_size, "unexpected first chunk size"); const std::size_t n_chunks = 1 // first chunk + (new_child_scalars.size() - chunk_size) / chunk_width // middle chunks