Skip to content
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
b4b73ec
add cuprated skeleton
Boog900 Aug 16, 2024
39d48fe
fmt and add deny exception
Boog900 Aug 16, 2024
a018469
add main chain batch handler
Boog900 Aug 20, 2024
f909c26
add blockchain init
Boog900 Aug 21, 2024
f25588d
very rough block manager
Boog900 Aug 23, 2024
1c93ea1
misc changes
Boog900 Aug 29, 2024
05d0cf2
move more config values
Boog900 Aug 29, 2024
d648871
add new tables & types
Boog900 Aug 29, 2024
e1ae848
add function to fully add an alt block
Boog900 Aug 30, 2024
ed887a7
resolve current todo!s
Boog900 Aug 30, 2024
bc619b6
add new requests
Boog900 Aug 31, 2024
029f439
WIP: starting re-orgs
Boog900 Sep 1, 2024
6927b05
add last service request
Boog900 Sep 5, 2024
21e4b3a
commit Cargo.lock
Boog900 Sep 5, 2024
a9d8eee
Merge branch 'main' into storage-alt-blocks
Boog900 Sep 5, 2024
123aedd
add test
Boog900 Sep 6, 2024
ba5c5ac
more docs + cleanup + alt blocks request
Boog900 Sep 7, 2024
f92375f
clippy + fmt
Boog900 Sep 7, 2024
a864f93
document types
Boog900 Sep 7, 2024
6119972
move tx_fee to helper
Boog900 Sep 8, 2024
b211210
more doc updates
Boog900 Sep 8, 2024
68807e7
fmt
Boog900 Sep 8, 2024
c03065b
fix imports
Boog900 Sep 8, 2024
1831fa6
remove config files
Boog900 Sep 9, 2024
20033ee
Merge branch 'main' into cuprated-blockchain
Boog900 Sep 9, 2024
da78cbd
fix merge errors
Boog900 Sep 9, 2024
b5f8475
Merge branch 'storage-alt-blocks' into cuprated-blockchain
Boog900 Sep 9, 2024
d4e0e30
fix generated coins
Boog900 Sep 9, 2024
915633f
handle more p2p requests + alt blocks
Boog900 Sep 12, 2024
01a3065
clean up handler code
Boog900 Sep 12, 2024
6ec5bc3
add function for incoming blocks
Boog900 Sep 12, 2024
90beed1
add docs to handler functions
Boog900 Sep 13, 2024
a16381e
broadcast new blocks + add commands
Boog900 Sep 14, 2024
d2ab8e2
add fluffy block handler
Boog900 Sep 15, 2024
291ffe3
fix new block handling
Boog900 Sep 15, 2024
c0a3f7a
small cleanup
Boog900 Sep 15, 2024
fa54df2
increase outbound peer count
Boog900 Sep 16, 2024
a7553c2
Merge branch 'main' into cuprated-blockchain
Boog900 Oct 1, 2024
69f9d84
fix merge
Boog900 Oct 3, 2024
caaeced
clean up the blockchain manger
Boog900 Oct 3, 2024
8cff481
add more docs + cleanup imports
Boog900 Oct 3, 2024
4af0f89
fix typo
Boog900 Oct 3, 2024
6702dbe
fix doc
Boog900 Oct 3, 2024
403964b
remove unrelated changes
Boog900 Oct 3, 2024
783f426
improve interface globals
Boog900 Oct 5, 2024
375a1e1
manger -> manager
Boog900 Oct 5, 2024
f50d921
enums instead of bools
Boog900 Oct 5, 2024
27a8acd
move chain service to separate file
Boog900 Oct 5, 2024
a21e489
more review fixes
Boog900 Oct 5, 2024
c87edf2
add link to issue
Boog900 Oct 6, 2024
e7aaf3c
fix syncer + update comment
Boog900 Oct 8, 2024
173f4f7
fmt
Boog900 Oct 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions binaries/cuprated/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ cuprate-p2p-core = { path = "../../p2p/p2p-core" }
cuprate-dandelion-tower = { path = "../../p2p/dandelion-tower" }
cuprate-async-buffer = { path = "../../p2p/async-buffer" }
cuprate-address-book = { path = "../../p2p/address-book" }
cuprate-blockchain = { path = "../../storage/blockchain" }
cuprate-blockchain = { path = "../../storage/blockchain", features = ["service"] }
cuprate-database-service = { path = "../../storage/service" }
cuprate-txpool = { path = "../../storage/txpool" }
cuprate-database = { path = "../../storage/database" }
Expand Down Expand Up @@ -70,8 +70,14 @@ tokio-util = { workspace = true }
tokio-stream = { workspace = true }
tokio = { workspace = true }
tower = { workspace = true }
tracing-subscriber = { workspace = true }
tracing-subscriber = { workspace = true, features = ["std", "fmt", "default"] }
tracing = { workspace = true }

[lints]
workspace = true

[profile.dev]
panic = "abort"

[profile.release]
panic = "abort"
96 changes: 95 additions & 1 deletion binaries/cuprated/src/blockchain.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,100 @@
//! Blockchain
//!
//! Will contain the chain manager and syncer.
//! Contains the blockchain manager, syncer and an interface to mutate the blockchain.
use std::sync::Arc;

use futures::FutureExt;
use tokio::sync::{mpsc, Notify};
use tower::{BoxError, Service, ServiceExt};

use cuprate_blockchain::service::{BlockchainReadHandle, BlockchainWriteHandle};
use cuprate_consensus::{generate_genesis_block, BlockChainContextService, ContextConfig};
use cuprate_cryptonight::cryptonight_hash_v0;
use cuprate_p2p::{block_downloader::BlockDownloaderConfig, NetworkInterface};
use cuprate_p2p_core::{ClearNet, Network};
use cuprate_types::{
blockchain::{BlockchainReadRequest, BlockchainWriteRequest},
VerifiedBlockInformation,
};

use crate::constants::PANIC_CRITICAL_SERVICE_ERROR;

pub mod interface;
mod manager;
mod syncer;
mod types;

use types::{
ConcreteBlockVerifierService, ConcreteTxVerifierService, ConsensusBlockchainReadHandle,
};

/// Checks if the genesis block is in the blockchain and adds it if not.
pub async fn check_add_genesis(
blockchain_read_handle: &mut BlockchainReadHandle,
blockchain_write_handle: &mut BlockchainWriteHandle,
network: Network,
) {
// Try to get the chain height, will fail if the genesis block is not in the DB.
if blockchain_read_handle
.ready()
.await
.expect(PANIC_CRITICAL_SERVICE_ERROR)
.call(BlockchainReadRequest::ChainHeight)
.await
.is_ok()
{
return;
}

let genesis = generate_genesis_block(network);

assert_eq!(genesis.miner_transaction.prefix().outputs.len(), 1);
assert!(genesis.transactions.is_empty());

blockchain_write_handle
.ready()
.await
.expect(PANIC_CRITICAL_SERVICE_ERROR)
.call(BlockchainWriteRequest::WriteBlock(
VerifiedBlockInformation {
block_blob: genesis.serialize(),
txs: vec![],
block_hash: genesis.hash(),
pow_hash: cryptonight_hash_v0(&genesis.serialize_pow_hash()),
height: 0,
generated_coins: genesis.miner_transaction.prefix().outputs[0]
.amount
.unwrap(),
weight: genesis.miner_transaction.weight(),
long_term_weight: genesis.miner_transaction.weight(),
cumulative_difficulty: 1,
block: genesis,
},
))
.await
.expect(PANIC_CRITICAL_SERVICE_ERROR);
Comment on lines +55 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need to change for now but I still think cuprate_constants::genesis should exist - having the genesis block inlined in some async function in cuprated feels wrong. It's not about de-duplicatation but more about separating concerns and being clean, these constants deserve a name instead of being inlined.

Also, this is only mainnet right?

Copy link
Member Author

@Boog900 Boog900 Oct 5, 2024

Choose a reason for hiding this comment

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

having the genesis block inlined in some async function in cuprated feels wrong

This is just mapping the genesis block got from generate_genesis_block to VerifiedBlockInformation.

Also, this is only mainnet right?

No this should work on all 3 current networks, although it does (like monerod) make some assumptions about the genesis blocks format

Copy link
Contributor

Choose a reason for hiding this comment

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

mapping

I realize this, I'm saying this mapping should exist in cuprate_constants::genesis as well, next to the Block.

}

/// Initializes the consensus services.
pub async fn init_consensus(
blockchain_read_handle: BlockchainReadHandle,
context_config: ContextConfig,
) -> Result<
(
ConcreteBlockVerifierService,
ConcreteTxVerifierService,
BlockChainContextService,
),
BoxError,
> {
let read_handle = ConsensusBlockchainReadHandle::new(blockchain_read_handle, BoxError::from);

let ctx_service =
cuprate_consensus::initialize_blockchain_context(context_config, read_handle.clone())
.await?;

let (block_verifier_svc, tx_verifier_svc) =
cuprate_consensus::initialize_verifier(read_handle, ctx_service.clone());

Ok((block_verifier_svc, tx_verifier_svc, ctx_service))
}
168 changes: 168 additions & 0 deletions binaries/cuprated/src/blockchain/interface.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
//! The blockchain manger interface.
//!
//! This module contains all the functions to mutate the blockchain's state in any way, through the
//! blockchain manger.
Copy link
Member Author

@Boog900 Boog900 Oct 3, 2024

Choose a reason for hiding this comment

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

This file contains the interface for the blockchain manager.

I decided to not go with a tower::service interface, as I just don't think it is needed here. The benefits of a Service interface here would just be uniformity with every other service, the drawbacks are the extra over head of holding/cloning another handle (admittedly low) and just the extra hassle of holding another service handle.

I think these free functions as a way to mutate the blockchain makes more sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you say this about every Service though? Breaking the pattern and using global statics seems lazy? This is yet another interface that must be learned and it doesn't come with docs on how to use it.

If it's a 1-off in cuprated I think it's fine but there should be documentation on what the statics are, how to use generally use the interface, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fair, the API here should be pretty easy to use - all you need to do is call the functions, I'll add some more docs.

Couldn't you say this about every Service though

Other service are exposed this one is not. There are other benefits to the Service interface, like the wide range of middleware but since we are not going to make use of them for the blockchain manager that isn't a benefit here.

use std::{
collections::{HashMap, HashSet},
sync::{Mutex, OnceLock},
};

use monero_serai::{block::Block, transaction::Transaction};
use rayon::prelude::*;
use tokio::sync::{mpsc, oneshot};
use tower::{Service, ServiceExt};

use cuprate_blockchain::service::BlockchainReadHandle;
use cuprate_consensus::transactions::new_tx_verification_data;
use cuprate_helper::cast::usize_to_u64;
use cuprate_types::{
blockchain::{BlockchainReadRequest, BlockchainResponse},
Chain,
};

use crate::{
blockchain::manager::BlockchainManagerCommand, constants::PANIC_CRITICAL_SERVICE_ERROR,
};

/// The channel used to send [`BlockchainManagerCommand`]s to the blockchain manger.
pub static COMMAND_TX: OnceLock<mpsc::Sender<BlockchainManagerCommand>> = OnceLock::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Having a pub static OnceLock read/write available to the entire binary doesn't seem necessary, there should also be docs on who/when/where this OnceLock should be initialized.

Can this be privately defined where it is initiated then accompanied with:

/// Access to the blockchain manager's [...]
///
/// # Invariant
/// Must be initialized with [...]
///
/// # Panics
/// Panics if not initialized in [...]
pub fn command_tx() -> &mpsc::Sender<BlockchainManagerCommand> {
    &*COMMAND_TX.get().expect("wasn't initialized")
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops yeah wasn't meant to be fully public

Copy link
Member Author

Choose a reason for hiding this comment

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

I added some more docs to this


/// A [`HashSet`] of block hashes that the blockchain manager is currently handling.
///
/// This is used over something like a dashmap as we expect a lot of collisions in a short amount of
/// time for new blocks so we would lose the benefit of sharded locks. A dashmap is made up of `RwLocks`
/// which are also more expensive than `Mutex`s.
pub static BLOCKS_BEING_HANDLED: OnceLock<Mutex<HashSet<[u8; 32]>>> = OnceLock::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be pub? It's only currently used in this file.

To be honest pub static Mutex is pretty bad code smell - global resources like this really should be documented with who/when/how it's used.

If this only needs to be mutated within this file, what about doing the same as COMMAND_TX? Make this private and expose a fn() -> &HashSet<_, _> while documented that it takes a lock, and the usage patterns of that lock.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved it into the only function it is needed in + made it a LazyLock


/// An error that can be returned from [`handle_incoming_block`].
#[derive(Debug, thiserror::Error)]
pub enum IncomingBlockError {
/// Some transactions in the block were unknown.
///
/// The inner values are the block hash and the indexes of the missing txs in the block.
#[error("Unknown transactions in block.")]
UnknownTransactions([u8; 32], Vec<u64>),
/// We are missing the block's parent.
#[error("The block has an unknown parent.")]
Orphan,
/// The block was invalid.
#[error(transparent)]
InvalidBlock(anyhow::Error),
}

/// Try to add a new block to the blockchain.
///
/// This returns a [`bool`] indicating if the block was added to the main-chain ([`true`]) or an alt-chain
/// ([`false`]).
///
/// If we already knew about this block or the blockchain manger is not setup yet `Ok(false)` is returned.
///
/// # Errors
///
/// This function will return an error if:
/// - the block was invalid
/// - we are missing transactions
/// - the block's parent is unknown
pub async fn handle_incoming_block(
block: Block,
given_txs: Vec<Transaction>,
blockchain_read_handle: &mut BlockchainReadHandle,
) -> Result<bool, IncomingBlockError> {
// FIXME: we should look in the tx-pool for txs when that is ready.

if !block_exists(block.header.previous, blockchain_read_handle)
.await
.expect(PANIC_CRITICAL_SERVICE_ERROR)
{
return Err(IncomingBlockError::Orphan);
}

let block_hash = block.hash();

if block_exists(block_hash, blockchain_read_handle)
.await
.expect(PANIC_CRITICAL_SERVICE_ERROR)
{
return Ok(false);
}

// TODO: remove this when we have a working tx-pool.
if given_txs.len() != block.transactions.len() {
return Err(IncomingBlockError::UnknownTransactions(
block_hash,
(0..usize_to_u64(block.transactions.len())).collect(),
));
}
Comment on lines +93 to +99
Copy link
Contributor

Choose a reason for hiding this comment

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

Why usize_to_u64? Shouldn't it be usize if it's an index?

Copy link
Member Author

Choose a reason for hiding this comment

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

These values are sent over the P2P network, which requires a u64. I could change this and probably will when the tx-pool is made and do the conversion in the P2P hanlder.

However I would rather wait to make the tx-pool rather than preemptively change this.


// TODO: check we actually got given the right txs.
let prepped_txs = given_txs
.into_par_iter()
.map(|tx| {
let tx = new_tx_verification_data(tx)?;
Ok((tx.tx_hash, tx))
})
.collect::<Result<_, anyhow::Error>>()
.map_err(IncomingBlockError::InvalidBlock)?;

let Some(incoming_block_tx) = COMMAND_TX.get() else {
// We could still be starting up the blockchain manger, so just return this as there is nothing
// else we can do.
return Ok(false);
};

// Add the blocks hash to the blocks being handled.
if !BLOCKS_BEING_HANDLED
.get_or_init(|| Mutex::new(HashSet::new()))
.lock()
.unwrap()
.insert(block_hash)
{
// If another place is already adding this block then we can stop.
return Ok(false);
}

// From this point on we MUST not early return without removing the block hash from `BLOCKS_BEING_HANDLED`.

let (response_tx, response_rx) = oneshot::channel();

incoming_block_tx
.send(BlockchainManagerCommand::AddBlock {
block,
prepped_txs,
response_tx,
})
.await
.expect("TODO: don't actually panic here, an err means we are shutting down");

let res = response_rx
.await
.expect("The blockchain manager will always respond")
.map_err(IncomingBlockError::InvalidBlock);

// Remove the block hash from the blocks being handled.
BLOCKS_BEING_HANDLED
.get()
.unwrap()
.lock()
.unwrap()
.remove(&block_hash);

res
}

/// Check if we have a block with the given hash.
async fn block_exists(
block_hash: [u8; 32],
blockchain_read_handle: &mut BlockchainReadHandle,
) -> Result<bool, anyhow::Error> {
let BlockchainResponse::FindBlock(chain) = blockchain_read_handle
.ready()
.await?
.call(BlockchainReadRequest::FindBlock(block_hash))
.await?
else {
panic!("Invalid blockchain response!");
};

Ok(chain.is_some())
}
Loading
Loading