Skip to content

Commit 2dc258e

Browse files
authored
Consensus: Fix reorgs (#384)
* add specific method for context * add new statemachine for tx verification * fix consensus crates build * working builds * fix CI * add docs * fix CI * fix docs * fix clippy * cleanup * add docs to `blockchain_context` * fix doc tests * add output cache * new monero-serai * todo * todo * Revert "new monero-serai" This reverts commit fe3f6ac. * use indexmap to request outputs * clean up * fix typos * fix CI * fix cargo hack * fix reorgs * check if a block is already present before adding it to the alt block cache * fmt
1 parent 6a9356b commit 2dc258e

File tree

7 files changed

+26
-20
lines changed

7 files changed

+26
-20
lines changed

binaries/cuprated/src/blockchain/manager/handler.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,12 +271,28 @@ impl super::BlockchainManager {
271271
block: Block,
272272
prepared_txs: HashMap<[u8; 32], TransactionVerificationData>,
273273
) -> Result<AddAltBlock, anyhow::Error> {
274+
// Check if a block already exists.
275+
let BlockchainResponse::FindBlock(chain) = self
276+
.blockchain_read_handle
277+
.ready()
278+
.await
279+
.expect(PANIC_CRITICAL_SERVICE_ERROR)
280+
.call(BlockchainReadRequest::FindBlock(block.hash()))
281+
.await
282+
.expect(PANIC_CRITICAL_SERVICE_ERROR)
283+
else {
284+
unreachable!();
285+
};
286+
287+
if chain.is_some() {
288+
// The block could also be in the main-chain here under some circumstances.
289+
return Ok(AddAltBlock::Cached);
290+
}
291+
274292
let alt_block_info =
275293
sanity_check_alt_block(block, prepared_txs, self.blockchain_context_service.clone())
276294
.await?;
277295

278-
// TODO: check in consensus crate if alt block with this hash already exists.
279-
280296
// If this alt chain has more cumulative difficulty, reorg.
281297
if alt_block_info.cumulative_difficulty
282298
> self
@@ -479,7 +495,7 @@ impl super::BlockchainManager {
479495

480496
/// The result from successfully adding an alt-block.
481497
enum AddAltBlock {
482-
/// The alt-block was cached.
498+
/// The alt-block was cached or was already present in the DB.
483499
Cached,
484500
/// The chain was reorged.
485501
Reorged,

binaries/cuprated/src/blockchain/syncer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{sync::Arc, time::Duration};
33

44
use futures::StreamExt;
55
use tokio::{
6-
sync::{mpsc, Notify},
6+
sync::{mpsc, oneshot, Notify},
77
time::interval,
88
};
99
use tower::{Service, ServiceExt};

binaries/cuprated/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ use cuprate_consensus_context::{
2727
BlockChainContextRequest, BlockChainContextResponse, BlockchainContextService,
2828
};
2929
use cuprate_helper::time::secs_to_hms;
30+
use cuprate_types::blockchain::BlockchainWriteRequest;
3031

3132
use crate::{
3233
config::Config, constants::PANIC_CRITICAL_SERVICE_ERROR, logging::CupratedTracingFilter,

consensus/context/src/alt_chains.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,8 @@ impl AltChainMap {
8383
}
8484

8585
/// Add an alt chain cache to the map.
86-
pub(crate) fn add_alt_cache(
87-
&mut self,
88-
prev_id: [u8; 32],
89-
alt_cache: Box<AltChainContextCache>,
90-
) {
91-
self.alt_cache_map.insert(prev_id, alt_cache);
86+
pub(crate) fn add_alt_cache(&mut self, alt_cache: Box<AltChainContextCache>) {
87+
self.alt_cache_map.insert(alt_cache.top_hash, alt_cache);
9288
}
9389

9490
/// Attempts to take an [`AltChainContextCache`] from the map, returning [`None`] if no cache is
@@ -119,7 +115,7 @@ impl AltChainMap {
119115
weight_cache: None,
120116
difficulty_cache: None,
121117
cached_rx_vm: None,
122-
chain_height: top_height,
118+
chain_height: top_height + 1,
123119
top_hash: prev_id,
124120
chain_id: None,
125121
parent_chain,

consensus/context/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,6 @@ pub enum BlockChainContextRequest {
307307
/// This variant is private and is not callable from outside this crate, the block verifier service will
308308
/// handle returning the alt cache to the context service.
309309
AddAltChainContextCache {
310-
/// The previous block field in a [`BlockHeader`](monero_serai::block::BlockHeader).
311-
prev_id: [u8; 32],
312310
/// The cache.
313311
cache: Box<AltChainContextCache>,
314312
/// An internal token to prevent external crates calling this request.

consensus/context/src/task.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,8 @@ impl<D: Database + Clone + Send + 'static> ContextTask<D> {
303303
.get_alt_vm(height, chain, &mut self.database)
304304
.await?,
305305
),
306-
BlockChainContextRequest::AddAltChainContextCache {
307-
prev_id,
308-
cache,
309-
_token,
310-
} => {
311-
self.alt_chain_cache_map.add_alt_cache(prev_id, cache);
306+
BlockChainContextRequest::AddAltChainContextCache { cache, _token } => {
307+
self.alt_chain_cache_map.add_alt_cache(cache);
312308
BlockChainContextResponse::Ok
313309
}
314310
BlockChainContextRequest::HardForkInfo(_)

consensus/src/block/alt_block.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,6 @@ where
178178
// Add this alt cache back to the context service.
179179
context_svc
180180
.oneshot(BlockChainContextRequest::AddAltChainContextCache {
181-
prev_id: block_info.block.header.previous,
182181
cache: alt_context_cache,
183182
_token: AltChainRequestToken,
184183
})

0 commit comments

Comments
 (0)