Skip to content

Commit

Permalink
Refer to empty/null accounts as nano::account{nullptr} (#3388)
Browse files Browse the repository at this point in the history
* Introduce null accounts as nano::account{nullptr}

* Fix formatting

* Default nano::account constructor to nano::account{0}

* Fix formatting

* Introduce nano::public_key::null() method instead of std::nullptr_t constructor

* Fix formatting

* Address code review #3

* Last code review addressing

* Fix formatting

* Address code review #4
  • Loading branch information
theohax authored Oct 6, 2021
1 parent e9fe080 commit 1eefcaf
Show file tree
Hide file tree
Showing 38 changed files with 195 additions and 123 deletions.
4 changes: 2 additions & 2 deletions nano/core_test/active_transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ TEST (active_transactions, confirm_active)
nano::lock_guard<nano::mutex> guard (node2.rep_crawler.probable_reps_mutex);
node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, *peers.begin ());
}
ASSERT_TIMELY (5s, election->votes ().size () != 1); // Votes were inserted (except for not_an_account)
ASSERT_TIMELY (5s, election->votes ().size () != 1); // Votes were inserted (except for the null account)
auto confirm_req_count (election->confirmation_request_count.load ());
// At least one confirmation request
ASSERT_GT (confirm_req_count, 0u);
Expand All @@ -62,7 +62,7 @@ TEST (active_transactions, confirm_active)
ASSERT_TIMELY (10s, node2.ledger.cache.cemented_count == 2 && node2.active.empty ());
// At least one more confirmation request
ASSERT_GT (election->confirmation_request_count, confirm_req_count);
// Blocks were cleared (except for not_an_account)
// Blocks were cleared (except for the null account)
ASSERT_EQ (1, election->blocks ().size ());
}
}
Expand Down
12 changes: 6 additions & 6 deletions nano/core_test/block_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ TEST (block_store, frontier_retrieval)
nano::logger_mt logger;
auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants);
ASSERT_TRUE (!store->init_error ());
nano::account account1 (0);
nano::account account1{};
nano::account_info info1 (0, 0, 0, 0, 0, 0, nano::epoch::epoch_0);
auto transaction (store->tx_begin_write ());
store->confirmation_height.put (transaction, account1, { 0, nano::block_hash (0) });
Expand All @@ -538,7 +538,7 @@ TEST (block_store, one_account)
nano::logger_mt logger;
auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants);
ASSERT_TRUE (!store->init_error ());
nano::account account (0);
nano::account account{};
nano::block_hash hash (0);
auto transaction (store->tx_begin_write ());
store->confirmation_height.put (transaction, account, { 20, nano::block_hash (15) });
Expand Down Expand Up @@ -783,7 +783,7 @@ TEST (block_store, large_iteration)
store->account.put (transaction, account, nano::account_info ());
}
std::unordered_set<nano::account> accounts2;
nano::account previous (0);
nano::account previous{};
auto transaction (store->tx_begin_read ());
for (auto i (store->account.begin (transaction, 0)), n (store->account.end ()); i != n; ++i)
{
Expand Down Expand Up @@ -1874,9 +1874,9 @@ TEST (block_store, confirmation_height)
nano::logger_mt logger;
auto store = nano::make_store (logger, path, nano::dev::constants);

nano::account account1 (0);
nano::account account2 (1);
nano::account account3 (2);
nano::account account1{};
nano::account account2{ 1 };
nano::account account3{ 2 };
nano::block_hash cemented_frontier1 (3);
nano::block_hash cemented_frontier2 (4);
nano::block_hash cemented_frontier3 (5);
Expand Down
6 changes: 3 additions & 3 deletions nano/core_test/ledger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1525,13 +1525,13 @@ TEST (ledger, block_destination_source)
ASSERT_TRUE (ledger.block_source (transaction, block1).is_zero ());
ASSERT_EQ (nano::dev::genesis->account (), ledger.block_destination (transaction, block2));
ASSERT_TRUE (ledger.block_source (transaction, block2).is_zero ());
ASSERT_TRUE (ledger.block_destination (transaction, block3).is_zero ());
ASSERT_TRUE (ledger.block_destination (transaction, block3) == nullptr);
ASSERT_EQ (block2.hash (), ledger.block_source (transaction, block3));
ASSERT_EQ (dest.pub, ledger.block_destination (transaction, block4));
ASSERT_TRUE (ledger.block_source (transaction, block4).is_zero ());
ASSERT_EQ (nano::dev::genesis->account (), ledger.block_destination (transaction, block5));
ASSERT_TRUE (ledger.block_source (transaction, block5).is_zero ());
ASSERT_TRUE (ledger.block_destination (transaction, block6).is_zero ());
ASSERT_TRUE (ledger.block_destination (transaction, block6) == nullptr);
ASSERT_EQ (block5.hash (), ledger.block_source (transaction, block6));
}

Expand Down Expand Up @@ -2411,7 +2411,7 @@ TEST (ledger, epoch_blocks_fork)
store->initialize (transaction, ledger.cache);
nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits<unsigned>::max () };
nano::keypair destination;
nano::send_block send1 (nano::dev::genesis->hash (), nano::account (0), nano::dev::constants.genesis_amount, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *pool.generate (nano::dev::genesis->hash ()));
nano::send_block send1 (nano::dev::genesis->hash (), nano::account{}, nano::dev::constants.genesis_amount, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *pool.generate (nano::dev::genesis->hash ()));
ASSERT_EQ (nano::process_result::progress, ledger.process (transaction, send1).code);
nano::state_block epoch1 (nano::dev::genesis->account (), nano::dev::genesis->hash (), nano::dev::genesis->account (), nano::dev::constants.genesis_amount, ledger.epoch_link (nano::epoch::epoch_1), nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *pool.generate (nano::dev::genesis->hash ()));
ASSERT_EQ (nano::process_result::fork, ledger.process (transaction, epoch1).code);
Expand Down
2 changes: 1 addition & 1 deletion nano/core_test/network.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ TEST (tcp_listener, tcp_node_id_handshake)

ASSERT_TIMELY (5s, write_done);

boost::optional<std::pair<nano::account, nano::signature>> response_zero (std::make_pair (nano::account (0), nano::signature (0)));
boost::optional<std::pair<nano::account, nano::signature>> response_zero (std::make_pair (nano::account{}, nano::signature (0)));
nano::node_id_handshake node_id_handshake_response{ nano::dev::network_params.network, boost::none, response_zero };
auto output (node_id_handshake_response.to_bytes ());
std::atomic<bool> done (false);
Expand Down
11 changes: 11 additions & 0 deletions nano/core_test/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ namespace
void add_required_children_node_config_tree (nano::jsonconfig & tree);
}

TEST (node, null_account)
{
const auto & null_account = nano::account::null ();
ASSERT_TRUE (null_account == nullptr);
ASSERT_FALSE (null_account != nullptr);

nano::account default_account{};
ASSERT_FALSE (default_account == nullptr);
ASSERT_TRUE (default_account != nullptr);
}

TEST (node, stop)
{
nano::system system (1);
Expand Down
2 changes: 1 addition & 1 deletion nano/core_test/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ TEST (system, DISABLED_generate_send_new)
// This indirectly waits for online weight to stabilize, required to prevent intermittent failures
ASSERT_TIMELY (5s, node1.wallets.reps ().voting > 0);
system.generate_send_new (node1, accounts);
nano::account new_account (0);
nano::account new_account{};
{
auto transaction (node1.wallets.tx_begin_read ());
auto iterator2 (system.wallet (0)->store.begin (transaction));
Expand Down
4 changes: 2 additions & 2 deletions nano/core_test/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ TEST (wallet, rekey)

TEST (account, encode_zero)
{
nano::account number0 (0);
nano::account number0{};
std::string str0;
number0.encode_account (str0);

Expand Down Expand Up @@ -367,7 +367,7 @@ TEST (account, encode_all)

TEST (account, encode_fail)
{
nano::account number0 (0);
nano::account number0{};
std::string str0;
number0.encode_account (str0);
str0[16] ^= 1;
Expand Down
25 changes: 20 additions & 5 deletions nano/lib/blocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <nano/lib/memory.hpp>
#include <nano/lib/numbers.hpp>
#include <nano/lib/threading.hpp>
#include <nano/secure/common.hpp>

#include <crypto/cryptopp/words.h>

Expand Down Expand Up @@ -151,8 +152,8 @@ bool nano::block::has_sideband () const

nano::account const & nano::block::representative () const
{
static nano::account rep{ 0 };
return rep;
static nano::account representative{};
return representative;
}

nano::block_hash const & nano::block::source () const
Expand All @@ -163,7 +164,7 @@ nano::block_hash const & nano::block::source () const

nano::account const & nano::block::destination () const
{
static nano::account destination{ 0 };
static nano::account destination{};
return destination;
}

Expand All @@ -175,7 +176,7 @@ nano::link const & nano::block::link () const

nano::account const & nano::block::account () const
{
static nano::account account{ 0 };
static nano::account account{};
return account;
}

Expand Down Expand Up @@ -363,6 +364,8 @@ nano::send_block::send_block (nano::block_hash const & previous_a, nano::account
signature (nano::sign_message (prv_a, pub_a, hash ())),
work (work_a)
{
debug_assert (destination_a != nullptr);
debug_assert (pub_a != nullptr);
}

nano::send_block::send_block (bool & error_a, nano::stream & stream_a) :
Expand Down Expand Up @@ -524,13 +527,18 @@ nano::open_block::open_block (nano::block_hash const & source_a, nano::account c
signature (nano::sign_message (prv_a, pub_a, hash ())),
work (work_a)
{
debug_assert (!representative_a.is_zero ());
debug_assert (representative_a != nullptr);
debug_assert (account_a != nullptr);
debug_assert (pub_a != nullptr);
}

nano::open_block::open_block (nano::block_hash const & source_a, nano::account const & representative_a, nano::account const & account_a, std::nullptr_t) :
hashables (source_a, representative_a, account_a),
work (0)
{
debug_assert (representative_a != nullptr);
debug_assert (account_a != nullptr);

signature.clear ();
}

Expand Down Expand Up @@ -787,6 +795,8 @@ nano::change_block::change_block (nano::block_hash const & previous_a, nano::acc
signature (nano::sign_message (prv_a, pub_a, hash ())),
work (work_a)
{
debug_assert (representative_a != nullptr);
debug_assert (pub_a != nullptr);
}

nano::change_block::change_block (bool & error_a, nano::stream & stream_a) :
Expand Down Expand Up @@ -1060,6 +1070,10 @@ nano::state_block::state_block (nano::account const & account_a, nano::block_has
signature (nano::sign_message (prv_a, pub_a, hash ())),
work (work_a)
{
debug_assert (account_a != nullptr);
debug_assert (representative_a != nullptr);
debug_assert (link_a.as_account () != nullptr);
debug_assert (pub_a != nullptr);
}

nano::state_block::state_block (bool & error_a, nano::stream & stream_a) :
Expand Down Expand Up @@ -1505,6 +1519,7 @@ nano::receive_block::receive_block (nano::block_hash const & previous_a, nano::b
signature (nano::sign_message (prv_a, pub_a, hash ())),
work (work_a)
{
debug_assert (pub_a != nullptr);
}

nano::receive_block::receive_block (bool & error_a, nano::stream & stream_a) :
Expand Down
2 changes: 1 addition & 1 deletion nano/lib/blocks.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class block_sideband final
bool deserialize (nano::stream &, nano::block_type);
static size_t size (nano::block_type);
nano::block_hash successor{ 0 };
nano::account account{ 0 };
nano::account account{};
nano::amount balance{ 0 };
uint64_t height{ 0 };
uint64_t timestamp{ 0 };
Expand Down
26 changes: 26 additions & 0 deletions nano/lib/numbers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <nano/crypto_lib/secure_memory.hpp>
#include <nano/lib/numbers.hpp>
#include <nano/lib/utility.hpp>
#include <nano/secure/common.hpp>

#include <crypto/cryptopp/aes.h>
#include <crypto/cryptopp/modes.h>
Expand Down Expand Up @@ -61,6 +62,16 @@ std::string nano::public_key::to_account () const
return result;
}

nano::public_key::public_key () :
uint256_union{ 0 }
{
}

const nano::public_key & nano::public_key::null ()
{
return nano::hardened_constants::get ().not_an_account;
}

std::string nano::public_key::to_node_id () const
{
return to_account ().replace (0, 4, "node");
Expand Down Expand Up @@ -781,6 +792,11 @@ std::string nano::uint128_union::to_string_dec () const
return result;
}

nano::hash_or_account::hash_or_account () :
account{}
{
}

nano::hash_or_account::hash_or_account (uint64_t value_a) :
raw (value_a)
{
Expand Down Expand Up @@ -939,6 +955,16 @@ nano::public_key::operator nano::hash_or_account const & () const
return reinterpret_cast<nano::hash_or_account const &> (*this);
}

bool nano::public_key::operator== (std::nullptr_t) const
{
return bytes == null ().bytes;
}

bool nano::public_key::operator!= (std::nullptr_t) const
{
return !(*this == nullptr);
}

nano::block_hash::operator nano::link const & () const
{
return reinterpret_cast<nano::link const &> (*this);
Expand Down
10 changes: 9 additions & 1 deletion nano/lib/numbers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ class public_key final : public uint256_union
public:
using uint256_union::uint256_union;

public_key ();

static const public_key & null ();

std::string to_node_id () const;
bool decode_node_id (std::string const & source_a);
void encode_account (std::string &) const;
Expand All @@ -124,6 +128,10 @@ class public_key final : public uint256_union
operator nano::link const & () const;
operator nano::root const & () const;
operator nano::hash_or_account const & () const;
bool operator== (std::nullptr_t) const;
bool operator!= (std::nullptr_t) const;
using uint256_union::operator==;
using uint256_union::operator!=;
};

class wallet_id : public uint256_union
Expand All @@ -137,7 +145,7 @@ using account = public_key;
class hash_or_account
{
public:
hash_or_account () = default;
hash_or_account ();
hash_or_account (uint64_t value_a);

bool is_zero () const;
Expand Down
2 changes: 1 addition & 1 deletion nano/lib/walletconfig.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ class wallet_config final
nano::error serialize_toml (nano::tomlconfig & toml_a) const;
nano::error deserialize_toml (nano::tomlconfig & toml_a);
nano::wallet_id wallet;
nano::account account{ 0 };
nano::account account{};
};
}
4 changes: 2 additions & 2 deletions nano/nano_node/entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1459,7 +1459,7 @@ int main (int argc, char * const * argv)
}
}
uint64_t previous_timestamp (0);
nano::account calculated_representative (0);
nano::account calculated_representative{};
while (!hash.is_zero () && block != nullptr)
{
++block_count;
Expand Down Expand Up @@ -1704,7 +1704,7 @@ int main (int argc, char * const * argv)
else
{
// Check if pending destination is correct
nano::account destination (0);
nano::account destination{};
bool previous_pruned = node->ledger.pruning && node->store.pruned.exists (transaction, block->previous ());
if (previous_pruned)
{
Expand Down
8 changes: 4 additions & 4 deletions nano/node/active_transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,11 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::b
{
if (election_status_type == nano::election_status_type::inactive_confirmation_height)
{
nano::account account (0);
nano::account account{};
nano::uint128_t amount (0);
bool is_state_send (false);
bool is_state_epoch (false);
nano::account pending_account (0);
nano::account pending_account{};
node.process_confirmed_data (transaction, block_a, block_a->hash (), account, amount, is_state_send, is_state_epoch, pending_account);
node.observers.blocks.notify (nano::election_status{ block_a, 0, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::zero (), 0, 1, 0, nano::election_status_type::inactive_confirmation_height }, {}, account, amount, is_state_send, is_state_epoch);
}
Expand All @@ -206,11 +206,11 @@ void nano::active_transactions::block_cemented_callback (std::shared_ptr<nano::b
add_recently_cemented (status_l);
auto destination (block_a->link ().is_zero () ? block_a->destination () : block_a->link ().as_account ());
node.receive_confirmed (transaction, hash, destination);
nano::account account (0);
nano::account account{};
nano::uint128_t amount (0);
bool is_state_send (false);
bool is_state_epoch (false);
nano::account pending_account (0);
nano::account pending_account{};
node.process_confirmed_data (transaction, block_a, hash, account, amount, is_state_send, is_state_epoch, pending_account);
election_lk.lock ();
election->status.type = *election_status_type;
Expand Down
2 changes: 1 addition & 1 deletion nano/node/active_transactions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ class active_transactions final
void confirm_expired_frontiers_pessimistically (nano::transaction const &, uint64_t, uint64_t &);
void frontiers_confirmation (nano::unique_lock<nano::mutex> &);
bool insert_election_from_frontiers_confirmation (std::shared_ptr<nano::block> const &, nano::account const &, nano::uint128_t, nano::election_behavior);
nano::account next_frontier_account{ 0 };
nano::account next_frontier_account{};
std::chrono::steady_clock::time_point next_frontier_check{ std::chrono::steady_clock::now () };
constexpr static size_t max_active_elections_frontier_insertion{ 1000 };
prioritize_num_uncemented priority_wallet_cementable_frontiers;
Expand Down
2 changes: 1 addition & 1 deletion nano/node/bootstrap/bootstrap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class bootstrap_initiator final
explicit bootstrap_initiator (nano::node &);
~bootstrap_initiator ();
void bootstrap (nano::endpoint const &, bool add_to_peers = true, std::string id_a = "");
void bootstrap (bool force = false, std::string id_a = "", uint32_t const frontiers_age_a = std::numeric_limits<uint32_t>::max (), nano::account const & start_account_a = nano::account (0));
void bootstrap (bool force = false, std::string id_a = "", uint32_t const frontiers_age_a = std::numeric_limits<uint32_t>::max (), nano::account const & start_account_a = nano::account{});
bool bootstrap_lazy (nano::hash_or_account const &, bool force = false, bool confirmed = true, std::string id_a = "");
void bootstrap_wallet (std::deque<nano::account> &);
void run_bootstrap ();
Expand Down
Loading

0 comments on commit 1eefcaf

Please sign in to comment.