diff --git a/plugins/trace_api_plugin/store_provider.cpp b/plugins/trace_api_plugin/store_provider.cpp index ae42e843da..a924180c59 100644 --- a/plugins/trace_api_plugin/store_provider.cpp +++ b/plugins/trace_api_plugin/store_provider.cpp @@ -107,8 +107,7 @@ namespace eosio::trace_api { slice_number = 0; } - uint32_t trx_block_num = 0; // number of the block that contains the target trx - uint32_t trx_entries = 0; // number of entries that contain the target trx + std::set trx_block_nums; while (true){ const bool found = _slice_directory.find_trx_id_slice(slice_number, open_state::read, trx_id_file); if( !found ) @@ -123,16 +122,21 @@ namespace eosio::trace_api { fc::raw::unpack(ds, entry); if (std::holds_alternative(entry)) { const auto& trxs_entry = std::get(entry); + bool found_in_block = false; for (auto i = 0U; i < trxs_entry.ids.size(); ++i) { if (trxs_entry.ids[i] == trx_id) { - trx_entries++; - trx_block_num = trxs_entry.block_num; + trx_block_nums.insert(trxs_entry.block_num); + found_in_block = true; + break; } } + // block can be seen again when a fork happens, if not in the new block remove it from blocks that have the trx + if (!found_in_block) + trx_block_nums.erase(trxs_entry.block_num); } else if (std::holds_alternative(entry)) { auto lib = std::get(entry).lib; - if (trx_entries > 0 && lib >= trx_block_num) { - return trx_block_num; + if (!trx_block_nums.empty() && lib >= *(--trx_block_nums.end())) { + return *(--trx_block_nums.end()); // *(--trx_block_nums.end()) is the block with highest block number which is final } } else { FC_ASSERT( false, "unpacked data should be a block_trxs_entry or a lib_entry_v0" );; @@ -143,8 +147,8 @@ namespace eosio::trace_api { } // transaction's block is not irreversible - if (trx_entries > 0) - return trx_block_num; + if (!trx_block_nums.empty()) + return *(--trx_block_nums.end()); return get_block_n{}; } diff --git a/plugins/trace_api_plugin/test/test_trace_file.cpp b/plugins/trace_api_plugin/test/test_trace_file.cpp index 99fbafa598..64f3690916 100644 --- a/plugins/trace_api_plugin/test/test_trace_file.cpp +++ b/plugins/trace_api_plugin/test/test_trace_file.cpp @@ -1066,5 +1066,234 @@ BOOST_AUTO_TEST_SUITE(slice_tests) BOOST_REQUIRE(!block2); } +// Verify basics of get_trx_block_number() + BOOST_FIXTURE_TEST_CASE(test_get_trx_block_number_basic, test_fixture) + { + chain::transaction_id_type trx_id1 = "0000000000000000000000000000000000000000000000000000000000000001"_h; + chain::transaction_id_type trx_id2 = "0000000000000000000000000000000000000000000000000000000000000002"_h; + uint32_t block_num1 = 1; + uint32_t block_num2 = 2; + + transaction_trace_v2 trx_trace1 { + trx_id1, + actions, + fc::enum_type{chain::transaction_receipt_header::status_enum::executed}, + 10, + 5, + { chain::signature_type() }, + { chain::time_point_sec(), 1, 0, 100, 50, 0 } + }; + + transaction_trace_v2 trx_trace2 { + trx_id2, + actions, + fc::enum_type{chain::transaction_receipt_header::status_enum::executed}, + 10, + 5, + { chain::signature_type() }, + { chain::time_point_sec(), 1, 0, 100, 50, 0 } + }; + + // block 1 includes trx_trace1 + block_trace_v2 block_trace1 { + "b000000000000000000000000000000000000000000000000000000000000001"_h, + block_num1, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + chain::block_timestamp_type(0), + "test"_n, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + 0, + std::vector { + trx_trace1 + } + }; + + // block 2 includes trx_trace2 + block_trace_v2 block_trace2 { + "b000000000000000000000000000000000000000000000000000000000000003"_h, + block_num2, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + chain::block_timestamp_type(0), + "test"_n, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + 0, + std::vector { + trx_trace2 + } + }; + + block_trxs_entry block_trxs_entry1 { + .ids = {trx_id1}, + .block_num = block_num1 + }; + + block_trxs_entry block_trxs_entry2 { + .ids = {trx_id2}, + .block_num = block_num2 + }; + + fc::temp_directory tempdir; + store_provider sp(tempdir.path(), 100, std::optional(), std::optional(), 0); + + // on_accepted_block of block 1 + sp.append(block_trace1); + sp.append_trx_ids(block_trxs_entry1); + + // block 1 is reversible and get_trx_block_number should find trx_id1 in block 1 + get_block_n block_num = sp.get_trx_block_number(trx_id1, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num1); + + // block 1 becomes final + sp.append_lib(block_num1); + + // get_trx_block_number should find trx_id1 in block 1 + block_num = sp.get_trx_block_number(trx_id1, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num1); + + // on_accepted_block of block 2 + sp.append(block_trace2); + sp.append_trx_ids(block_trxs_entry2); + + // get_trx_block_number should find both trx_id1 and trx_id2 + block_num = sp.get_trx_block_number(trx_id1, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num1); + block_num = sp.get_trx_block_number(trx_id2, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num2); + + // block 2 becomes final + sp.append_lib(block_num2); + + // get_trx_block_number should still find both trx_id1 and trx_id2 + block_num = sp.get_trx_block_number(trx_id1, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num1); + block_num = sp.get_trx_block_number(trx_id2, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, block_num2); + } + +// This test verifies the bug reported by https://github.com/AntelopeIO/spring/issues/942 +// is fixed. The bug was if the block containing a transaction forked out, +// get_trx_block_number() always returned the latest block whose block number was +// the same as the initial block's, but this latest block might not include the +// transaction anymore. + BOOST_FIXTURE_TEST_CASE(test_get_trx_block_number_forked, test_fixture) + { + chain::transaction_id_type target_trx_id = "0000000000000000000000000000000000000000000000000000000000000001"_h; + uint32_t initial_block_num = 1; + uint32_t final_block_num = 3; + + transaction_trace_v2 trx_trace1 { + target_trx_id, + actions, + fc::enum_type{chain::transaction_receipt_header::status_enum::executed}, + 10, + 5, + { chain::signature_type() }, + { chain::time_point_sec(), 1, 0, 100, 50, 0 } + }; + + transaction_trace_v2 trx_trace2 { + "0000000000000000000000000000000000000000000000000000000000000002"_h, + actions, + fc::enum_type{chain::transaction_receipt_header::status_enum::executed}, + 10, + 5, + { chain::signature_type() }, + { chain::time_point_sec(), 1, 0, 100, 50, 0 } + }; + + // Initial block including trx_trace1 + block_trace_v2 initial_block_trace { + "b000000000000000000000000000000000000000000000000000000000000001"_h, + initial_block_num, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + chain::block_timestamp_type(0), + "test"_n, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + 0, + std::vector { + trx_trace1 + } + }; + + // Initial block is forked. The original trx_trace1 is forked out and + // replaced by trx_trace2. + block_trace_v2 forked_block_trace = initial_block_trace; + forked_block_trace.transactions = std::vector { trx_trace2 }; + + // Final block including original trx_trace1 + block_trace_v2 final_block_trace { + "b000000000000000000000000000000000000000000000000000000000000003"_h, + final_block_num, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + chain::block_timestamp_type(0), + "test"_n, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + "0000000000000000000000000000000000000000000000000000000000000000"_h, + 0, + std::vector { + trx_trace1 + } + }; + + block_trxs_entry initial_block_trxs_entry { + .ids = {target_trx_id}, + .block_num = initial_block_num + }; + + block_trxs_entry forked_block_trxs_entry { + .ids = {trx_trace2.id}, + .block_num = initial_block_num + }; + + block_trxs_entry final_block_trxs_entry { + .ids = {target_trx_id}, + .block_num = final_block_num + }; + + fc::temp_directory tempdir; + store_provider sp(tempdir.path(), 100, std::optional(), std::optional(), 0); + + // on_accepted_block of the initial block + sp.append(initial_block_trace); // block 1 + sp.append_trx_ids(initial_block_trxs_entry); // block 1 + + // target trx is in the first block (is still reversible) + get_block_n block_num = sp.get_trx_block_number(target_trx_id, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, initial_block_num); + + // initial block forks out + sp.append(forked_block_trace); // block 1 + sp.append_trx_ids(forked_block_trxs_entry); // block 1 + + // forked block becomes final + sp.append_lib(initial_block_num); // block 1 + + // target trx is forked out. block 1 does not include + // target_trx (trx_trace1) but trx_trace2; + // therefore no block is found for target_trx_id. + block_num = sp.get_trx_block_number(target_trx_id, {}); + BOOST_REQUIRE(!block_num); + + // on_accepted_block of the final block + sp.append(final_block_trace); // block 3 + sp.append_trx_ids(final_block_trxs_entry); // block 3 + + // final block becomes final + sp.append_lib(final_block_num); // block 3 + + block_num = sp.get_trx_block_number(target_trx_id, {}); + BOOST_REQUIRE(block_num); + BOOST_REQUIRE_EQUAL(*block_num, final_block_num); // target trx is in final block + } BOOST_AUTO_TEST_SUITE_END()