Minecon724/dncurrency
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

Remove conf height put during ledger processing on a newly opened account (#2883)

Browse source 
* Don't initialize conf height table after a new account is opened

* Fix build error

* Readd allow_concurrent_memtable_write

* Add unconfirmed_frontiers test

* Drop confirmation height table with confirmation_height_clear

* Remove  unnecessary allow_concurrent_memtable_write change

* Check entry in conf height table for account doesn't exist if being rolled back (Serg review)

* Delete from conf height table if clearing account

* No need to pass existing conf height to clear function anymore
This commit is contained in:
Wesley Shillingford 2020-11-03 11:22:13 +00:00 • committed by GitHub
commit 36dc2e4cde
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 89 additions and 78 deletions
Download patch file Download diff file Expand all files Collapse all files

14
nano/core_test/block_store.cpp
View file

@ -1871,17 +1871,11 @@ TEST (block_store, confirmation_height)
store->confirmation_height_clear (transaction);
}
auto transaction (store->tx_begin_read ());
ASSERT_EQ (store->confirmation_height_count (transaction), 3);
ASSERT_EQ (store->confirmation_height_count (transaction), 0);
nano::confirmation_height_info confirmation_height_info;
ASSERT_FALSE (store->confirmation_height_get (transaction, account1, confirmation_height_info));
ASSERT_EQ (confirmation_height_info.height, 0);
ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
ASSERT_FALSE (store->confirmation_height_get (transaction, account2, confirmation_height_info));
ASSERT_EQ (confirmation_height_info.height, 0);
ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
ASSERT_FALSE (store->confirmation_height_get (transaction, account3, confirmation_height_info));
ASSERT_EQ (confirmation_height_info.height, 0);
ASSERT_EQ (confirmation_height_info.frontier, nano::block_hash (0));
ASSERT_TRUE (store->confirmation_height_get (transaction, account1, confirmation_height_info));
ASSERT_TRUE (store->confirmation_height_get (transaction, account2, confirmation_height_info));
ASSERT_TRUE (store->confirmation_height_get (transaction, account3, confirmation_height_info));
}
// Ledger versions are not forward compatible

10
nano/core_test/confirmation_height.cpp
View file

@ -129,19 +129,19 @@ TEST (confirmation_height, multiple_accounts)
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, send6).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, receive2).code);
// Check confirmation heights of all the accounts are uninitialized (0),
// Check confirmation heights of all the accounts (except genesis) are uninitialized (0),
// as we have any just added them to the ledger and not processed any live transactions yet.
nano::confirmation_height_info confirmation_height_info;
ASSERT_FALSE (node->store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
ASSERT_EQ (1, confirmation_height_info.height);
ASSERT_EQ (nano::genesis_hash, confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key1.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key2.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
ASSERT_FALSE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
ASSERT_TRUE (node->store.confirmation_height_get (transaction, key3.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}
@ -278,7 +278,7 @@ TEST (confirmation_height, gap_bootstrap)
ASSERT_FALSE (node1.store.confirmation_height_get (transaction, nano::dev_genesis_key.pub, confirmation_height_info));
ASSERT_EQ (1, confirmation_height_info.height);
ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
ASSERT_FALSE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
ASSERT_TRUE (node1.store.confirmation_height_get (transaction, destination.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}

43
nano/core_test/ledger.cpp
View file

@ -2984,7 +2984,7 @@ TEST (ledger, confirmation_height_not_updated)
ASSERT_EQ (genesis.hash (), confirmation_height_info.frontier);
nano::open_block open1 (send1.hash (), nano::genesis_account, key.pub, key.prv, key.pub, *pool.generate (key.pub));
ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, open1).code);
ASSERT_FALSE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
ASSERT_TRUE (store->confirmation_height_get (transaction, key.pub, confirmation_height_info));
ASSERT_EQ (0, confirmation_height_info.height);
ASSERT_EQ (nano::block_hash (0), confirmation_height_info.frontier);
}
@ -3229,7 +3229,7 @@ TEST (ledger, dependents_confirmed)
.build_shared ();
ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, *receive2).code);
ASSERT_FALSE (ledger.dependents_confirmed (transaction, *receive2));
ASSERT_FALSE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
ASSERT_TRUE (ledger.store.confirmation_height_get (transaction, key1.pub, height));
height.height += 1;
ledger.store.confirmation_height_put (transaction, key1.pub, height);
ASSERT_FALSE (ledger.dependents_confirmed (transaction, *receive2));
@ -3850,3 +3850,42 @@ TEST (ledger, migrate_lmdb_to_rocksdb)
ASSERT_EQ (unchecked_infos.front ().account, nano::genesis_account);
ASSERT_EQ (*unchecked_infos.front ().block, *send);
}
TEST (ledger, unconfirmed_frontiers)
{
nano::logger_mt logger;
auto store = nano::make_store (logger, nano::unique_path ());
ASSERT_TRUE (!store->init_error ());
nano::stat stats;
nano::ledger ledger (*store, stats);
nano::genesis genesis;
store->initialize (store->tx_begin_write (), genesis, ledger.cache);
nano::work_pool pool (std::numeric_limits<unsigned>::max ());
auto unconfirmed_frontiers = ledger.unconfirmed_frontiers ();
ASSERT_TRUE (unconfirmed_frontiers.empty ());
nano::state_block_builder builder;
nano::keypair key;
auto const latest = ledger.latest (store->tx_begin_read (), nano::genesis_account);
auto send = builder.make_block ()
.account (nano::genesis_account)
.previous (latest)
.representative (nano::genesis_account)
.balance (nano::genesis_amount - 100)
.link (key.pub)
.sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
.work (*pool.generate (latest))
.build ();
ASSERT_EQ (nano::process_result::progress, ledger.process (store->tx_begin_write (), *send).code);
unconfirmed_frontiers = ledger.unconfirmed_frontiers ();
ASSERT_EQ (unconfirmed_frontiers.size (), 1);
ASSERT_EQ (unconfirmed_frontiers.begin ()->first, 1);
nano::uncemented_info uncemented_info1{ latest, send->hash (), nano::genesis_account };
auto uncemented_info2 = unconfirmed_frontiers.begin ()->second;
ASSERT_EQ (uncemented_info1.account, uncemented_info2.account);
ASSERT_EQ (uncemented_info1.cemented_frontier, uncemented_info2.cemented_frontier);
ASSERT_EQ (uncemented_info1.frontier, uncemented_info2.frontier);
}

3
nano/lib/config.hpp
View file

@ -215,9 +215,6 @@ std::string get_rpc_toml_config_path (boost::filesystem::path const & data_path)
std::string get_access_toml_config_path (boost::filesystem::path const & data_path);
std::string get_qtwallet_toml_config_path (boost::filesystem::path const & data_path);
/** Called by gtest_main to enforce dev network */
void force_nano_dev_network ();
/** Checks if we are running inside a valgrind instance */
bool running_within_valgrind ();

21
nano/node/active_transactions.cpp
View file

@ -141,8 +141,7 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction
if (!this->confirmation_height_processor.is_processing_block (info.head))
{
nano::confirmation_height_info confirmation_height_info;
error = this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);
release_assert (!error);
this->node.store.confirmation_height_get (transaction_a, cementable_account.account, confirmation_height_info);
if (info.block_count > confirmation_height_info.height)
{
@ -491,8 +490,9 @@ void nano::active_transactions::confirm_expired_frontiers_pessimistically (nano:
auto const & account{ i->account };
nano::account_info account_info;
bool should_delete{ true };
if (!node.store.account_get (transaction_a, account, account_info) && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
if (!node.store.account_get (transaction_a, account, account_info))
{
node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
if (account_info.block_count > confirmation_height_info.height)
{
should_delete = false;
@ -696,12 +696,13 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
auto i (wallet->store.begin (wallet_transaction, next_wallet_frontier_account));
auto n (wallet->store.end ());
nano::confirmation_height_info confirmation_height_info;
for (; i != n && should_iterate (); ++i)
{
auto const & account (i->first);
if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.account_get (transaction_a, account, info) && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.account_get (transaction_a, account, info))
{
nano::confirmation_height_info confirmation_height_info;
node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
// If it exists in normal priority collection delete from there.
auto it = priority_cementable_frontiers.find (account);
if (it != priority_cementable_frontiers.end ())
@ -744,15 +745,16 @@ void nano::active_transactions::prioritize_frontiers_for_confirmation (nano::tra
nano::timer<std::chrono::milliseconds> timer (nano::timer_state::started);
auto i (node.store.accounts_begin (transaction_a, next_frontier_account));
auto n (node.store.accounts_end ());
nano::confirmation_height_info confirmation_height_info;
for (; i != n && should_iterate (); ++i)
{
auto const & account (i->first);
auto const & info (i->second);
if (priority_wallet_cementable_frontiers.find (account) == priority_wallet_cementable_frontiers.end ())
{
if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0 && !node.store.confirmation_height_get (transaction_a, account, confirmation_height_info))
if (expired_optimistic_election_infos.get<tag_account> ().count (account) == 0)
{
nano::confirmation_height_info confirmation_height_info;
node.store.confirmation_height_get (transaction_a, account, confirmation_height_info);
auto insert_newed = prioritize_account_for_confirmation (priority_cementable_frontiers, priority_cementable_frontiers_size, account, info, confirmation_height_info.height);
if (insert_newed)
{
@ -982,9 +984,8 @@ nano::election_insertion_result nano::active_transactions::activate (nano::accou
if (!node.store.account_get (transaction, account_a, account_info))
{
nano::confirmation_height_info conf_info;
auto error = node.store.confirmation_height_get (transaction, account_a, conf_info);
debug_assert (!error);
if (!error && conf_info.height < account_info.block_count)
node.store.confirmation_height_get (transaction, account_a, conf_info);
if (conf_info.height < account_info.block_count)
{
debug_assert (conf_info.frontier != account_info.head);
auto hash = conf_info.height == 0 ? account_info.open_block : node.store.block_successor (transaction, conf_info.frontier);

2
nano/node/blockprocessor.cpp
View file

@ -236,7 +236,7 @@ void nano::block_processor::process_batch (nano::unique_lock<std::mutex> & lock_
{
auto scoped_write_guard = write_database_queue.wait (nano::writer::process_batch);
block_post_events post_events ([& store = node.store] { return store.tx_begin_read (); });
auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }, { tables::confirmation_height }));
auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked }));
nano::timer<std::chrono::milliseconds> timer_l;
lock_a.lock ();
timer_l.start ();

2
nano/node/cli.cpp
View file

@ -574,7 +574,7 @@ std::error_code nano::handle_node_options (boost::program_options::variables_map
}
else
{
node.node->store.confirmation_height_clear (transaction, account, confirmation_height_info.height);
node.node->store.confirmation_height_clear (transaction, account);
}
std::cout << "Confirmation height of account " << account_str << " is set to " << conf_height_reset_num << std::endl;

16
nano/node/confirmation_height_bounded.cpp
View file

@ -112,9 +112,7 @@ void nano::confirmation_height_bounded::process ()
}
else
{
auto error = ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
(void)error;
debug_assert (!error);
ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
// This block was added to the confirmation height processor but is already confirmed
if (first_iter && confirmation_height_info.height >= block->sideband ().height && current == original_hash)
{
@ -378,7 +376,7 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
#ifndef NDEBUG
// Extra debug checks
nano::confirmation_height_info confirmation_height_info;
debug_assert (!ledger.store.confirmation_height_get (transaction, account, confirmation_height_info));
ledger.store.confirmation_height_get (transaction, account, confirmation_height_info);
auto block (ledger.store.block_get (transaction, confirmed_frontier));
debug_assert (block != nullptr);
debug_assert (block->sideband ().height == confirmation_height_info.height + num_blocks_cemented);
@ -390,16 +388,10 @@ void nano::confirmation_height_bounded::cement_blocks (nano::write_guard & scope
};
nano::confirmation_height_info confirmation_height_info;
error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
if (error)
{
auto error_str = (boost::format ("Failed to read confirmation height for account %1% (bounded processor)") % pending.account.to_account ()).str ();
logger.always_log (error_str);
std::cerr << error_str << std::endl;
}
ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
// Some blocks need to be cemented at least
if (!error && pending.top_height > confirmation_height_info.height)
if (pending.top_height > confirmation_height_info.height)
{
// The highest hash which will be cemented
nano::block_hash new_cemented_frontier;

12
nano/node/confirmation_height_unbounded.cpp
View file

@ -84,7 +84,7 @@ void nano::confirmation_height_unbounded::process ()
else
{
nano::confirmation_height_info confirmation_height_info;
release_assert (!ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info));
ledger.store.confirmation_height_get (read_transaction, account, confirmation_height_info);
confirmation_height = confirmation_height_info.height;
// This block was added to the confirmation height processor but is already confirmed
@ -349,15 +349,9 @@ void nano::confirmation_height_unbounded::cement_blocks (nano::write_guard & sco
{
auto & pending = pending_writes.front ();
nano::confirmation_height_info confirmation_height_info;
error = ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
if (error)
{
auto error_str = (boost::format ("Failed to read confirmation height for account %1% when writing block %2% (unbounded processor)") % pending.account.to_account () % pending.hash.to_string ()).str ();
logger.always_log (error_str);
std::cerr << error_str << std::endl;
}
ledger.store.confirmation_height_get (transaction, pending.account, confirmation_height_info);
auto confirmation_height = confirmation_height_info.height;
if (!error && pending.height > confirmation_height)
if (pending.height > confirmation_height)
{
auto block = ledger.store.block_get (transaction, pending.hash);
debug_assert (network_params.network.is_dev_network () || ledger.pruning || block != nullptr);

5
nano/node/json_handler.cpp
View file

@ -606,10 +606,7 @@ void nano::json_handler::account_info ()
auto transaction (node.store.tx_begin_read ());
auto info (account_info_impl (transaction, account));
nano::confirmation_height_info confirmation_height_info;
if (node.store.confirmation_height_get (transaction, account, confirmation_height_info) && include_confirmed)
{
ec = nano::error_common::account_not_found;
}
node.store.confirmation_height_get (transaction, account, confirmation_height_info);
if (!ec)
{
response_l.put ("frontier", info.head.to_string ());

4
nano/node/rocksdb/rocksdb.cpp
View file

@ -497,7 +497,8 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
{
db->GetIntProperty (table_to_column_family (table_a), "rocksdb.estimate-num-keys", &sum);
}
// These should only be used in tests to check database consistency
// Accounts and blocks should only be used in tests and CLI commands to check database consistency
// otherwise there can be performance issues.
else if (table_a == tables::accounts)
{
debug_assert (network_constants ().is_dev_network ());
@ -508,6 +509,7 @@ uint64_t nano::rocksdb_store::count (nano::transaction const & transaction_a, ta
}
else if (table_a == tables::blocks)
{
// This is also used in some CLI commands
for (auto i (blocks_begin (transaction_a)), n (blocks_end ()); i != n; ++i)
{
++sum;

2
nano/rpc_test/rpc.cpp
View file

@ -104,7 +104,7 @@ void reset_confirmation_height (nano::block_store & store, nano::account const &
nano::confirmation_height_info confirmation_height_info;
if (!store.confirmation_height_get (transaction, account, confirmation_height_info))
{
store.confirmation_height_clear (transaction, account, confirmation_height_info.height);
store.confirmation_height_clear (transaction, account);
}
}

2
nano/secure/blockstore.hpp
View file

@ -629,7 +629,7 @@ public:
virtual void account_del (nano::write_transaction const &, nano::account const &) = 0;
virtual bool account_exists (nano::transaction const &, nano::account const &) = 0;
virtual size_t account_count (nano::transaction const &) = 0;
virtual void confirmation_height_clear (nano::write_transaction const &, nano::account const &, uint64_t) = 0;
virtual void confirmation_height_clear (nano::write_transaction const &, nano::account const &) = 0;
virtual void confirmation_height_clear (nano::write_transaction const &) = 0;
virtual nano::store_iterator<nano::account, nano::account_info> accounts_begin (nano::transaction const &, nano::account const &) const = 0;
virtual nano::store_iterator<nano::account, nano::account_info> accounts_begin (nano::transaction const &) const = 0;

19
nano/secure/blockstore_partial.hpp
View file

@ -65,20 +65,14 @@ public:
return iterator != accounts_end () && nano::account (iterator->first) == account_a;
}
void confirmation_height_clear (nano::write_transaction const & transaction_a, nano::account const & account_a, uint64_t existing_confirmation_height_a) override
void confirmation_height_clear (nano::write_transaction const & transaction_a, nano::account const & account_a) override
{
if (existing_confirmation_height_a > 0)
{
confirmation_height_put (transaction_a, account_a, { 0, nano::block_hash{ 0 } });
}
confirmation_height_del (transaction_a, account_a);
}
void confirmation_height_clear (nano::write_transaction const & transaction_a) override
{
for (auto i (confirmation_height_begin (transaction_a)), n (confirmation_height_end ()); i != n; ++i)
{
confirmation_height_clear (transaction_a, i->first, i->second.height);
}
drop (transaction_a, nano::tables::confirmation_height);
}
bool pending_exists (nano::transaction const & transaction_a, nano::pending_key const & key_a) override
@ -414,7 +408,6 @@ public:
void account_put (nano::write_transaction const & transaction_a, nano::account const & account_a, nano::account_info const & info_a) override
{
// Check we are still in sync with other tables
debug_assert (confirmation_height_exists (transaction_a, account_a));
nano::db_val<Val> info (info_a);
auto status = put (transaction_a, tables::accounts, account_a, info);
release_assert (success (status));
@ -584,6 +577,12 @@ public:
nano::bufferstream stream (reinterpret_cast<uint8_t const *> (value.data ()), value.size ());
result = confirmation_height_info_a.deserialize (stream);
}
if (result)
{
confirmation_height_info_a.height = 0;
confirmation_height_info_a.frontier = 0;
}
return result;
}

12
nano/secure/ledger.cpp
View file

@ -1057,12 +1057,10 @@ bool nano::ledger::rollback (nano::write_transaction const & transaction_a, nano
while (!error && store.block_exists (transaction_a, block_a))
{
nano::confirmation_height_info confirmation_height_info;
auto latest_error = store.confirmation_height_get (transaction_a, account_l, confirmation_height_info);
debug_assert (!latest_error);
(void)latest_error;
store.confirmation_height_get (transaction_a, account_l, confirmation_height_info);
if (block_account_height > confirmation_height_info.height)
{
latest_error = store.account_get (transaction_a, account_l, account_info);
auto latest_error = store.account_get (transaction_a, account_l, account_info);
debug_assert (!latest_error);
auto block (store.block_get (transaction_a, account_info.head));
list_a.push_back (block);
@ -1266,8 +1264,6 @@ void nano::ledger::update_account (nano::write_transaction const & transaction_a
{
if (old_a.head.is_zero () && new_a.open_block == new_a.head)
{
debug_assert (!store.confirmation_height_exists (transaction_a, account_a));
store.confirmation_height_put (transaction_a, account_a, { 0, nano::block_hash (0) });
++cache.account_count;
}
if (!old_a.head.is_zero () && old_a.epoch () != new_a.epoch ())
@ -1279,7 +1275,7 @@ void nano::ledger::update_account (nano::write_transaction const & transaction_a
}
else
{
store.confirmation_height_del (transaction_a, account_a);
debug_assert (!store.confirmation_height_exists (transaction_a, account_a));
store.account_del (transaction_a, account_a);
debug_assert (cache.account_count > 0);
--cache.account_count;
@ -1345,7 +1341,7 @@ bool nano::ledger::block_confirmed (nano::transaction const & transaction_a, nan
if (block)
{
nano::confirmation_height_info confirmation_height_info;
release_assert (!store.confirmation_height_get (transaction_a, block->account ().is_zero () ? block->sideband ().account : block->account (), confirmation_height_info));
store.confirmation_height_get (transaction_a, block->account ().is_zero () ? block->sideband ().account : block->account (), confirmation_height_info);
confirmed = (confirmation_height_info.height >= block->sideband ().height);
}
return confirmed;