Merge pull request #1456 from subspace/overhaul-bundles/receipts-extr…

…action

Overhaul the extraction of bundles and receipts

crates/pallet-domains/src/lib.rs

@@ -26,14 +26,15 @@ use frame_support::traits::Get;
 use frame_system::offchain::SubmitTransaction;
 pub use pallet::*;
 use sp_core::H256;
-use sp_domains::bundle_election::{verify_system_bundle_solution, verify_vrf_proof};
+use sp_domains::bundle_election::verify_system_bundle_solution;
 use sp_domains::fraud_proof::FraudProof;
 use sp_domains::merkle_tree::Witness;
 use sp_domains::transaction::InvalidTransactionCode;
 use sp_domains::{BundleSolution, DomainId, ExecutionReceipt, ProofOfElection, SignedOpaqueBundle};
 use sp_runtime::traits::{BlockNumberProvider, CheckedSub, One, Zero};
 use sp_runtime::transaction_validity::TransactionValidityError;
 use sp_runtime::RuntimeAppPublic;
+use sp_std::vec::Vec;
 
 #[frame_support::pallet]
 mod pallet {
@@ -47,6 +48,7 @@ mod pallet {
     use sp_domains::{DomainId, ExecutorPublicKey, SignedOpaqueBundle};
     use sp_runtime::traits::{One, Zero};
     use sp_std::fmt::Debug;
+    use sp_std::vec::Vec;
 
     #[pallet::config]
     pub trait Config: frame_system::Config + pallet_receipts::Config {
@@ -60,6 +62,10 @@ mod pallet {
     #[pallet::without_storage_info]
     pub struct Pallet<T>(_);
 
+    /// Bundles submitted successfully in current block.
+    #[pallet::storage]
+    pub(super) type SuccessfulBundles<T> = StorageValue<_, Vec<H256>, ValueQuery>;
+
     #[derive(TypeInfo, Encode, Decode, PalletError, Debug, PartialEq)]
     pub enum BundleError {
         /// The signer of bundle is unexpected.
@@ -165,9 +171,13 @@ mod pallet {
                 .map_err(Error::<T>::from)?;
             }
 
+            let bundle_hash = signed_opaque_bundle.hash();
+
+            SuccessfulBundles::<T>::append(bundle_hash);
+
             Self::deposit_event(Event::BundleStored {
                 domain_id,
-                bundle_hash: signed_opaque_bundle.hash(),
+                bundle_hash,
                 bundle_author: signed_opaque_bundle.into_executor_public_key(),
             });
 
@@ -215,7 +225,9 @@ mod pallet {
                 );
             }
 
-            T::DbWeight::get().writes(1)
+            SuccessfulBundles::<T>::kill();
+
+            T::DbWeight::get().writes(2)
         }
     }
 
@@ -290,6 +302,10 @@ mod pallet {
 }
 
 impl<T: Config> Pallet<T> {
+    pub fn successful_bundles() -> Vec<H256> {
+        SuccessfulBundles::<T>::get()
+    }
+
     /// Returns the block number of the latest receipt.
     pub fn head_receipt_number() -> T::BlockNumber {
         pallet_receipts::Pallet::<T>::head_receipt_number(DomainId::SYSTEM)
@@ -523,13 +539,9 @@ impl<T: Config> Pallet<T> {
             return Err(BundleError::BadSignature);
         }
 
-        verify_vrf_proof(
-            &proof_of_election.executor_public_key,
-            &proof_of_election.vrf_output,
-            &proof_of_election.vrf_proof,
-            &proof_of_election.global_challenge,
-        )
-        .map_err(|_| BundleError::BadVrfProof)?;
+        proof_of_election
+            .verify_vrf_proof()
+            .map_err(|_| BundleError::BadVrfProof)?;
 
         Self::validate_execution_receipts(&bundle.receipts).map_err(BundleError::Receipt)?;
 

crates/pallet-receipts/src/lib.rs

@@ -25,19 +25,22 @@ use codec::{Decode, Encode};
 use frame_support::ensure;
 use frame_support::traits::Get;
 pub use pallet::*;
+use sp_core::H256;
 use sp_domains::fraud_proof::{FraudProof, InvalidStateTransitionProof, InvalidTransactionProof};
 use sp_domains::{DomainId, ExecutionReceipt};
 use sp_runtime::traits::{CheckedSub, One, Saturating, Zero};
 use sp_std::vec::Vec;
 
 #[frame_support::pallet]
 mod pallet {
-    use frame_support::pallet_prelude::{StorageMap, StorageNMap, *};
+    use frame_support::pallet_prelude::{StorageMap, StorageNMap, StorageValue, ValueQuery, *};
     use frame_support::PalletError;
+    use frame_system::pallet_prelude::*;
     use sp_core::H256;
     use sp_domains::{DomainId, ExecutionReceipt};
     use sp_runtime::traits::{CheckEqual, MaybeDisplay, SimpleBitOps};
     use sp_std::fmt::Debug;
+    use sp_std::vec::Vec;
 
     #[pallet::config]
     pub trait Config: frame_system::Config {
@@ -158,6 +161,18 @@ mod pallet {
         OptionQuery,
     >;
 
+    /// Fraud proof processed successfully in current block.
+    #[pallet::storage]
+    pub(super) type SuccessfulFraudProofs<T> = StorageValue<_, Vec<H256>, ValueQuery>;
+
+    #[pallet::hooks]
+    impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+        fn on_initialize(_now: BlockNumberFor<T>) -> Weight {
+            SuccessfulFraudProofs::<T>::kill();
+            T::DbWeight::get().writes(1)
+        }
+    }
+
     #[pallet::event]
     #[pallet::generate_deposit(pub (super) fn deposit_event)]
     pub enum Event<T: Config> {
@@ -167,8 +182,8 @@ mod pallet {
             primary_number: T::BlockNumber,
             primary_hash: T::Hash,
         },
-        /// A fraud proof was processed.
-        FraudProofProcessed {
+        /// An invalid state transition proof was processed.
+        InvalidStateTransitionProofProcessed {
             domain_id: DomainId,
             new_best_number: T::BlockNumber,
             new_best_hash: T::Hash,
@@ -208,6 +223,10 @@ impl From<FraudProofError> for Error {
 }
 
 impl<T: Config> Pallet<T> {
+    pub fn successful_fraud_proofs() -> Vec<H256> {
+        SuccessfulFraudProofs::<T>::get()
+    }
+
     /// Returns the block number of the latest receipt.
     pub fn head_receipt_number(domain_id: DomainId) -> T::BlockNumber {
         <HeadReceiptNumber<T>>::get(domain_id)
@@ -297,16 +316,18 @@ impl<T: Config> Pallet<T> {
     pub fn process_fraud_proof(
         fraud_proof: FraudProof<T::BlockNumber, T::Hash>,
     ) -> Result<(), Error> {
+        let proof_hash = fraud_proof.hash();
         match fraud_proof {
             FraudProof::InvalidStateTransition(proof) => {
-                Self::process_invalid_state_transition_proof(proof)
+                Self::process_invalid_state_transition_proof(proof)?
             }
             FraudProof::InvalidTransaction(_proof) => {
                 // TODO: slash the executor accordingly.
-                Ok(())
             }
-            _ => Err(FraudProofError::Unimplemented.into()),
+            _ => return Err(FraudProofError::Unimplemented.into()),
         }
+        SuccessfulFraudProofs::<T>::append(proof_hash);
+        Ok(())
     }
 
     fn process_invalid_state_transition_proof(
@@ -333,7 +354,7 @@ impl<T: Config> Pallet<T> {
             to_remove -= One::one();
         }
         // TODO: slash the executor accordingly.
-        Self::deposit_event(Event::FraudProofProcessed {
+        Self::deposit_event(Event::InvalidStateTransitionProofProcessed {
             domain_id,
             new_best_number,
             new_best_hash,

crates/sp-domains/src/bundle_election.rs

@@ -168,7 +168,7 @@ pub enum VrfProofError {
 }
 
 /// Verify the vrf proof generated in the bundle election.
-pub fn verify_vrf_proof(
+pub(crate) fn verify_vrf_proof(
     public_key: &ExecutorPublicKey,
     vrf_output: &[u8],
     vrf_proof: &[u8],

crates/sp-domains/src/fraud_proof.rs

@@ -201,6 +201,16 @@ impl<Number, Hash> FraudProof<Number, Hash> {
     }
 }
 
+impl<Number, Hash> FraudProof<Number, Hash>
+where
+    Number: Encode,
+    Hash: Encode,
+{
+    pub fn hash(&self) -> H256 {
+        BlakeTwo256::hash(&self.encode())
+    }
+}
+
 /// Proves an invalid state transition by challenging the trace at specific index in a bad receipt.
 #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)]
 pub struct InvalidStateTransitionProof {

crates/sp-domains/src/lib.rs

@@ -23,6 +23,7 @@ pub mod merkle_tree;
 pub mod transaction;
 
 use crate::fraud_proof::FraudProof;
+use bundle_election::VrfProofError;
 use merkle_tree::Witness;
 use parity_scale_codec::{Decode, Encode};
 use scale_info::TypeInfo;
@@ -217,6 +218,17 @@ pub struct ProofOfElection<DomainHash> {
     pub system_block_hash: DomainHash,
 }
 
+impl<DomainHash> ProofOfElection<DomainHash> {
+    pub fn verify_vrf_proof(&self) -> Result<(), VrfProofError> {
+        bundle_election::verify_vrf_proof(
+            &self.executor_public_key,
+            &self.vrf_output,
+            &self.vrf_proof,
+            &self.global_challenge,
+        )
+    }
+}
+
 impl<DomainHash: Default> ProofOfElection<DomainHash> {
     #[cfg(feature = "std")]
     pub fn dummy(domain_id: DomainId, executor_public_key: ExecutorPublicKey) -> Self {
@@ -440,8 +452,8 @@ sp_api::decl_runtime_apis! {
             domain_id: DomainId,
         ) -> OpaqueBundles<Block, DomainHash>;
 
-        /// Extract the hashes of bundles stored in the block
-        fn extract_stored_bundle_hashes() -> Vec<H256>;
+        /// Returns the hash of successfully submitted bundles.
+        fn successful_bundle_hashes() -> Vec<H256>;
 
         /// Extract the receipts from the given extrinsics.
         fn extract_receipts(

crates/subspace-runtime/src/domains.rs

@@ -1,7 +1,6 @@
-use crate::{Block, BlockNumber, Hash, RuntimeCall, RuntimeEvent, System, UncheckedExtrinsic};
+use crate::{Block, BlockNumber, Domains, Hash, Receipts, RuntimeCall, UncheckedExtrinsic};
 use sp_consensus_subspace::digests::CompatibleDigestItem;
 use sp_consensus_subspace::FarmerPublicKey;
-use sp_core::H256;
 use sp_domains::fraud_proof::FraudProof;
 use sp_domains::transaction::PreValidationObject;
 use sp_domains::{DomainId, ExecutionReceipt};
@@ -16,21 +15,20 @@ pub(crate) fn extract_system_bundles(
     sp_domains::OpaqueBundles<Block, domain_runtime_primitives::Hash>,
     sp_domains::SignedOpaqueBundles<Block, domain_runtime_primitives::Hash>,
 ) {
+    let successful_bundles = Domains::successful_bundles();
     let (system_bundles, core_bundles): (Vec<_>, Vec<_>) = extrinsics
         .into_iter()
-        .filter_map(|uxt| {
-            if let RuntimeCall::Domains(pallet_domains::Call::submit_bundle {
+        .filter_map(|uxt| match uxt.function {
+            RuntimeCall::Domains(pallet_domains::Call::submit_bundle {
                 signed_opaque_bundle,
-            }) = uxt.function
-            {
+            }) if successful_bundles.contains(&signed_opaque_bundle.hash()) => {
                 if signed_opaque_bundle.domain_id().is_system() {
                     Some((Some(signed_opaque_bundle.bundle), None))
                 } else {
                     Some((None, Some(signed_opaque_bundle)))
                 }
-            } else {
-                None
             }
+            _ => None,
         })
         .unzip();
     (
@@ -43,40 +41,35 @@ pub(crate) fn extract_core_bundles(
     extrinsics: Vec<UncheckedExtrinsic>,
     domain_id: DomainId,
 ) -> sp_domains::OpaqueBundles<Block, domain_runtime_primitives::Hash> {
+    let successful_bundles = Domains::successful_bundles();
     extrinsics
         .into_iter()
         .filter_map(|uxt| match uxt.function {
             RuntimeCall::Domains(pallet_domains::Call::submit_bundle {
                 signed_opaque_bundle,
-            }) if signed_opaque_bundle.domain_id() == domain_id => {
+            }) if signed_opaque_bundle.domain_id() == domain_id
+                && successful_bundles.contains(&signed_opaque_bundle.hash()) =>
+            {
                 Some(signed_opaque_bundle.bundle)
             }
             _ => None,
         })
         .collect()
 }
 
-pub(crate) fn extract_stored_bundle_hashes() -> Vec<H256> {
-    System::read_events_no_consensus()
-        .filter_map(|e| match e.event {
-            RuntimeEvent::Domains(pallet_domains::Event::BundleStored { bundle_hash, .. }) => {
-                Some(bundle_hash)
-            }
-            _ => None,
-        })
-        .collect::<Vec<_>>()
-}
-
 pub(crate) fn extract_receipts(
     extrinsics: Vec<UncheckedExtrinsic>,
     domain_id: DomainId,
 ) -> Vec<ExecutionReceipt<BlockNumber, Hash, domain_runtime_primitives::Hash>> {
+    let successful_bundles = Domains::successful_bundles();
     extrinsics
         .into_iter()
         .filter_map(|uxt| match uxt.function {
             RuntimeCall::Domains(pallet_domains::Call::submit_bundle {
                 signed_opaque_bundle,
-            }) if signed_opaque_bundle.domain_id() == domain_id => {
+            }) if signed_opaque_bundle.domain_id() == domain_id
+                && successful_bundles.contains(&signed_opaque_bundle.hash()) =>
+            {
                 Some(signed_opaque_bundle.bundle.receipts)
             }
             _ => None,
@@ -89,11 +82,13 @@ pub(crate) fn extract_fraud_proofs(
     extrinsics: Vec<UncheckedExtrinsic>,
     domain_id: DomainId,
 ) -> Vec<FraudProof<BlockNumber, Hash>> {
+    let successful_fraud_proofs = Receipts::successful_fraud_proofs();
     extrinsics
         .into_iter()
         .filter_map(|uxt| match uxt.function {
             RuntimeCall::Domains(pallet_domains::Call::submit_fraud_proof { fraud_proof })
-                if fraud_proof.domain_id() == domain_id =>
+                if fraud_proof.domain_id() == domain_id
+                    && successful_fraud_proofs.contains(&fraud_proof.hash()) =>
             {
                 Some(fraud_proof)
             }

crates/subspace-runtime/src/lib.rs

@@ -787,8 +787,8 @@ impl_runtime_apis! {
             crate::domains::extract_core_bundles(extrinsics, domain_id)
         }
 
-        fn extract_stored_bundle_hashes() -> Vec<H256> {
-            crate::domains::extract_stored_bundle_hashes()
+        fn successful_bundle_hashes() -> Vec<H256> {
+            Domains::successful_bundles()
         }
 
         fn extract_receipts(

crates/subspace-transaction-pool/src/bundle_validator.rs

@@ -52,14 +52,14 @@ where
         }
     }
 
-    fn extract_stored_bundles_at(
+    fn successfully_submitted_bundles_at(
         &self,
         block_hash: Block::Hash,
     ) -> sp_blockchain::Result<HashSet<Hash>> {
         let bundle_hashes: HashSet<_> = self
             .client
             .runtime_api()
-            .extract_stored_bundle_hashes(block_hash)?
+            .successful_bundle_hashes(block_hash)?
             .into_iter()
             .collect();
         Ok(bundle_hashes)
@@ -96,7 +96,7 @@ where
             }
         }
         for (hash, number) in blocks {
-            let bundles = self.extract_stored_bundles_at(hash)?;
+            let bundles = self.successfully_submitted_bundles_at(hash)?;
             bundle_stored_in_last_k.push_front(BlockBundle::new(hash, number, bundles));
         }
         Ok(())
@@ -142,7 +142,7 @@ where
 
         // Add bundles from the new block of the best fork
         for enacted_block in enacted {
-            let bundles = self.extract_stored_bundles_at(enacted_block.hash)?;
+            let bundles = self.successfully_submitted_bundles_at(enacted_block.hash)?;
             bundle_stored_in_last_k.push_front(BlockBundle::new(
                 enacted_block.hash,
                 enacted_block.number,