Move where we check if we should delay reporting of Batches

This commit is contained in:
Luke Parker
2024-12-30 10:18:38 -05:00
parent 1de8136739
commit 458f4fe170
3 changed files with 82 additions and 62 deletions

View File

@@ -5,10 +5,11 @@ use group::GroupEncoding;
use scale::{Encode, Decode, IoReader}; use scale::{Encode, Decode, IoReader};
use borsh::{BorshSerialize, BorshDeserialize}; use borsh::{BorshSerialize, BorshDeserialize};
use serai_db::{Get, DbTxn, create_db}; use serai_db::{Get, DbTxn, create_db, db_channel};
use serai_primitives::Balance; use serai_primitives::Balance;
use serai_validator_sets_primitives::Session; use serai_validator_sets_primitives::Session;
use serai_in_instructions_primitives::Batch;
use primitives::EncodableG; use primitives::EncodableG;
use crate::{ScannerFeed, KeyFor, AddressFor}; use crate::{ScannerFeed, KeyFor, AddressFor};
@@ -40,6 +41,12 @@ create_db!(
} }
); );
db_channel!(
ScannerReport {
InternalBatches: <G: GroupEncoding>() -> (Session, EncodableG<G>, Batch),
}
);
pub(crate) struct ReturnInformation<S: ScannerFeed> { pub(crate) struct ReturnInformation<S: ScannerFeed> {
pub(crate) address: AddressFor<S>, pub(crate) address: AddressFor<S>,
pub(crate) balance: Balance, pub(crate) balance: Balance,

View File

@@ -16,7 +16,7 @@ use crate::{
}; };
mod db; mod db;
pub(crate) use db::{BatchInfo, ReturnInformation}; pub(crate) use db::{BatchInfo, ReturnInformation, InternalBatches};
use db::ReportDb; use db::ReportDb;
pub(crate) fn take_info_for_batch<S: ScannerFeed>( pub(crate) fn take_info_for_batch<S: ScannerFeed>(
@@ -103,62 +103,6 @@ impl<D: Db, S: ScannerFeed> ContinuallyRan for ReportTask<D, S> {
let network = S::NETWORK; let network = S::NETWORK;
let mut batch_id = ReportDb::<S>::acquire_batch_id(&mut txn); let mut batch_id = ReportDb::<S>::acquire_batch_id(&mut txn);
/*
If this is the handover Batch, the first Batch signed by a session which retires the
prior validator set, then this should only be signed after the prior validator set's
actions are fully validated.
The new session will only be responsible for signing this Batch if the prior key has
retired, successfully completed all its on-external-network actions.
We check here the prior session has successfully completed all its on-Serai-network
actions by ensuring we've validated all Batches expected from it. Only then do we sign
the Batch confirming the handover.
We also wait for the Batch confirming the handover to be accepted on-chain, ensuring we
don't verify the prior session's Batches, sign the handover Batch and the following
Batch, have the prior session publish a malicious Batch where our handover Batch should
be, before our following Batch becomes our handover Batch.
*/
if session_to_sign_batch != Session(0) {
// We may have Session(1)'s first Batch be Batch 0 if Session(0) never publishes a
// Batch. This is fine as we'll hit the distinct Session check and then set the correct
// values into this DB entry. All other sessions must complete the handover process,
// which requires having published at least one Batch
let (last_session, first_batch) =
ReportDb::<S>::last_session_to_sign_batch_and_first_batch(&txn)
.unwrap_or((Session(0), 0));
// Because this boolean was expanded, we lose short-circuiting. That's fine
let handover_batch = last_session != session_to_sign_batch;
let batch_after_handover_batch =
(last_session == session_to_sign_batch) && ((first_batch + 1) == batch_id);
if handover_batch || batch_after_handover_batch {
let verified_prior_batch = substrate::last_acknowledged_batch::<S>(&txn)
// Since `batch_id = 0` in the Session(0)-never-published-a-Batch case, we don't
// check `last_acknowledged_batch >= (batch_id - 1)` but instead this
.map(|last_acknowledged_batch| (last_acknowledged_batch + 1) >= batch_id)
// We've never verified any Batches
.unwrap_or(false);
if !verified_prior_batch {
// Drop this txn, restoring the Batch to be worked on in the future
drop(txn);
return Ok(block_number > next_to_potentially_report);
}
}
// If this is the handover Batch, update the last session to sign a Batch
if handover_batch {
ReportDb::<S>::set_last_session_to_sign_batch_and_first_batch(
&mut txn,
session_to_sign_batch,
batch_id,
);
}
}
// TODO: The above code doesn't work if we end up with two Batches (the handover and the
// following) within this one Block due to Batch size limits
// start with empty batch // start with empty batch
let mut batches = vec![Batch { network, id: batch_id, instructions: vec![] }]; let mut batches = vec![Batch { network, id: batch_id, instructions: vec![] }];
// We also track the return information for the InInstructions within a Batch in case // We also track the return information for the InInstructions within a Batch in case
@@ -209,8 +153,10 @@ impl<D: Db, S: ScannerFeed> ContinuallyRan for ReportTask<D, S> {
} }
for batch in batches { for batch in batches {
Batches::send(&mut txn, &batch); InternalBatches::send(
BatchesToSign::send(&mut txn, &external_key_for_session_to_sign_batch, &batch); &mut txn,
&(session_to_sign_batch, EncodableG(external_key_for_session_to_sign_batch), batch),
);
} }
} }
@@ -220,6 +166,73 @@ impl<D: Db, S: ScannerFeed> ContinuallyRan for ReportTask<D, S> {
txn.commit(); txn.commit();
} }
// TODO: This should be its own task. The above doesn't error, doesn't return early, so this
// is fine, but this is precarious and would be better as its own task
{
let mut txn = self.db.txn();
while let Some((session_to_sign_batch, external_key_for_session_to_sign_batch, batch)) =
InternalBatches::<KeyFor<S>>::peek(&txn)
{
/*
If this is the handover Batch, the first Batch signed by a session which retires the
prior validator set, then this should only be signed after the prior validator set's
actions are fully validated.
The new session will only be responsible for signing this Batch if the prior key has
retired, successfully completed all its on-external-network actions.
We check here the prior session has successfully completed all its on-Serai-network
actions by ensuring we've validated all Batches expected from it. Only then do we sign
the Batch confirming the handover.
We also wait for the Batch confirming the handover to be accepted on-chain, ensuring we
don't verify the prior session's Batches, sign the handover Batch and the following
Batch, have the prior session publish a malicious Batch where our handover Batch should
be, before our following Batch becomes our handover Batch.
*/
if session_to_sign_batch != Session(0) {
// We may have Session(1)'s first Batch be Batch 0 if Session(0) never publishes a
// Batch. This is fine as we'll hit the distinct Session check and then set the correct
// values into this DB entry. All other sessions must complete the handover process,
// which requires having published at least one Batch
let (last_session, first_batch) =
ReportDb::<S>::last_session_to_sign_batch_and_first_batch(&txn)
.unwrap_or((Session(0), 0));
// Because this boolean was expanded, we lose short-circuiting. That's fine
let handover_batch = last_session != session_to_sign_batch;
let batch_after_handover_batch =
(last_session == session_to_sign_batch) && ((first_batch + 1) == batch.id);
if handover_batch || batch_after_handover_batch {
let verified_prior_batch = substrate::last_acknowledged_batch::<S>(&txn)
// Since `batch.id = 0` in the Session(0)-never-published-a-Batch case, we don't
// check `last_acknowledged_batch >= (batch.id - 1)` but instead this
.map(|last_acknowledged_batch| (last_acknowledged_batch + 1) >= batch.id)
// We've never verified any Batches
.unwrap_or(false);
if !verified_prior_batch {
break;
}
}
// If this is the handover Batch, update the last session to sign a Batch
if handover_batch {
ReportDb::<S>::set_last_session_to_sign_batch_and_first_batch(
&mut txn,
session_to_sign_batch,
batch.id,
);
}
}
// Since we should handle this batch now, recv it from the channel
InternalBatches::<KeyFor<S>>::try_recv(&mut txn).unwrap();
Batches::send(&mut txn, &batch);
BatchesToSign::send(&mut txn, &external_key_for_session_to_sign_batch.0, &batch);
}
txn.commit();
}
// Run dependents if we decided to report any blocks // Run dependents if we decided to report any blocks
Ok(next_to_potentially_report <= highest_reportable) Ok(next_to_potentially_report <= highest_reportable)
} }

View File

@@ -422,7 +422,7 @@ impl<
block: [u8; 32], block: [u8; 32],
) { ) {
// Don't cosign blocks with already retired keys // Don't cosign blocks with already retired keys
if Some(session.0) <= db::LatestRetiredSession::get(txn).map(|session| session.0) { if Some(session.0) <= db::LatestRetiredSession::get(&txn).map(|session| session.0) {
return; return;
} }
@@ -444,7 +444,7 @@ impl<
slash_report: &Vec<Slash>, slash_report: &Vec<Slash>,
) { ) {
// Don't sign slash reports with already retired keys // Don't sign slash reports with already retired keys
if Some(session.0) <= db::LatestRetiredSession::get(txn).map(|session| session.0) { if Some(session.0) <= db::LatestRetiredSession::get(&txn).map(|session| session.0) {
return; return;
} }