From 2ac4aa79d8cd408918d91fa708880142a247d595 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Thu, 11 Apr 2024 15:24:53 +0100 Subject: [PATCH 1/3] Use enclosing transaction within scheduler::optimistic::run_one rather than opening a new one. --- nano/node/scheduler/optimistic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nano/node/scheduler/optimistic.cpp b/nano/node/scheduler/optimistic.cpp index cda286665..2a3c3732d 100644 --- a/nano/node/scheduler/optimistic.cpp +++ b/nano/node/scheduler/optimistic.cpp @@ -155,7 +155,7 @@ void nano::scheduler::optimistic::run_one (store::transaction const & transactio if (block) { // Ensure block is not already confirmed - if (!node.block_confirmed_or_being_confirmed (block->hash ())) + if (!node.block_confirmed_or_being_confirmed (transaction, block->hash ())) { // Try to insert it into AEC // We check for AEC vacancy inside our predicate From 7cd86ccf62d67842d083cd0b8932362d9e4868da Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Sat, 13 Apr 2024 11:45:24 +0100 Subject: [PATCH 2/3] Eliminate recursive opening of transactions in unit tests. Recursive transactions are unnecessary and will deadlock when memory protection is needed/added to ledger transactions. --- nano/core_test/wallet.cpp | 3 +-- nano/rpc_test/rpc.cpp | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/nano/core_test/wallet.cpp b/nano/core_test/wallet.cpp index 05d49392f..e099e7576 100644 --- a/nano/core_test/wallet.cpp +++ b/nano/core_test/wallet.cpp @@ -650,8 +650,7 @@ TEST (wallet, work_generate) } nano::keypair key; auto block (wallet->send_action (nano::dev::genesis_key.pub, key.pub, 100)); - auto transaction (node1.store.tx_begin_read ()); - ASSERT_TIMELY (10s, node1.ledger.account_balance (transaction, nano::dev::genesis_key.pub) != amount1); + ASSERT_TIMELY (10s, node1.ledger.account_balance (node1.ledger.store.tx_begin_read (), nano::dev::genesis_key.pub) != amount1); system.deadline_set (10s); auto again (true); while (again) diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 78893bc53..27961b142 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -5770,8 +5770,8 @@ TEST (rpc, block_confirmed) // Open an account directly in the ledger { - auto transaction = node->store.tx_begin_write (); - nano::block_hash latest (node->latest (nano::dev::genesis_key.pub)); + auto transaction = node->ledger.store.tx_begin_write (); + nano::block_hash latest (node->ledger.latest (transaction, nano::dev::genesis_key.pub)); auto send1 = builder .send () .previous (latest) From e1cabb31e26e0dfc39d2ad83fc51a111f37c0ac0 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Sat, 13 Apr 2024 11:47:43 +0100 Subject: [PATCH 3/3] Use temporary expression ledger transactions to avoid long-running and recursive transaction opening. In this algorithm the receivable_iterator was not incremented so the stored transaction was not used. The outer loop already re-opens the transaction in order to get an up-to-date view of the ledger. --- nano/node/epoch_upgrader.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/nano/node/epoch_upgrader.cpp b/nano/node/epoch_upgrader.cpp index c4f683d49..5ac75b436 100644 --- a/nano/node/epoch_upgrader.cpp +++ b/nano/node/epoch_upgrader.cpp @@ -134,9 +134,8 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc uint64_t attempts (0); for (auto i (accounts_list.get ().begin ()), n (accounts_list.get ().end ()); i != n && attempts < upgrade_batch_size && attempts < count_limit && !stopped; ++i) { - auto transaction (store.tx_begin_read ()); nano::account const & account (i->account); - auto info = ledger.account_info (transaction, account); + auto info = ledger.account_info (ledger.store.tx_begin_read (), account); if (info && info->epoch () < epoch_a) { ++attempts; @@ -210,11 +209,10 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc std::atomic upgraded_pending (0); uint64_t workers (0); uint64_t attempts (0); - auto transaction = store.tx_begin_read (); - for (auto current = ledger.receivable_upper_bound (transaction, 0), end = ledger.receivable_end (); current != end && attempts < upgrade_batch_size && attempts < count_limit && !stopped;) + for (auto current = ledger.receivable_upper_bound (ledger.store.tx_begin_read (), 0), end = ledger.receivable_end (); current != end && attempts < upgrade_batch_size && attempts < count_limit && !stopped;) { auto const & [key, info] = *current; - if (!store.account.exists (transaction, key.account)) + if (!store.account.exists (ledger.store.tx_begin_read (), key.account)) { if (info.epoch < epoch_a) { @@ -257,7 +255,7 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc } } // Move to next pending item - current = ledger.receivable_upper_bound (transaction, key.account, key.hash); + current = ledger.receivable_upper_bound (ledger.store.tx_begin_read (), key.account, key.hash); } else { @@ -268,7 +266,7 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc } else { - current = ledger.receivable_upper_bound (transaction, key.account); + current = ledger.receivable_upper_bound (ledger.store.tx_begin_read (), key.account); } } }