From 6272c40561f71894ba33f7834611b6fa56d841b5 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 31 Dec 2024 18:10:47 -0500 Subject: [PATCH] Restore `block_hash` to Batch It's not only helpful (to easily check where Serai's view of the external network is) but it's necessary in case of a non-trivial chain fork to determine which blockchain Serai considers canonical. --- coordinator/substrate/src/canonical.rs | 2 + processor/bin/src/lib.rs | 13 +---- processor/messages/src/lib.rs | 1 + processor/scanner/src/batch/mod.rs | 16 +++++- processor/scanner/src/lib.rs | 20 ++----- processor/scanner/src/substrate/db.rs | 33 +++--------- processor/scanner/src/substrate/mod.rs | 53 +++++++------------ substrate/abi/src/in_instructions.rs | 1 + substrate/in-instructions/pallet/src/lib.rs | 2 + .../in-instructions/primitives/src/lib.rs | 1 + 10 files changed, 55 insertions(+), 87 deletions(-) diff --git a/coordinator/substrate/src/canonical.rs b/coordinator/substrate/src/canonical.rs index d778bc7c..b9479aeb 100644 --- a/coordinator/substrate/src/canonical.rs +++ b/coordinator/substrate/src/canonical.rs @@ -160,6 +160,7 @@ impl ContinuallyRan for CanonicalEventStream { network: batch_network, publishing_session, id, + external_network_block_hash, in_instructions_hash, in_instruction_results, } = this_batch @@ -173,6 +174,7 @@ impl ContinuallyRan for CanonicalEventStream { batch = Some(ExecutedBatch { id: *id, publisher: *publishing_session, + external_network_block_hash: *external_network_block_hash, in_instructions_hash: *in_instructions_hash, in_instruction_results: in_instruction_results .iter() diff --git a/processor/bin/src/lib.rs b/processor/bin/src/lib.rs index 7dc794bf..119c4f40 100644 --- a/processor/bin/src/lib.rs +++ b/processor/bin/src/lib.rs @@ -277,23 +277,14 @@ pub async fn main_loop< } => { let scanner = scanner.as_mut().unwrap(); - if let Some(messages::substrate::ExecutedBatch { - id, - publisher, - in_instructions_hash, - in_instruction_results, - }) = batch - { + if let Some(batch) = batch { let key_to_activate = KeyToActivate::>::try_recv(txn.as_mut().unwrap()).map(|key| key.0); // This is a cheap call as it internally just queues this to be done later let _: () = scanner.acknowledge_batch( txn.take().unwrap(), - id, - publisher, - in_instructions_hash, - in_instruction_results, + batch, /* `acknowledge_batch` takes burns to optimize handling returns with standard payments. That's why handling these with a Batch (and not waiting until the diff --git a/processor/messages/src/lib.rs b/processor/messages/src/lib.rs index 1b6e1996..438beb5b 100644 --- a/processor/messages/src/lib.rs +++ b/processor/messages/src/lib.rs @@ -188,6 +188,7 @@ pub mod substrate { pub struct ExecutedBatch { pub id: u32, pub publisher: Session, + pub external_network_block_hash: [u8; 32], pub in_instructions_hash: [u8; 32], pub in_instruction_results: Vec, } diff --git a/processor/scanner/src/batch/mod.rs b/processor/scanner/src/batch/mod.rs index 8563ac4e..158306ca 100644 --- a/processor/scanner/src/batch/mod.rs +++ b/processor/scanner/src/batch/mod.rs @@ -10,6 +10,7 @@ use serai_in_instructions_primitives::{MAX_BATCH_SIZE, Batch}; use primitives::{EncodableG, task::ContinuallyRan}; use crate::{ db::{Returnable, ScannerGlobalDb, InInstructionData, ScanToBatchDb, BatchData, BatchToReportDb}, + index, scan::next_to_scan_for_outputs_block, ScannerFeed, KeyFor, }; @@ -100,10 +101,16 @@ impl ContinuallyRan for BatchTask { // If this block is notable, create the Batch(s) for it if notable { let network = S::NETWORK; + let external_network_block_hash = index::block_id(&txn, block_number); let mut batch_id = BatchDb::::acquire_batch_id(&mut txn); // start with empty batch - let mut batches = vec![Batch { network, id: batch_id, instructions: vec![] }]; + let mut batches = vec![Batch { + network, + id: batch_id, + external_network_block_hash, + instructions: vec![], + }]; // We also track the return information for the InInstructions within a Batch in case // they error let mut return_information = vec![vec![]]; @@ -123,7 +130,12 @@ impl ContinuallyRan for BatchTask { batch_id = BatchDb::::acquire_batch_id(&mut txn); // make a new batch with this instruction included - batches.push(Batch { network, id: batch_id, instructions: vec![in_instruction] }); + batches.push(Batch { + network, + id: batch_id, + external_network_block_hash, + instructions: vec![in_instruction], + }); // Since we're allocating a new batch, allocate a new set of return addresses for it return_information.push(vec![]); } diff --git a/processor/scanner/src/lib.rs b/processor/scanner/src/lib.rs index 3575f0d7..510af61b 100644 --- a/processor/scanner/src/lib.rs +++ b/processor/scanner/src/lib.rs @@ -11,9 +11,9 @@ use borsh::{BorshSerialize, BorshDeserialize}; use serai_db::{Get, DbTxn, Db}; use serai_primitives::{NetworkId, Coin, Amount}; -use serai_validator_sets_primitives::Session; use serai_coins_primitives::OutInstructionWithBalance; +use messages::substrate::ExecutedBatch; use primitives::{task::*, Address, ReceivedOutput, Block, Payment}; // Logic for deciding where in its lifetime a multisig is. @@ -444,29 +444,17 @@ impl Scanner { /// `queue_burns`. Doing so will cause them to be executed multiple times. /// /// The calls to this function must be ordered with regards to `queue_burns`. - #[allow(clippy::too_many_arguments)] pub fn acknowledge_batch( &mut self, mut txn: impl DbTxn, - batch_id: u32, - publisher: Session, - in_instructions_hash: [u8; 32], - in_instruction_results: Vec, + batch: ExecutedBatch, burns: Vec, key_to_activate: Option>, ) { - log::info!("acknowledging batch {batch_id}"); + log::info!("acknowledging batch {}", batch.id); // Queue acknowledging this block via the Substrate task - substrate::queue_acknowledge_batch::( - &mut txn, - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, - burns, - key_to_activate, - ); + substrate::queue_acknowledge_batch::(&mut txn, batch, burns, key_to_activate); // Commit this txn so this data is flushed txn.commit(); // Then run the Substrate task diff --git a/processor/scanner/src/substrate/db.rs b/processor/scanner/src/substrate/db.rs index af6679d4..1e0181b8 100644 --- a/processor/scanner/src/substrate/db.rs +++ b/processor/scanner/src/substrate/db.rs @@ -6,16 +6,14 @@ use borsh::{BorshSerialize, BorshDeserialize}; use serai_db::{Get, DbTxn, create_db, db_channel}; use serai_coins_primitives::OutInstructionWithBalance; -use serai_validator_sets_primitives::Session; + +use messages::substrate::ExecutedBatch; use crate::{ScannerFeed, KeyFor}; #[derive(BorshSerialize, BorshDeserialize)] struct AcknowledgeBatchEncodable { - batch_id: u32, - publisher: Session, - in_instructions_hash: [u8; 32], - in_instruction_results: Vec, + batch: ExecutedBatch, burns: Vec, key_to_activate: Option>, } @@ -27,10 +25,7 @@ enum ActionEncodable { } pub(crate) struct AcknowledgeBatch { - pub(crate) batch_id: u32, - pub(crate) publisher: Session, - pub(crate) in_instructions_hash: [u8; 32], - pub(crate) in_instruction_results: Vec, + pub(crate) batch: ExecutedBatch, pub(crate) burns: Vec, pub(crate) key_to_activate: Option>, } @@ -64,20 +59,14 @@ impl SubstrateDb { pub(crate) fn queue_acknowledge_batch( txn: &mut impl DbTxn, - batch_id: u32, - publisher: Session, - in_instructions_hash: [u8; 32], - in_instruction_results: Vec, + batch: ExecutedBatch, burns: Vec, key_to_activate: Option>, ) { Actions::send( txn, &ActionEncodable::AcknowledgeBatch(AcknowledgeBatchEncodable { - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, + batch, burns, key_to_activate: key_to_activate.map(|key| key.to_bytes().as_ref().to_vec()), }), @@ -91,17 +80,11 @@ impl SubstrateDb { let action_encodable = Actions::try_recv(txn)?; Some(match action_encodable { ActionEncodable::AcknowledgeBatch(AcknowledgeBatchEncodable { - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, + batch, burns, key_to_activate, }) => Action::AcknowledgeBatch(AcknowledgeBatch { - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, + batch, burns, key_to_activate: key_to_activate.map(|key| { let mut repr = as GroupEncoding>::Repr::default(); diff --git a/processor/scanner/src/substrate/mod.rs b/processor/scanner/src/substrate/mod.rs index 506debd4..6b22a259 100644 --- a/processor/scanner/src/substrate/mod.rs +++ b/processor/scanner/src/substrate/mod.rs @@ -3,12 +3,12 @@ use core::{marker::PhantomData, future::Future}; use serai_db::{Get, DbTxn, Db}; use serai_coins_primitives::{OutInstruction, OutInstructionWithBalance}; -use serai_validator_sets_primitives::Session; +use messages::substrate::ExecutedBatch; use primitives::task::ContinuallyRan; use crate::{ db::{ScannerGlobalDb, SubstrateToEventualityDb, AcknowledgedBatches}, - batch, ScannerFeed, KeyFor, + index, batch, ScannerFeed, KeyFor, }; mod db; @@ -19,22 +19,11 @@ pub(crate) fn last_acknowledged_batch(getter: &impl Get) -> Opti } pub(crate) fn queue_acknowledge_batch( txn: &mut impl DbTxn, - batch_id: u32, - publisher: Session, - in_instructions_hash: [u8; 32], - in_instruction_results: Vec, + batch: ExecutedBatch, burns: Vec, key_to_activate: Option>, ) { - SubstrateDb::::queue_acknowledge_batch( - txn, - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, - burns, - key_to_activate, - ) + SubstrateDb::::queue_acknowledge_batch(txn, batch, burns, key_to_activate) } pub(crate) fn queue_queue_burns( txn: &mut impl DbTxn, @@ -73,40 +62,38 @@ impl ContinuallyRan for SubstrateTask { }; match action { - Action::AcknowledgeBatch(AcknowledgeBatch { - batch_id, - publisher, - in_instructions_hash, - in_instruction_results, - mut burns, - key_to_activate, - }) => { + Action::AcknowledgeBatch(AcknowledgeBatch { batch, mut burns, key_to_activate }) => { // Check if we have the information for this batch let Some(batch::BatchInfo { block_number, session_to_sign_batch, external_key_for_session_to_sign_batch, - in_instructions_hash: expected_in_instructions_hash, - }) = batch::take_info_for_batch::(&mut txn, batch_id) + in_instructions_hash, + }) = batch::take_info_for_batch::(&mut txn, batch.id) else { // If we don't, drop this txn (restoring the action to the database) drop(txn); return Ok(made_progress); }; assert_eq!( - publisher, session_to_sign_batch, + batch.publisher, session_to_sign_batch, "batch acknowledged on-chain was acknowledged by an unexpected publisher" ); assert_eq!( - in_instructions_hash, expected_in_instructions_hash, - "batch acknowledged on-chain was distinct" + batch.external_network_block_hash, + index::block_id(&txn, block_number), + "batch acknowledged on-chain was for a distinct block" + ); + assert_eq!( + batch.in_instructions_hash, in_instructions_hash, + "batch acknowledged on-chain had distinct InInstructions" ); - SubstrateDb::::set_last_acknowledged_batch(&mut txn, batch_id); + SubstrateDb::::set_last_acknowledged_batch(&mut txn, batch.id); AcknowledgedBatches::send( &mut txn, &external_key_for_session_to_sign_batch.0, - batch_id, + batch.id, ); // Mark we made progress and handle this @@ -143,17 +130,17 @@ impl ContinuallyRan for SubstrateTask { // Return the balances for any InInstructions which failed to execute { - let return_information = batch::take_return_information::(&mut txn, batch_id) + let return_information = batch::take_return_information::(&mut txn, batch.id) .expect("didn't save the return information for Batch we published"); assert_eq!( - in_instruction_results.len(), + batch.in_instruction_results.len(), return_information.len(), "amount of InInstruction succeededs differed from amount of return information saved" ); // We map these into standard Burns for (result, return_information) in - in_instruction_results.into_iter().zip(return_information) + batch.in_instruction_results.into_iter().zip(return_information) { if result == messages::substrate::InInstructionResult::Succeeded { continue; diff --git a/substrate/abi/src/in_instructions.rs b/substrate/abi/src/in_instructions.rs index 89729f7a..ddf8c657 100644 --- a/substrate/abi/src/in_instructions.rs +++ b/substrate/abi/src/in_instructions.rs @@ -20,6 +20,7 @@ pub enum Event { network: NetworkId, publishing_session: Session, id: u32, + external_network_block_hash: [u8; 32], in_instructions_hash: [u8; 32], in_instruction_results: bitvec::vec::BitVec, }, diff --git a/substrate/in-instructions/pallet/src/lib.rs b/substrate/in-instructions/pallet/src/lib.rs index 79d4c717..2666f5b2 100644 --- a/substrate/in-instructions/pallet/src/lib.rs +++ b/substrate/in-instructions/pallet/src/lib.rs @@ -63,6 +63,7 @@ pub mod pallet { Batch { network: NetworkId, publishing_session: Session, + external_network_block_hash: [u8; 32], id: u32, in_instructions_hash: [u8; 32], in_instruction_results: BitVec, @@ -356,6 +357,7 @@ pub mod pallet { network: batch.network, publishing_session: if valid_by_prior { prior_session } else { current_session }, id: batch.id, + external_network_block_hash: batch.external_network_block_hash, in_instructions_hash, in_instruction_results, }); diff --git a/substrate/in-instructions/primitives/src/lib.rs b/substrate/in-instructions/primitives/src/lib.rs index ef88061b..a4d97db8 100644 --- a/substrate/in-instructions/primitives/src/lib.rs +++ b/substrate/in-instructions/primitives/src/lib.rs @@ -106,6 +106,7 @@ pub struct InInstructionWithBalance { pub struct Batch { pub network: NetworkId, pub id: u32, + pub external_network_block_hash: [u8; 32], pub instructions: Vec, }