Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert store::pending::get to return an optional rather than using an out-argument and returning an error code. #4479

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions nano/core_test/block_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,15 +295,15 @@ TEST (block_store, add_pending)
ASSERT_TRUE (!store->init_error ());
nano::keypair key1;
nano::pending_key key2 (0, 0);
nano::pending_info pending1;
auto transaction (store->tx_begin_write ());
ASSERT_TRUE (store->pending.get (transaction, key2, pending1));
ASSERT_FALSE (store->pending.get (transaction, key2));
nano::pending_info pending1;
store->pending.put (transaction, key2, pending1);
nano::pending_info pending2;
ASSERT_FALSE (store->pending.get (transaction, key2, pending2));
std::optional<nano::pending_info> pending2;
ASSERT_TRUE (pending2 = store->pending.get (transaction, key2));
ASSERT_EQ (pending1, pending2);
store->pending.del (transaction, key2);
ASSERT_TRUE (store->pending.get (transaction, key2, pending2));
ASSERT_FALSE (store->pending.get (transaction, key2));
}

TEST (block_store, pending_iterator)
Expand Down
3 changes: 1 addition & 2 deletions nano/core_test/ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5481,8 +5481,7 @@ TEST (ledger, migrate_lmdb_to_rocksdb)
nano::store::rocksdb::component rocksdb_store{ logger, path / "rocksdb", nano::dev::constants };
auto rocksdb_transaction (rocksdb_store.tx_begin_read ());

nano::pending_info pending_info{};
ASSERT_FALSE (rocksdb_store.pending.get (rocksdb_transaction, nano::pending_key (nano::dev::genesis_key.pub, send->hash ()), pending_info));
ASSERT_TRUE (rocksdb_store.pending.get (rocksdb_transaction, nano::pending_key (nano::dev::genesis_key.pub, send->hash ())));

for (auto i = rocksdb_store.online_weight.begin (rocksdb_transaction); i != rocksdb_store.online_weight.end (); ++i)
{
Expand Down
14 changes: 6 additions & 8 deletions nano/qt/qt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2322,8 +2322,7 @@ void nano_qt::block_creation::create_receive ()
if (!destination.is_zero ())
{
nano::pending_key pending_key (destination, source_l);
nano::pending_info pending;
if (!wallet.node.store.pending.get (block_transaction, pending_key, pending))
if (auto pending = wallet.node.store.pending.get (block_transaction, pending_key))
{
nano::account_info info;
auto error (wallet.node.store.account.get (block_transaction, pending_key.account, info));
Expand All @@ -2333,10 +2332,10 @@ void nano_qt::block_creation::create_receive ()
auto error (wallet.wallet_m->store.fetch (transaction, pending_key.account, key));
if (!error)
{
nano::state_block receive (pending_key.account, info.head, info.representative, info.balance.number () + pending.amount.number (), source_l, key, pending_key.account, 0);
nano::state_block receive (pending_key.account, info.head, info.representative, info.balance.number () + pending.value ().amount.number (), source_l, key, pending_key.account, 0);
nano::block_details details;
details.is_receive = true;
details.epoch = std::max (info.epoch (), pending.epoch);
details.epoch = std::max (info.epoch (), pending.value ().epoch);
auto required_difficulty{ wallet.node.network_params.work.threshold (receive.work_version (), details) };
if (wallet.node.work_generate_blocking (receive, required_difficulty).has_value ())
{
Expand Down Expand Up @@ -2487,8 +2486,7 @@ void nano_qt::block_creation::create_open ()
if (!destination.is_zero ())
{
nano::pending_key pending_key (destination, source_l);
nano::pending_info pending;
if (!wallet.node.store.pending.get (block_transaction, pending_key, pending))
if (auto pending = wallet.node.store.pending.get (block_transaction, pending_key))
{
nano::account_info info;
auto error (wallet.node.store.account.get (block_transaction, pending_key.account, info));
Expand All @@ -2498,10 +2496,10 @@ void nano_qt::block_creation::create_open ()
auto error (wallet.wallet_m->store.fetch (transaction, pending_key.account, key));
if (!error)
{
nano::state_block open (pending_key.account, 0, representative_l, pending.amount, source_l, key, pending_key.account, 0);
nano::state_block open (pending_key.account, 0, representative_l, pending.value ().amount, source_l, key, pending_key.account, 0);
nano::block_details details;
details.is_receive = true;
details.epoch = pending.epoch;
details.epoch = pending.value ().epoch;
auto const required_difficulty{ wallet.node.network_params.work.threshold (open.work_version (), details) };
if (wallet.node.work_generate_blocking (open, required_difficulty).has_value ())
{
Expand Down
50 changes: 23 additions & 27 deletions nano/secure/ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,24 @@ class rollback_visitor : public nano::block_visitor
void send_block (nano::send_block const & block_a) override
{
auto hash (block_a.hash ());
nano::pending_info pending;
nano::pending_key key (block_a.hashables.destination, hash);
while (!error && ledger.store.pending.get (transaction, key, pending))
auto pending = ledger.store.pending.get (transaction, key);
while (!error && !pending.has_value ())
{
error = ledger.rollback (transaction, ledger.latest (transaction, block_a.hashables.destination), list);
pending = ledger.store.pending.get (transaction, key);
}
if (!error)
{
auto info = ledger.account_info (transaction, pending.source);
auto info = ledger.account_info (transaction, pending.value ().source);
debug_assert (info);
ledger.store.pending.del (transaction, key);
ledger.cache.rep_weights.representation_add (info->representative, pending.amount.number ());
ledger.cache.rep_weights.representation_add (info->representative, pending.value ().amount.number ());
nano::account_info new_info (block_a.hashables.previous, info->representative, info->open_block, ledger.balance (transaction, block_a.hashables.previous).value (), nano::seconds_since_epoch (), info->block_count - 1, nano::epoch::epoch_0);
ledger.update_account (transaction, pending.source, *info, new_info);
ledger.update_account (transaction, pending.value ().source, *info, new_info);
ledger.store.block.del (transaction, hash);
ledger.store.frontier.del (transaction, hash);
ledger.store.frontier.put (transaction, block_a.hashables.previous, pending.source);
ledger.store.frontier.put (transaction, block_a.hashables.previous, pending.value ().source);
ledger.store.block.successor_clear (transaction, block_a.hashables.previous);
ledger.stats.inc (nano::stat::type::rollback, nano::stat::detail::send);
}
Expand Down Expand Up @@ -315,12 +316,12 @@ void ledger_processor::state_block_impl (nano::state_block & block_a)
if (result == nano::block_status::progress)
{
nano::pending_key key (block_a.hashables.account, block_a.hashables.link.as_block_hash ());
nano::pending_info pending;
result = ledger.store.pending.get (transaction, key, pending) ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
auto pending = ledger.store.pending.get (transaction, key);
result = !pending ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
if (result == nano::block_status::progress)
{
result = amount == pending.amount ? nano::block_status::progress : nano::block_status::balance_mismatch;
source_epoch = pending.epoch;
result = amount == pending.value ().amount ? nano::block_status::progress : nano::block_status::balance_mismatch;
source_epoch = pending.value ().epoch;
epoch = std::max (epoch, source_epoch);
}
}
Expand Down Expand Up @@ -577,18 +578,18 @@ void ledger_processor::receive_block (nano::receive_block & block_a)
if (result == nano::block_status::progress)
{
nano::pending_key key (account, block_a.hashables.source);
nano::pending_info pending;
result = ledger.store.pending.get (transaction, key, pending) ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
auto pending = ledger.store.pending.get (transaction, key);
result = !pending ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
if (result == nano::block_status::progress)
{
result = pending.epoch == nano::epoch::epoch_0 ? nano::block_status::progress : nano::block_status::unreceivable; // Are we receiving a state-only send? (Malformed)
result = pending.value ().epoch == nano::epoch::epoch_0 ? nano::block_status::progress : nano::block_status::unreceivable; // Are we receiving a state-only send? (Malformed)
if (result == nano::block_status::progress)
{
nano::block_details block_details (nano::epoch::epoch_0, false /* unused */, false /* unused */, false /* unused */);
result = ledger.constants.work.difficulty (block_a) >= ledger.constants.work.threshold (block_a.work_version (), block_details) ? nano::block_status::progress : nano::block_status::insufficient_work; // Does this block have sufficient work? (Malformed)
if (result == nano::block_status::progress)
{
auto new_balance (info->balance.number () + pending.amount.number ());
auto new_balance (info->balance.number () + pending.value ().amount.number ());
#ifdef NDEBUG
if (ledger.store.block.exists (transaction, block_a.hashables.source))
{
Expand All @@ -601,7 +602,7 @@ void ledger_processor::receive_block (nano::receive_block & block_a)
ledger.store.block.put (transaction, hash, block_a);
nano::account_info new_info (hash, info->representative, info->open_block, new_balance, nano::seconds_since_epoch (), info->block_count + 1, nano::epoch::epoch_0);
ledger.update_account (transaction, account, *info, new_info);
ledger.cache.rep_weights.representation_add (info->representative, pending.amount.number ());
ledger.cache.rep_weights.representation_add (info->representative, pending.value ().amount.number ());
ledger.store.frontier.del (transaction, block_a.hashables.previous);
ledger.store.frontier.put (transaction, hash, account);
ledger.stats.inc (nano::stat::type::ledger, nano::stat::detail::receive);
Expand Down Expand Up @@ -640,14 +641,14 @@ void ledger_processor::open_block (nano::open_block & block_a)
if (result == nano::block_status::progress)
{
nano::pending_key key (block_a.hashables.account, block_a.hashables.source);
nano::pending_info pending;
result = ledger.store.pending.get (transaction, key, pending) ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
auto pending = ledger.store.pending.get (transaction, key);
result = !pending ? nano::block_status::unreceivable : nano::block_status::progress; // Has this source already been received (Malformed)
if (result == nano::block_status::progress)
{
result = block_a.hashables.account == ledger.constants.burn_account ? nano::block_status::opened_burn_account : nano::block_status::progress; // Is it burning 0 account? (Malicious)
if (result == nano::block_status::progress)
{
result = pending.epoch == nano::epoch::epoch_0 ? nano::block_status::progress : nano::block_status::unreceivable; // Are we receiving a state-only send? (Malformed)
result = pending.value ().epoch == nano::epoch::epoch_0 ? nano::block_status::progress : nano::block_status::unreceivable; // Are we receiving a state-only send? (Malformed)
if (result == nano::block_status::progress)
{
nano::block_details block_details (nano::epoch::epoch_0, false /* unused */, false /* unused */, false /* unused */);
Expand All @@ -663,11 +664,11 @@ void ledger_processor::open_block (nano::open_block & block_a)
}
#endif
ledger.store.pending.del (transaction, key);
block_a.sideband_set (nano::block_sideband (block_a.hashables.account, 0, pending.amount, 1, nano::seconds_since_epoch (), block_details, nano::epoch::epoch_0 /* unused */));
block_a.sideband_set (nano::block_sideband (block_a.hashables.account, 0, pending.value ().amount, 1, nano::seconds_since_epoch (), block_details, nano::epoch::epoch_0 /* unused */));
ledger.store.block.put (transaction, hash, block_a);
nano::account_info new_info (hash, block_a.representative_field ().value (), hash, pending.amount.number (), nano::seconds_since_epoch (), 1, nano::epoch::epoch_0);
nano::account_info new_info (hash, block_a.representative_field ().value (), hash, pending.value ().amount.number (), nano::seconds_since_epoch (), 1, nano::epoch::epoch_0);
ledger.update_account (transaction, block_a.hashables.account, info, new_info);
ledger.cache.rep_weights.representation_add (block_a.representative_field ().value (), pending.amount.number ());
ledger.cache.rep_weights.representation_add (block_a.representative_field ().value (), pending.value ().amount.number ());
ledger.store.frontier.put (transaction, hash, block_a.hashables.account);
ledger.stats.inc (nano::stat::type::ledger, nano::stat::detail::open);
}
Expand Down Expand Up @@ -880,12 +881,7 @@ nano::uint128_t nano::ledger::account_receivable (store::transaction const & tra

std::optional<nano::pending_info> nano::ledger::pending_info (store::transaction const & transaction, nano::pending_key const & key) const
{
nano::pending_info result;
if (!store.pending.get (transaction, key, result))
{
return result;
}
return std::nullopt;
return store.pending.get (transaction, key);
}

nano::block_status nano::ledger::process (store::write_transaction const & transaction_a, std::shared_ptr<nano::block> block_a)
Expand Down
8 changes: 5 additions & 3 deletions nano/store/lmdb/pending.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ void nano::store::lmdb::pending::del (store::write_transaction const & transacti
store.release_assert_success (status);
}

bool nano::store::lmdb::pending::get (store::transaction const & transaction, nano::pending_key const & key, nano::pending_info & pending_a)
std::optional<nano::pending_info> nano::store::lmdb::pending::get (store::transaction const & transaction, nano::pending_key const & key)
{
nano::store::lmdb::db_val value;
auto status1 = store.get (transaction, tables::pending, key, value);
release_assert (store.success (status1) || store.not_found (status1));
bool result (true);
std::optional<nano::pending_info> result;
if (store.success (status1))
{
nano::bufferstream stream (reinterpret_cast<uint8_t const *> (value.data ()), value.size ());
result = pending_a.deserialize (stream);
result = nano::pending_info{};
auto error = result.value ().deserialize (stream);
release_assert (!error);
}
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion nano/store/lmdb/pending.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class pending : public nano::store::pending
explicit pending (nano::store::lmdb::component & store_a);
void put (store::write_transaction const & transaction_a, nano::pending_key const & key_a, nano::pending_info const & pending_info_a) override;
void del (store::write_transaction const & transaction_a, nano::pending_key const & key_a) override;
bool get (store::transaction const & transaction_a, nano::pending_key const & key_a, nano::pending_info & pending_a) override;
std::optional<nano::pending_info> get (store::transaction const & transaction_a, nano::pending_key const & key_a) override;
bool exists (store::transaction const & transaction_a, nano::pending_key const & key_a) override;
bool any (store::transaction const & transaction_a, nano::account const & account_a) override;
store::iterator<nano::pending_key, nano::pending_info> begin (store::transaction const & transaction_a, nano::pending_key const & key_a) const override;
Expand Down
3 changes: 2 additions & 1 deletion nano/store/pending.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <nano/store/iterator.hpp>

#include <functional>
#include <optional>

namespace nano
{
Expand All @@ -22,7 +23,7 @@ class pending
public:
virtual void put (store::write_transaction const &, nano::pending_key const &, nano::pending_info const &) = 0;
virtual void del (store::write_transaction const &, nano::pending_key const &) = 0;
virtual bool get (store::transaction const &, nano::pending_key const &, nano::pending_info &) = 0;
virtual std::optional<nano::pending_info> get (store::transaction const &, nano::pending_key const &) = 0;
virtual bool exists (store::transaction const &, nano::pending_key const &) = 0;
virtual bool any (store::transaction const &, nano::account const &) = 0;
virtual store::iterator<nano::pending_key, nano::pending_info> begin (store::transaction const &, nano::pending_key const &) const = 0;
Expand Down
8 changes: 5 additions & 3 deletions nano/store/rocksdb/pending.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@ void nano::store::rocksdb::pending::del (store::write_transaction const & transa
store.release_assert_success (status);
}

bool nano::store::rocksdb::pending::get (store::transaction const & transaction, nano::pending_key const & key, nano::pending_info & pending)
std::optional<nano::pending_info> nano::store::rocksdb::pending::get (store::transaction const & transaction, nano::pending_key const & key)
{
nano::store::rocksdb::db_val value;
auto status1 = store.get (transaction, tables::pending, key, value);
release_assert (store.success (status1) || store.not_found (status1));
bool result (true);
std::optional<nano::pending_info> result;
if (store.success (status1))
{
nano::bufferstream stream (reinterpret_cast<uint8_t const *> (value.data ()), value.size ());
result = pending.deserialize (stream);
result = nano::pending_info{};
auto error = result.value ().deserialize (stream);
release_assert (!error);
}
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion nano/store/rocksdb/pending.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class pending : public nano::store::pending
explicit pending (nano::store::rocksdb::component & store_a);
void put (store::write_transaction const & transaction_a, nano::pending_key const & key_a, nano::pending_info const & pending_info_a) override;
void del (store::write_transaction const & transaction_a, nano::pending_key const & key_a) override;
bool get (store::transaction const & transaction_a, nano::pending_key const & key_a, nano::pending_info & pending_a) override;
std::optional<nano::pending_info> get (store::transaction const & transaction_a, nano::pending_key const & key_a) override;
bool exists (store::transaction const & transaction_a, nano::pending_key const & key_a) override;
bool any (store::transaction const & transaction_a, nano::account const & account_a) override;
store::iterator<nano::pending_key, nano::pending_info> begin (store::transaction const & transaction_a, nano::pending_key const & key_a) const override;
Expand Down
Loading