Skip to content

Conversation

fpelliccioni
Copy link
Contributor

No description provided.

@fpelliccioni fpelliccioni force-pushed the feat/domain-obj-deserialization branch 8 times, most recently from e24333c to 9e431d6 Compare October 24, 2024 14:56
@fpelliccioni fpelliccioni force-pushed the feat/domain-obj-deserialization branch from 9e431d6 to a5c4021 Compare October 27, 2024 20:31
@fpelliccioni fpelliccioni force-pushed the feat/domain-obj-deserialization branch from a5c4021 to 69e390f Compare April 30, 2025 14:03
@fpelliccioni fpelliccioni force-pushed the feat/domain-obj-deserialization branch from 69e390f to cc6ded2 Compare April 30, 2025 22:52
@fpelliccioni fpelliccioni changed the title feat: refactor domain objects deserialization feat: utxo set cache Apr 30, 2025
@fpelliccioni fpelliccioni requested a review from Copilot April 30, 2025 22:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a UTXO set cache along with changes to database and block caching settings. The key changes are:

  • Increasing default database size constants for mainnet.
  • Adding a new block_cache_capacity setting and integrating a block cache in the database.
  • Updating UTXO-related functions and refactoring internal database operations.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/settings.cpp Updated database size constants and added block_cache_capacity.
src/data_base.cpp Introduced block cache initialization and duplicate cache flush calls in close().
include/kth/database/databases/utxo_database.ipp Renamed insert_utxo to insert_utxo_with_fixed_data for clarity.
include/kth/database/databases/internal_database.* Switched from std::unordered_map/set to boost::unordered_flat_map/set for UTXO handling.
include/kth/database/data_base.hpp Added block_cache_t implementation to support the new UTXO cache.
Comments suppressed due to low confidence (1)

include/kth/database/databases/utxo_database.ipp:37

  • Ensure that the new function name insert_utxo_with_fixed_data is used consistently throughout the project and update references accordingly.
result_code internal_database_basis<Clock>::insert_utxo_with_fixed_data(domain::chain::output_point const& point, domain::chain::output const& output, data_chunk const& fixed_data, KTH_DB_txn* db_txn) {

, reorg_pool_limit(100) //TODO(fernando): look for a good default
, db_max_size(get_db_max_size_mainnet(db_mode))
, safe_mode(true)
, cache_capacity(0)
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding a comment to explain the role of block_cache_capacity and the rationale behind its default value.

Suggested change
, cache_capacity(0)
, cache_capacity(0)
// Specifies the maximum number of blocks that can be cached in memory.
// The default value of 100 is chosen as a balance between memory usage and performance.

Copilot uses AI. Check for mistakes.

Comment on lines +102 to +105

LOG_INFO(LOG_DATABASE, "Flushing the block cache to DB (again)...");
block_cache_->flush_to_db();
LOG_INFO(LOG_DATABASE, "Block cache flushed to DB.");
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate call to flush_to_db() in the close() method may result in unnecessary double flushing. Confirm if this is intended or remove the redundant call.

Suggested change
LOG_INFO(LOG_DATABASE, "Flushing the block cache to DB (again)...");
block_cache_->flush_to_db();
LOG_INFO(LOG_DATABASE, "Block cache flushed to DB.");

Copilot uses AI. Check for mistakes.

// bool is_stale
// );

db_->push_blocks_and_utxos(blocks_, start_height_, utxo_map_, utxo_to_remove_, is_stale());
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider capturing and handling the return result of push_blocks_and_utxos to log potential issues and ensure robust error handling during block cache flush operations.

Suggested change
db_->push_blocks_and_utxos(blocks_, start_height_, utxo_map_, utxo_to_remove_, is_stale());
auto result = db_->push_blocks_and_utxos(blocks_, start_height_, utxo_map_, utxo_to_remove_, is_stale());
if (result != result_code::success) {
LOG_ERROR(LOG_DATABASE, "Failed to flush block cache to database. Error code: ", result);
}

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant