Skip to content

Commit

Permalink
Convert store::pending::get to return an optional rather than using a…
Browse files Browse the repository at this point in the history
…n out-argument and returning an error code. (#4479)
  • Loading branch information
clemahieu authored Mar 12, 2024
1 parent 623a135 commit 7515537
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 51 deletions.
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

0 comments on commit 7515537

Please sign in to comment.