Replace hook virtual calls with std::function

- Instead of inheriting from a pure virtual base class, we know just use
  lambdas for hook callbacks.
- The signature of hooks also changes to take an info struct rather than
  a list of parameters, so that you don't have to change everything to
  ignore unwanted parameters if someone adds something to a hook.
This commit is contained in:
Jason Rhinelander 2022-07-27 14:20:29 -03:00
parent f6080918b4
commit f382c1971e
No known key found for this signature in database
GPG key ID: C4992CE7A88D4262
13 changed files with 113 additions and 116 deletions

View file

@ -167,10 +167,10 @@ namespace cryptonote
return result;
}
//---------------------------------------------------------------------------
bool checkpoints::block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, checkpoint_t const *checkpoint)
bool checkpoints::block_added(const block_added_info& info)
{
uint64_t const height = get_block_height(block);
if (height < service_nodes::CHECKPOINT_STORE_PERSISTENTLY_INTERVAL || block.major_version < hf::hf12_checkpointing)
uint64_t const height = get_block_height(info.block);
if (height < service_nodes::CHECKPOINT_STORE_PERSISTENTLY_INTERVAL || info.block.major_version < hf::hf12_checkpointing)
return true;
uint64_t end_cull_height = 0;
@ -203,13 +203,13 @@ namespace cryptonote
}
}
if (checkpoint)
update_checkpoint(*checkpoint);
if (info.checkpoint)
update_checkpoint(*info.checkpoint);
return true;
}
//---------------------------------------------------------------------------
void checkpoints::blockchain_detached(uint64_t height, bool /*by_pop_blocks*/)
void checkpoints::blockchain_detached(uint64_t height)
{
m_last_cull_height = std::min(m_last_cull_height, height);

View file

@ -111,12 +111,10 @@ namespace cryptonote
* either from a json file or via DNS from a checkpoint-hosting server.
*/
class checkpoints
: public cryptonote::BlockAddedHook,
public cryptonote::BlockchainDetachedHook
{
public:
bool block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, checkpoint_t const *checkpoint) override;
void blockchain_detached(uint64_t height, bool by_pop_blocks) override;
bool block_added(const block_added_info& info);
void blockchain_detached(uint64_t height);
bool get_checkpoint(uint64_t height, checkpoint_t &checkpoint) const;
/**

View file

@ -36,23 +36,27 @@
namespace cryptonote {
class BlockAddedHook
{
public:
virtual bool block_added(const block& block, const std::vector<transaction>& txs, struct checkpoint_t const *checkpoint) = 0;
struct checkpoint_t;
struct block_added_info {
const cryptonote::block& block;
const std::vector<transaction>& txs;
const checkpoint_t* const checkpoint;
};
class BlockchainDetachedHook
{
public:
virtual void blockchain_detached(uint64_t height, bool by_pop_blocks) = 0;
using BlockAddedHook = std::function<bool(const block_added_info& info)>;
struct detached_info {
uint64_t height;
bool by_pop_blocks;
};
class InitHook
{
public:
virtual void init() = 0;
using BlockchainDetachedHook = std::function<void(const detached_info& info)>;
using InitHook = std::function<void()>;
struct batch_sn_payment;
struct block_reward_parts;
struct miner_tx_info {
const cryptonote::block& block;
const block_reward_parts& reward_parts;
const std::vector<cryptonote::batch_sn_payment>& batched_sn_payments;
};
using ValidateMinerTxHook = std::function<bool(const miner_tx_info& info)>;
struct address_parse_info
{
@ -76,18 +80,6 @@ namespace cryptonote {
: address_info{addr, 0}, amount{amt} {}
};
class ValidateMinerTxHook
{
public:
virtual bool validate_miner_tx(cryptonote::block const &block, struct block_reward_parts const &reward_parts, std::optional<std::vector<cryptonote::batch_sn_payment>> const &batched_sn_payments) const = 0;
};
class AltBlockAddedHook
{
public:
virtual bool alt_block_added(const block &block, const std::vector<transaction>& txs, struct checkpoint_t const *checkpoint) = 0;
};
#pragma pack(push, 1)
struct public_address_outer_blob
{

View file

@ -318,7 +318,7 @@ bool Blockchain::load_missing_blocks_into_oxen_subsystems()
// If the batching database falls behind it NEEDS the service node list information at that point in time
if (sqlite_height < snl_height)
{
m_service_node_list.blockchain_detached(sqlite_height, true);
m_service_node_list.blockchain_detached(sqlite_height);
snl_height = std::min(sqlite_height, m_service_node_list.height()) + 1;
}
start_height_options.push_back(snl_height);
@ -604,10 +604,10 @@ bool Blockchain::init(BlockchainDB* db, sqlite3 *ons_db, std::shared_ptr<crypton
}
hook_block_added(m_checkpoints);
hook_blockchain_detached(m_checkpoints);
for (InitHook* hook : m_init_hooks)
hook->init();
hook_block_added([this] (const auto& info) { return m_checkpoints.block_added(info); });
hook_blockchain_detached([this] (const auto& info) { m_checkpoints.blockchain_detached(info.height); });
for (const auto& hook : m_init_hooks)
hook();
if (!m_db->is_read_only() && !load_missing_blocks_into_oxen_subsystems())
{
@ -723,9 +723,9 @@ void Blockchain::pop_blocks(uint64_t nblocks)
return;
}
auto split_height = m_db->height();
for (BlockchainDetachedHook* hook : m_blockchain_detached_hooks)
hook->blockchain_detached(split_height, true /*by_pop_blocks*/);
detached_info hook_data{m_db->height(), /*by_pop_blocks=*/true};
for (const auto& hook : m_blockchain_detached_hooks)
hook(hook_data);
if (!pop_batching_rewards)
m_service_node_list.reset_batching_to_latest_height();
load_missing_blocks_into_oxen_subsystems();
@ -824,8 +824,8 @@ bool Blockchain::reset_and_set_genesis_block(const block& b)
m_db->reset();
m_db->drop_alt_blocks();
for (InitHook* hook : m_init_hooks)
hook->init();
for (const auto& hook : m_init_hooks)
hook();
db_wtxn_guard wtxn_guard(m_db);
block_verification_context bvc{};
@ -1051,8 +1051,9 @@ bool Blockchain::rollback_blockchain_switching(const std::list<block_and_checkpo
}
// Revert all changes from switching to the alt chain before adding the original chain back in
for (BlockchainDetachedHook* hook : m_blockchain_detached_hooks)
hook->blockchain_detached(rollback_height, false /*by_pop_blocks*/);
detached_info rollback_hook_data{rollback_height, /*by_pop_blocks=*/false};
for (const auto& hook : m_blockchain_detached_hooks)
hook(rollback_hook_data);
load_missing_blocks_into_oxen_subsystems();
//return back original chain
@ -1113,8 +1114,9 @@ bool Blockchain::switch_to_alternative_blockchain(const std::list<block_extended
}
auto split_height = m_db->height();
for (BlockchainDetachedHook* hook : m_blockchain_detached_hooks)
hook->blockchain_detached(split_height, false /*by_pop_blocks*/);
detached_info split_hook_data{split_height, /*by_pop_blocks=*/false};
for (const auto& hook : m_blockchain_detached_hooks)
hook(split_hook_data);
load_missing_blocks_into_oxen_subsystems();
//connecting new alternative chain
@ -1379,10 +1381,10 @@ bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_bl
if (m_nettype != network_type::FAKECHAIN)
throw std::logic_error("Blockchain missing SQLite Database");
}
cryptonote::block bl = b;
for (ValidateMinerTxHook* hook : m_validate_miner_tx_hooks)
miner_tx_info hook_data{b, reward_parts, batched_sn_payments};
for (const auto& hook : m_validate_miner_tx_hooks)
{
if (!hook->validate_miner_tx(b, reward_parts, batched_sn_payments))
if (!hook(hook_data))
return false;
}
@ -2095,9 +2097,10 @@ bool Blockchain::handle_alternative_block(const block& b, const crypto::hash& id
txs.push_back(tx);
}
for (AltBlockAddedHook *hook : m_alt_block_added_hooks)
block_added_info hook_data{b, txs, checkpoint};
for (const auto& hook : m_alt_block_added_hooks)
{
if (!hook->alt_block_added(b, txs, checkpoint))
if (!hook(hook_data))
return false;
}
}
@ -4502,9 +4505,9 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash&
auto abort_block = oxen::defer([&]() {
pop_block_from_blockchain();
auto old_height = m_db->height();
for (BlockchainDetachedHook* hook : m_blockchain_detached_hooks)
hook->blockchain_detached(old_height, false /*by_pop_blocks*/);
detached_info hook_data{m_db->height(), false /*by_pop_blocks*/};
for (const auto& hook : m_blockchain_detached_hooks)
hook(hook_data);
});
// TODO(oxen): Not nice, making the hook take in a vector of pair<transaction,
@ -4545,9 +4548,10 @@ bool Blockchain::handle_block_to_main_chain(const block& bl, const crypto::hash&
throw std::logic_error("Blockchain missing SQLite Database");
}
for (BlockAddedHook* hook : m_block_added_hooks)
block_added_info hook_data{bl, only_txs, checkpoint};
for (const auto& hook : m_block_added_hooks)
{
if (!hook->block_added(bl, only_txs, checkpoint))
if (!hook(hook_data))
{
MGINFO_RED("Block added hook signalled failure");
bvc.m_verifivation_failed = true;

View file

@ -979,14 +979,22 @@ namespace cryptonote
/**
* @brief add a hook for processing new blocks and rollbacks for reorgs
*
* TODO: replace these with more versatile std::functions
*/
void hook_block_added (BlockAddedHook& hook) { m_block_added_hooks.push_back(&hook); }
void hook_blockchain_detached(BlockchainDetachedHook& hook) { m_blockchain_detached_hooks.push_back(&hook); }
void hook_init (InitHook& hook) { m_init_hooks.push_back(&hook); }
void hook_validate_miner_tx (ValidateMinerTxHook& hook) { m_validate_miner_tx_hooks.push_back(&hook); }
void hook_alt_block_added (AltBlockAddedHook& hook) { m_alt_block_added_hooks.push_back(&hook); }
void hook_block_added(BlockAddedHook hook) {
m_block_added_hooks.push_back(std::move(hook));
}
void hook_blockchain_detached(BlockchainDetachedHook hook) {
m_blockchain_detached_hooks.push_back(std::move(hook));
}
void hook_init(InitHook hook) {
m_init_hooks.push_back(std::move(hook));
}
void hook_validate_miner_tx(ValidateMinerTxHook hook) {
m_validate_miner_tx_hooks.push_back(std::move(hook));
}
void hook_alt_block_added(BlockAddedHook hook) {
m_alt_block_added_hooks.push_back(std::move(hook));
}
/**
* @brief returns the timestamps of the last N blocks
@ -1119,11 +1127,11 @@ namespace cryptonote
// some invalid blocks
std::set<crypto::hash> m_invalid_blocks;
std::vector<BlockAddedHook*> m_block_added_hooks;
std::vector<BlockchainDetachedHook*> m_blockchain_detached_hooks;
std::vector<InitHook*> m_init_hooks;
std::vector<ValidateMinerTxHook*> m_validate_miner_tx_hooks;
std::vector<AltBlockAddedHook*> m_alt_block_added_hooks;
std::vector<BlockAddedHook> m_block_added_hooks;
std::vector<BlockAddedHook> m_alt_block_added_hooks;
std::vector<BlockchainDetachedHook> m_blockchain_detached_hooks;
std::vector<InitHook> m_init_hooks;
std::vector<ValidateMinerTxHook> m_validate_miner_tx_hooks;
checkpoints m_checkpoints;

View file

@ -768,15 +768,15 @@ namespace cryptonote
m_service_node_list.set_quorum_history_storage(command_line::get_arg(vm, arg_store_quorum_history));
// NOTE: Implicit dependency. Service node list needs to be hooked before checkpoints.
m_blockchain_storage.hook_blockchain_detached(m_service_node_list);
m_blockchain_storage.hook_init(m_service_node_list);
m_blockchain_storage.hook_validate_miner_tx(m_service_node_list);
m_blockchain_storage.hook_alt_block_added(m_service_node_list);
m_blockchain_storage.hook_blockchain_detached([this] (const auto& info) { return m_service_node_list.blockchain_detached(info.height); });
m_blockchain_storage.hook_init([this] { m_service_node_list.init(); });
m_blockchain_storage.hook_validate_miner_tx([this] (const auto& info) { return m_service_node_list.validate_miner_tx(info); });
m_blockchain_storage.hook_alt_block_added([this] (const auto& info) { return m_service_node_list.alt_block_added(info); });
// NOTE: There is an implicit dependency on service node lists being hooked first!
m_blockchain_storage.hook_init(m_quorum_cop);
m_blockchain_storage.hook_block_added(m_quorum_cop);
m_blockchain_storage.hook_blockchain_detached(m_quorum_cop);
m_blockchain_storage.hook_init([this] { m_quorum_cop.init(); });
m_blockchain_storage.hook_block_added([this] (const auto& info) { return m_quorum_cop.block_added(info.block, info.txs); });
m_blockchain_storage.hook_blockchain_detached([this] (const auto& info) { m_quorum_cop.blockchain_detached(info.height, info.by_pop_blocks); });
}
// Checkpoints

View file

@ -2292,7 +2292,7 @@ namespace service_nodes
m_state.update_from_block(m_blockchain.get_db(), nettype, m_transient.state_history, m_transient.state_archive, {}, block, txs, m_service_node_keys);
}
void service_node_list::blockchain_detached(uint64_t height, bool /*by_pop_blocks*/)
void service_node_list::blockchain_detached(uint64_t height)
{
std::lock_guard lock(m_sn_mutex);
@ -2492,8 +2492,11 @@ namespace service_nodes
return true;
}
bool service_node_list::validate_miner_tx(const cryptonote::block& block, const cryptonote::block_reward_parts& reward_parts, const std::optional<std::vector<cryptonote::batch_sn_payment>>& batched_sn_payments) const
bool service_node_list::validate_miner_tx(const cryptonote::miner_tx_info& info) const
{
const auto& block = info.block;
const auto& reward_parts = info.reward_parts;
const auto& batched_sn_payments = info.batched_sn_payments;
const auto hf_version = block.major_version;
if (hf_version < hf::hf9_service_nodes)
return true;
@ -2578,7 +2581,7 @@ namespace service_nodes
size_t expected_vouts_size;
switch (mode) {
case verify_mode::batched_sn_rewards:
expected_vouts_size = batched_sn_payments ? batched_sn_payments->size() : 0;
expected_vouts_size = batched_sn_payments.size();
break;
case verify_mode::pulse_block_leader_is_producer:
case verify_mode::pulse_different_block_producer:
@ -2709,18 +2712,18 @@ namespace service_nodes
case verify_mode::batched_sn_rewards:
{
// NB: this amount is in milli-atomics, not atomics
uint64_t total_payout_in_our_db = batched_sn_payments ? std::accumulate(
batched_sn_payments->begin(),
batched_sn_payments->end(),
uint64_t total_payout_in_our_db = std::accumulate(
batched_sn_payments.begin(),
batched_sn_payments.end(),
uint64_t{0},
[](auto&& a, auto&& b) { return a + b.amount; }) : 0;
[](auto&& a, auto&& b) { return a + b.amount; });
uint64_t total_payout_in_vouts = 0;
const auto deterministic_keypair = cryptonote::get_deterministic_keypair_from_height(height);
for (size_t vout_index = 0; vout_index < block.miner_tx.vout.size(); vout_index++)
{
const auto& vout = block.miner_tx.vout[vout_index];
const auto& batch_payment = (*batched_sn_payments)[vout_index];
const auto& batch_payment = batched_sn_payments[vout_index];
if (!std::holds_alternative<cryptonote::txout_to_key>(vout.target))
{
@ -2768,7 +2771,7 @@ namespace service_nodes
return true;
}
bool service_node_list::alt_block_added(cryptonote::block const &block, std::vector<cryptonote::transaction> const &txs, cryptonote::checkpoint_t const *checkpoint)
bool service_node_list::alt_block_added(const cryptonote::block_added_info& info)
{
// NOTE: The premise is to search the main list and the alternative list for
// the parent of the block we just received, generate the new Service Node
@ -2779,6 +2782,7 @@ namespace service_nodes
// store into the alt-chain until it gathers enough blocks to cause
// a reorganization (more checkpoints/PoW than the main chain).
auto& block = info.block;
if (block.major_version < hf::hf9_service_nodes)
return true;
@ -2819,14 +2823,14 @@ namespace service_nodes
// NOTE: Generate the next Service Node list state from this Alt block.
state_t alt_state = *starting_state;
alt_state.update_from_block(m_blockchain.get_db(), m_blockchain.nettype(), m_transient.state_history, m_transient.state_archive, m_transient.alt_state, block, txs, m_service_node_keys);
alt_state.update_from_block(m_blockchain.get_db(), m_blockchain.nettype(), m_transient.state_history, m_transient.state_archive, m_transient.alt_state, block, info.txs, m_service_node_keys);
auto alt_it = m_transient.alt_state.find(block_hash);
if (alt_it != m_transient.alt_state.end())
alt_it->second = std::move(alt_state);
else
m_transient.alt_state.emplace(block_hash, std::move(alt_state));
return verify_block(block, true /*alt_block*/, checkpoint);
return verify_block(block, true /*alt_block*/, info.checkpoint);
}
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

View file

@ -448,11 +448,6 @@ namespace service_nodes
};
class service_node_list
: public cryptonote::BlockAddedHook,
public cryptonote::BlockchainDetachedHook,
public cryptonote::InitHook,
public cryptonote::ValidateMinerTxHook,
public cryptonote::AltBlockAddedHook
{
public:
explicit service_node_list(cryptonote::Blockchain& blockchain);
@ -460,15 +455,15 @@ namespace service_nodes
service_node_list(const service_node_list &) = delete;
service_node_list &operator=(const service_node_list &) = delete;
bool block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, cryptonote::checkpoint_t const *checkpoint) override;
bool block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, const cryptonote::checkpoint_t* checkpoint);
void reset_batching_to_latest_height();
bool state_history_exists(uint64_t height);
bool process_batching_rewards(const cryptonote::block& block);
bool pop_batching_rewards_block(const cryptonote::block& block);
void blockchain_detached(uint64_t height, bool by_pop_blocks) override;
void init() override;
bool validate_miner_tx(const cryptonote::block& block, const cryptonote::block_reward_parts& base_reward, const std::optional<std::vector<cryptonote::batch_sn_payment>>& batched_sn_payments) const override;
bool alt_block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, cryptonote::checkpoint_t const *checkpoint) override;
void blockchain_detached(uint64_t height);
void init();
bool validate_miner_tx(const cryptonote::miner_tx_info& info) const;
bool alt_block_added(const cryptonote::block_added_info& info);
payout get_block_leader() const { std::lock_guard lock{m_sn_mutex}; return m_state.get_block_leader(); }
bool is_service_node(const crypto::public_key& pubkey, bool require_active = true) const;
bool is_key_image_locked(crypto::key_image const &check_image, uint64_t *unlock_height = nullptr, service_node_info::contribution_t *the_locked_contribution = nullptr) const;

View file

@ -510,7 +510,7 @@ namespace service_nodes
}
}
bool quorum_cop::block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, cryptonote::checkpoint_t const * /*checkpoint*/)
bool quorum_cop::block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs)
{
process_quorums(block);
uint64_t const height = cryptonote::get_block_height(block) + 1; // chain height = new top block height + 1

View file

@ -110,16 +110,13 @@ namespace service_nodes
};
class quorum_cop
: public cryptonote::BlockAddedHook,
public cryptonote::BlockchainDetachedHook,
public cryptonote::InitHook
{
public:
explicit quorum_cop(cryptonote::core& core);
void init() override;
bool block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs, cryptonote::checkpoint_t const * /*checkpoint*/) override;
void blockchain_detached(uint64_t height, bool by_pop_blocks) override;
void init();
bool block_added(const cryptonote::block& block, const std::vector<cryptonote::transaction>& txs);
void blockchain_detached(uint64_t height, bool by_pop_blocks);
void set_votes_relayed (std::vector<quorum_vote_t> const &relayed_votes);
std::vector<quorum_vote_t> get_relayable_votes(uint64_t current_height, cryptonote::hf hf_version, bool quorum_relay);

View file

@ -2,6 +2,7 @@
#include "lmq_server.h"
#include "cryptonote_config.h"
#include "oxenmq/oxenmq.h"
#include <fmt/core.h>
#undef OXEN_DEFAULT_LOG_CATEGORY
#define OXEN_DEFAULT_LOG_CATEGORY "daemon.rpc"
@ -314,7 +315,7 @@ omq_rpc::omq_rpc(cryptonote::core& core, core_rpc_server& rpc, const boost::prog
}
});
core_.get_blockchain_storage().hook_block_added(*this);
core_.get_blockchain_storage().hook_block_added([this] (const auto& info) { send_block_notifications(info.block); return true; });
core_.get_pool().add_notify([this](const crypto::hash& id, const transaction& tx, const std::string& blob, const tx_pool_options& opts) {
send_mempool_notifications(id, tx, blob, opts);
});
@ -356,15 +357,13 @@ static void send_notifies(Mutex& mutex, Subs& subs, const char* desc, Call call)
}
}
bool omq_rpc::block_added(const block& block, const std::vector<transaction>& txs, const checkpoint_t *)
void omq_rpc::send_block_notifications(const block& block)
{
auto& omq = core_.get_omq();
std::string height = std::to_string(get_block_height(block));
std::string height = fmt::format("{}", get_block_height(block));
send_notifies(subs_mutex_, block_subs_, "block", [&](auto& conn, auto& sub) {
omq.send(conn, "notify.block", height, std::string_view{block.hash.data, sizeof(block.hash.data)});
});
return true;
}
void omq_rpc::send_mempool_notifications(const crypto::hash& id, const transaction& tx, const std::string& blob, const tx_pool_options& opts)

View file

@ -44,7 +44,7 @@ void init_omq_options(boost::program_options::options_description& desc);
* cryptonote_core--but it works with it to add RPC endpoints, make it listen on RPC ports, and
* handles RPC requests.
*/
class omq_rpc final : public cryptonote::BlockAddedHook {
class omq_rpc final {
enum class mempool_sub_type { all, blink };
struct mempool_sub {
@ -65,7 +65,7 @@ class omq_rpc final : public cryptonote::BlockAddedHook {
public:
omq_rpc(cryptonote::core& core, core_rpc_server& rpc, const boost::program_options::variables_map& vm);
bool block_added(const block& block, const std::vector<transaction>& txs, const checkpoint_t *) override;
void send_block_notifications(const block& block);
void send_mempool_notifications(const crypto::hash& id, const transaction& tx, const std::string& blob, const tx_pool_options& opts);
};

View file

@ -367,7 +367,7 @@ TEST(checkpoints_blockchain_detached, detach_to_checkpoint_height)
test_db->update_block_checkpoint(checkpoint);
// NOTE: Detaching to height. Our top checkpoint should be the 1st checkpoint, we should be deleting the checkpoint at checkpoint.height
cp.blockchain_detached(SECOND_HEIGHT, false /*by_pop_blocks*/);
cp.blockchain_detached(SECOND_HEIGHT);
checkpoint_t top_checkpoint;
ASSERT_TRUE(test_db->get_top_checkpoint(top_checkpoint));
ASSERT_TRUE(top_checkpoint.height == FIRST_HEIGHT);
@ -383,7 +383,7 @@ TEST(checkpoints_blockchain_detached, detach_to_1)
checkpoint.height += service_nodes::CHECKPOINT_INTERVAL;
test_db->update_block_checkpoint(checkpoint);
cp.blockchain_detached(1 /*height*/, false /*by_pop_blocks*/);
cp.blockchain_detached(1 /*height*/);
checkpoint_t top_checkpoint;
ASSERT_FALSE(test_db->get_top_checkpoint(top_checkpoint));
}