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

cuprated: config & args #304

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

cuprated: config & args #304

wants to merge 25 commits into from

Conversation

Boog900
Copy link
Member

@Boog900 Boog900 commented Oct 6, 2024

What

Starts the config and arguments to cuprated.

@github-actions github-actions bot added A-p2p Related to P2P. A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-workspace Changes to a root workspace file or general repo file. A-helper Related to cuprate-helper. A-binaries Related to binaries. labels Oct 6, 2024
@github-actions github-actions bot added A-storage Related to storage. A-net Related to networking. labels Oct 26, 2024
Copy link
Member Author

@Boog900 Boog900 left a comment

Choose a reason for hiding this comment

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

Example generated config file:

#     ____                      _
#    / ___|   _ _ __  _ __ __ _| |_ ___
#   | |  | | | | '_ \| '__/ _` | __/ _ \
#   | |__| |_| | |_) | | | (_| | ||  __/
#    \____\__,_| .__/|_|  \__,_|\__\___|
#              |_|
#

## The network to run on, valid values: "Mainnet", "Testnet", "Stagenet".
network = "Mainnet"

## Tracing config.
[tracing]
## The minimum level for log events to be displayed.
level = "info"

## Clear-net config.
[p2p.clear_net]
## The number of outbound connections we should make and maintain.
outbound_connections = 64
## The number of extra connections we should make under load from the rest of Cuprate, i.e. when syncing.
extra_outbound_connections = 8
## The maximum number of incoming we should allow.
max_inbound_connections = 128
## The percent of outbound connections that should be to nodes we have not connected to before.
gray_peers_percent = 0.7
## The port to accept connections on, if left `0` no connections will be accepted.
p2p_port = 0
## The IP address to listen to connections on.
server.ip = "0.0.0.0"

## The Clear-net addressbook config.
[p2p.clear_net.address_book_config]
## The size of the white peer list, which contains peers we have made a connection to before.
max_white_list_length = 1_000
## The size of the gray peer list, which contains peers we have not made a connection to before.
max_gray_list_length = 5_000
## The folder to store the address book.
peer_store_folder = "/home/user/.cache/cuprate"
## The amount of time between address book saves.
peer_save_period = { secs = 90, nanos = 0 }

## The block downloader config.
[p2p.block_downloader]
## The size of the buffer of sequential blocks waiting to be verified and added to the chain (bytes).
buffer_size = 50_000_000
## The size of the queue of blocks which are waiting for a parent block to be downloaded (bytes).
in_progress_queue_size = 50_000_000
## The target size of a batch of blocks (bytes), must not exceed 100MB.
target_batch_size = 5_000_000
## The number of blocks in the first bacth (you probably shouldn't change this).
initial_batch_len = 1
## The amount of time between checking the pool of connected peers for free peers to download blocks.
check_client_pool_interval = { secs = 30, nanos = 0 }

## Storage config
[storage]
## The amount of reader threads to spawn.
reader_threads = "OnePerThread"

## Txpool storage config.
[storage.txpool]
## The txpool storage location.
path = "/home/user/.local/share/cuprate/txpool"
## The database sync mode for the txpool.
sync_mode = "Async"
## The maximum size of all the txs in the pool (bytes).
max_txpool_size = 100_000_000

## Blockchain storage config.
[storage.blockchain]
## The blockchain storage location.
path = "/home/user/.local/share/cuprate/blockchain"
## The database sync mode for the blockchain.
sync_mode = "Async"

binaries/cuprated/Cargo.toml Show resolved Hide resolved
binaries/cuprated/src/config.rs Show resolved Hide resolved
binaries/cuprated/src/config.rs Show resolved Hide resolved
binaries/cuprated/src/config/Cuprate.toml Outdated Show resolved Hide resolved
binaries/cuprated/src/config/args.rs Show resolved Hide resolved
binaries/cuprated/src/config/default.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the A-net Related to networking. label Oct 26, 2024
@Boog900 Boog900 marked this pull request as ready for review October 26, 2024 20:26
helper/src/network.rs Outdated Show resolved Hide resolved
p2p/address-book/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@hinto-janai hinto-janai left a comment

Choose a reason for hiding this comment

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

nits only, will leave other review

p2p/address-book/src/book/tests.rs Outdated Show resolved Hide resolved
p2p/address-book/src/lib.rs Outdated Show resolved Hide resolved
p2p/address-book/src/lib.rs Outdated Show resolved Hide resolved
p2p/address-book/src/store.rs Outdated Show resolved Hide resolved
p2p/address-book/src/store.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config/Cuprate.toml Outdated Show resolved Hide resolved
binaries/cuprated/src/config/p2p.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
binaries/cuprated/src/config.rs Outdated Show resolved Hide resolved
@@ -5,6 +5,11 @@
//! into it's own crate.
//!
//! `#[no_std]` compatible.
// TODO: move to types crate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to cuprate_types? strum can automatically implement the FromStr/Display and lots of other stuff.

Also please add a doc-test asserting the string representations since we rely on it for PATHs now, something like this:

/// ## Serialization and string formatting
/// ```rust
/// use cuprate_rpc_types::{
/// misc::Status,
/// CORE_RPC_STATUS_BUSY, CORE_RPC_STATUS_NOT_MINING, CORE_RPC_STATUS_OK,
/// CORE_RPC_STATUS_PAYMENT_REQUIRED
/// };
/// use serde_json::to_string;
///
/// let other = Status::Other("OTHER".into());
///
/// assert_eq!(to_string(&Status::Ok).unwrap(), r#""OK""#);
/// assert_eq!(to_string(&Status::Busy).unwrap(), r#""BUSY""#);
/// assert_eq!(to_string(&Status::NotMining).unwrap(), r#""NOT MINING""#);
/// assert_eq!(to_string(&Status::PaymentRequired).unwrap(), r#""PAYMENT REQUIRED""#);
/// assert_eq!(to_string(&other).unwrap(), r#""OTHER""#);
///
/// assert_eq!(Status::Ok.as_ref(), CORE_RPC_STATUS_OK);
/// assert_eq!(Status::Busy.as_ref(), CORE_RPC_STATUS_BUSY);
/// assert_eq!(Status::NotMining.as_ref(), CORE_RPC_STATUS_NOT_MINING);
/// assert_eq!(Status::PaymentRequired.as_ref(), CORE_RPC_STATUS_PAYMENT_REQUIRED);
/// assert_eq!(other.as_ref(), "OTHER");
///
/// assert_eq!(format!("{}", Status::Ok), CORE_RPC_STATUS_OK);
/// assert_eq!(format!("{}", Status::Busy), CORE_RPC_STATUS_BUSY);
/// assert_eq!(format!("{}", Status::NotMining), CORE_RPC_STATUS_NOT_MINING);
/// assert_eq!(format!("{}", Status::PaymentRequired), CORE_RPC_STATUS_PAYMENT_REQUIRED);
/// assert_eq!(format!("{}", other), "OTHER");
///
/// assert_eq!(format!("{:?}", Status::Ok), "Ok");
/// assert_eq!(format!("{:?}", Status::Busy), "Busy");
/// assert_eq!(format!("{:?}", Status::NotMining), "NotMining");
/// assert_eq!(format!("{:?}", Status::PaymentRequired), "PaymentRequired");
/// assert_eq!(format!("{:?}", other), r#"Other("OTHER")"#);
/// ```

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in another PR this is used in a lot of places so don't really want to do it here.

binaries/cuprated/src/config/storage.rs Outdated Show resolved Hide resolved
storage/blockchain/src/config.rs Outdated Show resolved Hide resolved
helper/src/fs.rs Outdated Show resolved Hide resolved
storage/txpool/src/config.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot added the A-docs Related to documentation. label Nov 10, 2024
Copy link
Contributor

@hinto-janai hinto-janai left a comment

Choose a reason for hiding this comment

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

1 thing + 2 nits, there's other nits too but this needs to be merged and I don't want to hold it up any longer.

Comment on lines +38 to +39
// This will create the config file and exit.
create_default_config_file(config_folder)
Copy link
Contributor

Choose a reason for hiding this comment

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

A few notes:

  1. Programs often output this type of stuff to std{out,err} such that users can redirect (program > output.txt, program | other_program) 1 2 3 although it has to be correctly handled when logging gets involved (op-reth config --default > config.txt gets it wrong and includes a log line... lol).

  2. With initial CLI control flow / logging, I think they should be in the same local area for clarity, in this case, I think they should be in apply_args, called functions contain only logic.

  3. Considering 1/2 and since create_default_config_file won't be used elsewhere, binaries/cuprated/src/config/default.rs could be removed, the test in it could goto src/config/constants.rs.

Suggested change
// This will create the config file and exit.
create_default_config_file(config_folder)
println!("{EXAMPLE_CONFIG}");
exit(0);

Footnotes

  1. https://doc.rust-lang.org/book/ch12-06-writing-to-stderr-instead-of-stdout.html#checking-where-errors-are-written

  2. https://reth.rs/cli/reth/config.html

  3. https://nginx.org/en/docs/switches.html

Comment on lines +24 to +32
/// The size in bytes of the buffer between the block downloader and the place which
/// is consuming the downloaded blocks.
pub buffer_size: usize,
/// The size of the in progress queue (in bytes) at which we stop requesting more blocks.
pub in_progress_queue_size: usize,
/// The [`Duration`] between checking the client pool for free peers.
pub check_client_pool_interval: Duration,
/// The target size of a single batch of blocks (in bytes).
pub target_batch_size: usize,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think _bytes over _size on these variables would make things more obvious but this can be done later.

/// assert_eq!(addressbook_path(&**CUPRATE_CACHE_DIR, Network::Stagenet).as_path(), CUPRATE_CACHE_DIR.join(Network::Stagenet.to_string()).join("addressbook"));
/// assert_eq!(addressbook_path(&**CUPRATE_CACHE_DIR, Network::Testnet).as_path(), CUPRATE_CACHE_DIR.join(Network::Testnet.to_string()).join("addressbook"));
/// ```
pub fn addressbook_path(cache_dir: &Path, network: Network) -> PathBuf {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is AddressBook/address_book in all other code.

Suggested change
pub fn addressbook_path(cache_dir: &Path, network: Network) -> PathBuf {
pub fn address_book_path(cache_dir: &Path, network: Network) -> PathBuf {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-binaries Related to binaries. A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-docs Related to documentation. A-helper Related to cuprate-helper. A-p2p Related to P2P. A-storage Related to storage. A-workspace Changes to a root workspace file or general repo file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants