diff --git a/processor/src/key_gen.rs b/processor/src/key_gen.rs index 115a4d1b..0c4d07b0 100644 --- a/processor/src/key_gen.rs +++ b/processor/src/key_gen.rs @@ -134,7 +134,11 @@ impl KeyGen { key: &::G, ) -> (ThresholdKeys, ThresholdKeys) { // This is safe, despite not having a txn, since it's a static value - // At worst, it's not set when it's expected to be set, yet that should be handled contextually + // The only concern is it may not be set when expected, or it may be set unexpectedly + // Since this unwraps, it being unset when expected to be set will cause a panic + // The only other concern is if it's set when it's not safe to use + // The keys are only written on confirmation, and the transaction writing them is atomic to + // every associated operation KeyGenDb::::keys(&self.db, key) } diff --git a/processor/src/main.rs b/processor/src/main.rs index c98661a2..a4e042c1 100644 --- a/processor/src/main.rs +++ b/processor/src/main.rs @@ -183,6 +183,7 @@ async fn sign_plans( let mut block_hash = >::Id::default(); block_hash.as_mut().copy_from_slice(&context.coin_latest_finalized_block.0); + // block_number call is safe since it unwraps let block_number = substrate_mutable .scanner .block_number(&block_hash) @@ -229,26 +230,26 @@ async fn handle_coordinator_msg( let mut needed_hash = >::Id::default(); needed_hash.as_mut().copy_from_slice(&block_hash.0); - let block_number; - loop { + let block_number = loop { // Ensure our scanner has scanned this block, which means our daemon has this block at // a sufficient depth - let Some(block_number_inner) = scanner.block_number(&needed_hash).await else { - warn!( - "node is desynced. we haven't scanned {} which should happen after {} confirms", - hex::encode(&needed_hash), - C::CONFIRMATIONS, - ); - sleep(Duration::from_secs(10)).await; - continue; + // The block_number may be set even if scanning isn't complete + let Some(block_number) = scanner.block_number(&needed_hash).await else { + warn!( + "node is desynced. we haven't scanned {} which should happen after {} confirms", + hex::encode(&needed_hash), + C::CONFIRMATIONS, + ); + sleep(Duration::from_secs(10)).await; + continue; + }; + break block_number; }; - block_number = block_number_inner; - break; - } // While the scanner has cemented this block, that doesn't mean it's been scanned for all // keys // ram_scanned will return the lowest scanned block number out of all keys + // This is a safe call which fulfills the unfulfilled safety requirements from the prior call while scanner.ram_scanned().await < block_number { sleep(Duration::from_secs(1)).await; } @@ -313,6 +314,7 @@ async fn handle_coordinator_msg( let mut activation_block_hash = >::Id::default(); activation_block_hash.as_mut().copy_from_slice(&activation_block.0); + // This block_number call is safe since it unwraps let activation_number = substrate_mutable .scanner .block_number(&activation_block_hash) diff --git a/processor/src/scanner.rs b/processor/src/scanner.rs index b65d7ba4..335bf9f6 100644 --- a/processor/src/scanner.rs +++ b/processor/src/scanner.rs @@ -288,9 +288,10 @@ impl ScannerHandle { scanner.keys.push(key); } + // This perform a database read which isn't safe with regards to if the value is set or not + // It may be set, when it isn't expected to be set, or not set, when it is expected to be set + // Since the value is static, if it's set, it's correctly set pub async fn block_number(&self, id: &>::Id) -> Option { - // This is safe, despite not having a txn, since it's a static value - // At worst, it's not set when it's expected to be set, yet that should be handled contextually ScannerDb::::block_number(&self.scanner.read().await.db, id) } @@ -405,6 +406,9 @@ impl Scanner { let block_id = block.id(); // These block calls are safe, despite not having a txn, since they're static values + // only written to/read by this thread + // There's also no error caused by them being unexpectedly written (if the commit is + // made and then the processor suddenly reboots) if let Some(id) = ScannerDb::::block(&scanner.db, i) { if id != block_id { panic!("reorg'd from finalized {} to {}", hex::encode(id), hex::encode(block_id));