diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 1156ad11a8..e7e0dfc3dc 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -5529,7 +5529,12 @@ TEST (ledger, cement_unbounded) auto ctx = nano::test::ledger_diamond (5); auto & ledger = ctx.ledger (); auto bottom = ctx.blocks ().back (); - auto confirmed = ledger.confirm (ledger.tx_begin_write (), bottom->hash ()); + + std::deque<std::shared_ptr<nano::block>> confirmed; + { + auto tx = ledger.tx_begin_write (); + confirmed = ledger.confirm (tx, bottom->hash ()); + } ASSERT_TRUE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); // Check that all blocks got confirmed in a single call ASSERT_TRUE (std::all_of (ctx.blocks ().begin (), ctx.blocks ().end (), [&] (auto const & block) { @@ -5547,8 +5552,13 @@ TEST (ledger, cement_bounded) auto & ledger = ctx.ledger (); auto bottom = ctx.blocks ().back (); - // This should only cement some of the dependencies - auto confirmed1 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 3); + std::deque<std::shared_ptr<nano::block>> confirmed1, confirmed2, confirmed3; + + { + // This should only cement some of the dependencies + auto tx = ledger.tx_begin_write (); + confirmed1 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 3); + } ASSERT_FALSE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_EQ (confirmed1.size (), 3); // Only topmost dependencies should get cemented during this call @@ -5556,8 +5566,11 @@ TEST (ledger, cement_bounded) return ledger.dependents_confirmed (ledger.tx_begin_read (), *block); })); - // This should cement a few more dependencies - auto confirmed2 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 16); + { + // This should cement a few more dependencies + auto tx = ledger.tx_begin_write (); + confirmed2 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 16); + } ASSERT_FALSE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_EQ (confirmed2.size (), 16); // Only topmost dependencies should get cemented during this call @@ -5565,8 +5578,11 @@ TEST (ledger, cement_bounded) return ledger.dependents_confirmed (ledger.tx_begin_read (), *block); })); - // This should cement the remaining dependencies - auto confirmed3 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 64); + { + // This should cement the remaining dependencies + auto tx = ledger.tx_begin_write (); + confirmed3 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 64); + } ASSERT_TRUE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_LE (confirmed3.size (), 64); // Every block in the ledger should be cemented @@ -5575,15 +5591,20 @@ TEST (ledger, cement_bounded) })); } -// Tests that bounded cementing works when mumber of blocks is large but tree height is small (recursion stack is small) +// Tests that bounded cementing works when number of blocks is large but tree height is small (recursion stack is small) TEST (ledger, cement_bounded_diamond) { auto ctx = nano::test::ledger_diamond (4); auto & ledger = ctx.ledger (); auto bottom = ctx.blocks ().back (); - // This should only cement some of the dependencies - auto confirmed1 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 3); + std::deque<std::shared_ptr<nano::block>> confirmed1, confirmed2, confirmed3, confirmed4; + + { + // This should only cement some of the dependencies + auto tx = ledger.tx_begin_write (); + confirmed1 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 3); + } ASSERT_FALSE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_EQ (confirmed1.size (), 3); // Only topmost dependencies should get cemented during this call @@ -5591,8 +5612,11 @@ TEST (ledger, cement_bounded_diamond) return ledger.dependents_confirmed (ledger.tx_begin_read (), *block); })); - // This should cement a few more dependencies - auto confirmed2 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 16); + { + // This should cement a few more dependencies + auto tx = ledger.tx_begin_write (); + confirmed2 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 16); + } ASSERT_FALSE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_EQ (confirmed2.size (), 16); // Only topmost dependencies should get cemented during this call @@ -5600,8 +5624,11 @@ TEST (ledger, cement_bounded_diamond) return ledger.dependents_confirmed (ledger.tx_begin_read (), *block); })); - // A few more bounded calls should confirm the whole tree - auto confirmed3 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 64); + { + // A few more bounded calls should confirm the whole tree + auto tx = ledger.tx_begin_write (); + confirmed3 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 64); + } ASSERT_FALSE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_EQ (confirmed3.size (), 64); // Only topmost dependencies should get cemented during this call @@ -5609,7 +5636,10 @@ TEST (ledger, cement_bounded_diamond) return ledger.dependents_confirmed (ledger.tx_begin_read (), *block); })); - auto confirmed4 = ledger.confirm (ledger.tx_begin_write (), bottom->hash (), /* max cementing batch size */ 64); + { + auto tx = ledger.tx_begin_write (); + confirmed4 = ledger.confirm (tx, bottom->hash (), /* max cementing batch size */ 64); + } ASSERT_TRUE (ledger.confirmed.block_exists (ledger.tx_begin_read (), bottom->hash ())); ASSERT_LT (confirmed4.size (), 64); // Every block in the ledger should be cemented diff --git a/nano/core_test/request_aggregator.cpp b/nano/core_test/request_aggregator.cpp index 5cc40202bb..fcb580ef9a 100644 --- a/nano/core_test/request_aggregator.cpp +++ b/nano/core_test/request_aggregator.cpp @@ -290,8 +290,11 @@ TEST (request_aggregator, split) ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), block)); request.emplace_back (block->hash (), block->root ()); } - // Confirm all blocks - node.ledger.confirm (node.ledger.tx_begin_write (), blocks.back ()->hash ()); + { + // Confirm all blocks + auto tx = node.ledger.tx_begin_write (); + node.ledger.confirm (tx, blocks.back ()->hash ()); + } ASSERT_TIMELY_EQ (5s, max_vbh + 2, node.ledger.cemented_count ()); ASSERT_EQ (max_vbh + 1, request.size ()); auto client = std::make_shared<nano::transport::tcp_socket> (node); diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index 31a161c8ac..666b2759d5 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -851,7 +851,7 @@ std::deque<std::shared_ptr<nano::block>> nano::ledger::confirm (secure::write_tr { // We must only confirm blocks that have their dependencies confirmed debug_assert (dependents_confirmed (transaction, *block)); - confirm (transaction, *block); + confirm_one (transaction, *block); result.push_back (block); } } @@ -878,7 +878,7 @@ std::deque<std::shared_ptr<nano::block>> nano::ledger::confirm (secure::write_tr return result; } -void nano::ledger::confirm (secure::write_transaction const & transaction, nano::block const & block) +void nano::ledger::confirm_one (secure::write_transaction & transaction, nano::block const & block) { debug_assert ((!store.confirmation_height.get (transaction, block.account ()) && block.sideband ().height == 1) || store.confirmation_height.get (transaction, block.account ()).value ().height + 1 == block.sideband ().height); confirmation_height_info info{ block.sideband ().height, block.hash () }; diff --git a/nano/secure/ledger.hpp b/nano/secure/ledger.hpp index 284a9cfc54..3419731e11 100644 --- a/nano/secure/ledger.hpp +++ b/nano/secure/ledger.hpp @@ -100,7 +100,7 @@ class ledger final private: void initialize (nano::generate_cache_flags const &); - void confirm (secure::write_transaction const & transaction, nano::block const & block); + void confirm_one (secure::write_transaction &, nano::block const & block); std::unique_ptr<ledger_set_any> any_impl; std::unique_ptr<ledger_set_confirmed> confirmed_impl; diff --git a/nano/test_common/testutil.cpp b/nano/test_common/testutil.cpp index d77eefc47b..59554846fa 100644 --- a/nano/test_common/testutil.cpp +++ b/nano/test_common/testutil.cpp @@ -138,7 +138,8 @@ void nano::test::confirm (nano::ledger & ledger, std::shared_ptr<nano::block> co void nano::test::confirm (nano::ledger & ledger, nano::block_hash const & hash) { - ledger.confirm (ledger.tx_begin_write (), hash); + auto transaction = ledger.tx_begin_write (); + ledger.confirm (transaction, hash); } bool nano::test::block_or_pruned_all_exists (nano::node & node, std::vector<nano::block_hash> hashes)