[Storage] Remove Accumulator consistency proof from state proof

Closes: aptos-labs#996
sonic7077 · May 15, 2022 · 8f1bb6d · 8f1bb6d
1 parent 8acad77
commit 8f1bb6d
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 216 deletions.
diff --git a/consensus/src/persistent_liveness_storage.rs b/consensus/src/persistent_liveness_storage.rs
@@ -483,7 +483,7 @@ impl PersistentLivenessStorage for StorageWriteProxy {
     }
 
     fn retrieve_epoch_change_proof(&self, version: u64) -> Result<EpochChangeProof> {
-        let (_, proofs, _) = self
+        let (_, proofs) = self
             .aptos_db
             .get_state_proof(version)
             .map_err(DbError::from)?

diff --git a/execution/executor-test-helpers/src/integration_test_impl.rs b/execution/executor-test-helpers/src/integration_test_impl.rs
@@ -168,14 +168,12 @@ pub fn test_execution_with_storage_impl() -> Arc<AptosDB> {
         .commit_blocks(vec![block1_id], ledger_info_with_sigs)
         .unwrap();
 
-    let initial_accumulator = db.reader.get_accumulator_summary(0).unwrap();
     let state_proof = db.reader.get_state_proof(0).unwrap();
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
-    let trusted_state =
-        match trusted_state.verify_and_ratchet(&state_proof, Some(&initial_accumulator)) {
-            Ok(TrustedStateChange::Epoch { new_state, .. }) => new_state,
-            _ => panic!("unexpected state change"),
-        };
+    let trusted_state = match trusted_state.verify_and_ratchet(&state_proof) {
+        Ok(TrustedStateChange::Epoch { new_state, .. }) => new_state,
+        _ => panic!("unexpected state change"),
+    };
     let current_version = state_proof.latest_ledger_info().version();
     assert_eq!(trusted_state.version(), 9);
 
@@ -354,9 +352,7 @@ pub fn test_execution_with_storage_impl() -> Arc<AptosDB> {
         .unwrap();
 
     let state_proof = db.reader.get_state_proof(trusted_state.version()).unwrap();
-    let trusted_state_change = trusted_state
-        .verify_and_ratchet(&state_proof, None)
-        .unwrap();
+    let trusted_state_change = trusted_state.verify_and_ratchet(&state_proof).unwrap();
     assert!(matches!(
         trusted_state_change,
         TrustedStateChange::Version { .. }

diff --git a/execution/executor/tests/db_bootstrapper_test.rs b/execution/executor/tests/db_bootstrapper_test.rs
@@ -73,18 +73,12 @@ fn test_empty_db() {
         waypoint
     );
 
-    let initial_accumulator = db_rw
-        .reader
-        .get_accumulator_summary(waypoint.version())
-        .unwrap();
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
     let state_proof = db_rw
         .reader
         .get_state_proof(trusted_state.version())
         .unwrap();
-    let trusted_state_change = trusted_state
-        .verify_and_ratchet(&state_proof, Some(&initial_accumulator))
-        .unwrap();
+    let trusted_state_change = trusted_state.verify_and_ratchet(&state_proof).unwrap();
     assert!(trusted_state_change.is_epoch_change());
 
     // `maybe_bootstrap()` does nothing on non-empty DB.
@@ -310,17 +304,11 @@ fn test_pre_genesis() {
     assert!(maybe_bootstrap::<AptosVM>(&db_rw, &genesis_txn, waypoint).unwrap());
 
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
-    let initial_accumulator = db_rw
-        .reader
-        .get_accumulator_summary(trusted_state.version())
-        .unwrap();
     let state_proof = db_rw
         .reader
         .get_state_proof(trusted_state.version())
         .unwrap();
-    let trusted_state_change = trusted_state
-        .verify_and_ratchet(&state_proof, Some(&initial_accumulator))
-        .unwrap();
+    let trusted_state_change = trusted_state.verify_and_ratchet(&state_proof).unwrap();
     assert!(trusted_state_change.is_epoch_change());
 
     // Effect of bootstrapping reflected.
@@ -351,14 +339,8 @@ fn test_new_genesis() {
     assert_eq!(get_balance(&account2, &db), 2_000_000);
 
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
-    let initial_accumulator = db
-        .reader
-        .get_accumulator_summary(trusted_state.version())
-        .unwrap();
     let state_proof = db.reader.get_state_proof(trusted_state.version()).unwrap();
-    let trusted_state_change = trusted_state
-        .verify_and_ratchet(&state_proof, Some(&initial_accumulator))
-        .unwrap();
+    let trusted_state_change = trusted_state.verify_and_ratchet(&state_proof).unwrap();
     assert!(trusted_state_change.is_epoch_change());
 
     // New genesis transaction: set validator set, bump epoch and overwrite account1 balance.
@@ -398,18 +380,11 @@ fn test_new_genesis() {
 
     // Client bootable from waypoint.
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
-    let initial_accumulator = db
-        .reader
-        .get_accumulator_summary(trusted_state.version())
-        .unwrap();
     let state_proof = db.reader.get_state_proof(trusted_state.version()).unwrap();
-    let trusted_state_change = trusted_state
-        .verify_and_ratchet(&state_proof, Some(&initial_accumulator))
-        .unwrap();
+    let trusted_state_change = trusted_state.verify_and_ratchet(&state_proof).unwrap();
     assert!(trusted_state_change.is_epoch_change());
     let trusted_state = trusted_state_change.new_state().unwrap();
     assert_eq!(trusted_state.version(), 5);
-    assert!(state_proof.consistency_proof().is_empty());
 
     // Effect of bootstrapping reflected.
     assert_eq!(get_balance(&account1, &db), 1_000_000);

diff --git a/execution/executor/tests/storage_integration_test.rs b/execution/executor/tests/storage_integration_test.rs
@@ -32,15 +32,9 @@ fn test_genesis() {
     let (_, db, _executor, waypoint) = create_db_and_executor(path.path(), &genesis);
 
     let trusted_state = TrustedState::from_epoch_waypoint(waypoint);
-    let initial_accumulator = db
-        .reader
-        .get_accumulator_summary(trusted_state.version())
-        .unwrap();
     let state_proof = db.reader.get_state_proof(trusted_state.version()).unwrap();
 
-    trusted_state
-        .verify_and_ratchet(&state_proof, Some(&initial_accumulator))
-        .unwrap();
+    trusted_state.verify_and_ratchet(&state_proof).unwrap();
     let li = state_proof.latest_ledger_info();
     assert_eq!(li.version(), 0);
 

diff --git a/storage/aptosdb/src/lib.rs b/storage/aptosdb/src/lib.rs
@@ -47,7 +47,7 @@ use crate::{
     system_store::SystemStore,
     transaction_store::TransactionStore,
 };
-use anyhow::{ensure, format_err, Result};
+use anyhow::{ensure, Result};
 use aptos_config::config::{RocksdbConfig, StoragePrunerConfig, NO_OP_STORAGE_PRUNER_CONFIG};
 use aptos_crypto::hash::{HashValue, SPARSE_MERKLE_PLACEHOLDER_HASH};
 use aptos_infallible::Mutex;
@@ -955,32 +955,7 @@ impl DbReader for AptosDB {
                 EpochChangeProof::new(vec![], /* more = */ false)
             };
 
-            // Only return a consistency proof up to the verifiable end LI. If a
-            // client still needs to sync more epoch change LI's, then they cannot
-            // verify the latest LI nor verify a consistency proof up to the latest
-            // LI. If the client needs more epochs, we just return the consistency
-            // proof up to the last epoch change LI.
-            let verifiable_li = if epoch_change_proof.more {
-                epoch_change_proof
-                    .ledger_info_with_sigs
-                    .last()
-                    .ok_or_else(|| format_err!(
-                        "No epoch changes despite claiming the client needs to sync more epochs: known_epoch={}, end_epoch={}",
-                        known_epoch, end_epoch,
-                    ))?
-                    .ledger_info()
-            } else {
-                ledger_info
-            };
-
-            let consistency_proof = self
-                .ledger_store
-                .get_consistency_proof(Some(known_version), verifiable_li.version())?;
-            Ok(StateProof::new(
-                ledger_info_with_sigs,
-                epoch_change_proof,
-                consistency_proof,
-            ))
+            Ok(StateProof::new(ledger_info_with_sigs, epoch_change_proof))
         })
     }
 

diff --git a/types/src/state_proof.rs b/types/src/state_proof.rs
@@ -4,7 +4,6 @@
 use crate::{
     epoch_change::EpochChangeProof,
     ledger_info::{LedgerInfo, LedgerInfoWithSignatures},
-    proof::AccumulatorConsistencyProof,
 };
 #[cfg(any(test, feature = "fuzzing"))]
 use proptest_derive::Arbitrary;
@@ -24,48 +23,25 @@ use serde::{Deserialize, Serialize};
 pub struct StateProof {
     latest_li_w_sigs: LedgerInfoWithSignatures,
     epoch_changes: EpochChangeProof,
-    consistency_proof: AccumulatorConsistencyProof,
 }
 
 impl StateProof {
     pub fn new(
         latest_li_w_sigs: LedgerInfoWithSignatures,
         epoch_changes: EpochChangeProof,
-        consistency_proof: AccumulatorConsistencyProof,
     ) -> Self {
         Self {
             latest_li_w_sigs,
             epoch_changes,
-            consistency_proof,
         }
     }
 
-    pub fn into_inner(
-        self,
-    ) -> (
-        LedgerInfoWithSignatures,
-        EpochChangeProof,
-        AccumulatorConsistencyProof,
-    ) {
-        (
-            self.latest_li_w_sigs,
-            self.epoch_changes,
-            self.consistency_proof,
-        )
+    pub fn into_inner(self) -> (LedgerInfoWithSignatures, EpochChangeProof) {
+        (self.latest_li_w_sigs, self.epoch_changes)
     }
 
-    pub fn as_inner(
-        &self,
-    ) -> (
-        &LedgerInfoWithSignatures,
-        &EpochChangeProof,
-        &AccumulatorConsistencyProof,
-    ) {
-        (
-            &self.latest_li_w_sigs,
-            &self.epoch_changes,
-            &self.consistency_proof,
-        )
+    pub fn as_inner(&self) -> (&LedgerInfoWithSignatures, &EpochChangeProof) {
+        (&self.latest_li_w_sigs, &self.epoch_changes)
     }
 
     #[inline]
@@ -82,11 +58,6 @@ impl StateProof {
     pub fn epoch_changes(&self) -> &EpochChangeProof {
         &self.epoch_changes
     }
-
-    #[inline]
-    pub fn consistency_proof(&self) -> &AccumulatorConsistencyProof {
-        &self.consistency_proof
-    }
 }
 
 #[cfg(test)]

diff --git a/types/src/trusted_state.rs b/types/src/trusted_state.rs
@@ -5,13 +5,12 @@ use crate::{
     epoch_change::{EpochChangeProof, Verifier},
     epoch_state::EpochState,
     ledger_info::{LedgerInfo, LedgerInfoWithSignatures},
-    proof::{AccumulatorConsistencyProof, TransactionAccumulatorSummary},
+    proof::TransactionAccumulatorSummary,
     state_proof::StateProof,
     transaction::Version,
     waypoint::Waypoint,
 };
 use anyhow::{bail, ensure, format_err, Result};
-use aptos_crypto::HashValue;
 use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher};
 #[cfg(any(test, feature = "fuzzing"))]
 use proptest_derive::Arbitrary;
@@ -35,12 +34,6 @@ pub enum TrustedState {
         waypoint: Waypoint,
         /// The current epoch and validator set inside that epoch.
         epoch_state: EpochState,
-        /// The current verified view of the transaction accumulator. Note that this
-        /// is not the complete accumulator; rather, it is a summary containing only
-        /// the frozen subtrees at the currently verified state version. We use the
-        /// accumulator summary to verify accumulator consistency proofs when
-        /// applying state proofs.
-        accumulator: TransactionAccumulatorSummary,
     },
 }
 
@@ -91,7 +84,6 @@ impl TrustedState {
         Ok(Self::EpochState {
             waypoint: Waypoint::new_epoch_boundary(epoch_change_li)?,
             epoch_state,
-            accumulator,
         })
     }
 
@@ -110,24 +102,6 @@ impl TrustedState {
         }
     }
 
-    pub fn accumulator_root_hash(&self) -> Option<HashValue> {
-        match self {
-            Self::EpochWaypoint(_) => None,
-            Self::EpochState { accumulator, .. } => Some(accumulator.root_hash()),
-        }
-    }
-
-    pub fn accumulator_summary(&self) -> Option<&TransactionAccumulatorSummary> {
-        match self {
-            Self::EpochWaypoint(_) => None,
-            Self::EpochState { accumulator, .. } => Some(accumulator),
-        }
-    }
-
-    pub fn need_accumulator(&self) -> bool {
-        self.accumulator_summary().is_none()
-    }
-
     /// Verify and ratchet forward our trusted state using an [`EpochChangeProof`]
     /// (that moves us into the latest epoch), a [`LedgerInfoWithSignatures`]
     /// inside that epoch, and an [`AccumulatorConsistencyProof`] from our current
@@ -161,22 +135,17 @@ impl TrustedState {
     pub fn verify_and_ratchet<'a>(
         &self,
         state_proof: &'a StateProof,
-        initial_accumulator: Option<&'a TransactionAccumulatorSummary>,
     ) -> Result<TrustedStateChange<'a>> {
         self.verify_and_ratchet_inner(
             state_proof.latest_ledger_info_w_sigs(),
             state_proof.epoch_changes(),
-            state_proof.consistency_proof(),
-            initial_accumulator,
         )
     }
 
     pub fn verify_and_ratchet_inner<'a>(
         &self,
         latest_li: &'a LedgerInfoWithSignatures,
         epoch_change_proof: &'a EpochChangeProof,
-        consistency_proof: &'a AccumulatorConsistencyProof,
-        initial_accumulator: Option<&'a TransactionAccumulatorSummary>,
     ) -> Result<TrustedStateChange<'a>> {
         // Abort early if the response is stale.
         let curr_version = self.version();
@@ -187,28 +156,6 @@ impl TrustedState {
             target_version, curr_version,
         );
 
-        let curr_accumulator = match self {
-            // If we're verifying from an epoch waypoint, the user needs to provide
-            // an initial accumulator. Note that we assume this accumulator is
-            // untrusted.
-            Self::EpochWaypoint(_) => {
-                // When verifying from a waypoint, we need to check that the initial
-                // untrusted accumulator is consistent with the received waypoint
-                // ledger info.
-                let curr_accumulator = initial_accumulator
-                    .expect("Client must provide an initial untrusted accumulator when verifying from a waypoint");
-                let waypoint_li = epoch_change_proof
-                    .ledger_info_with_sigs
-                    .first()
-                    .ok_or_else(|| format_err!("Empty epoch change proof"))?
-                    .ledger_info();
-                curr_accumulator.verify_consistency(waypoint_li)?;
-                curr_accumulator
-            }
-            // Otherwise, we should already have a _trusted_ accumulator.
-            Self::EpochState { accumulator, .. } => accumulator,
-        };
-
         if self.epoch_change_verification_required(latest_li.ledger_info().next_block_epoch()) {
             // Verify the EpochChangeProof to move us into the latest epoch.
             let epoch_change_li = epoch_change_proof.verify(self)?;
@@ -238,15 +185,9 @@ impl TrustedState {
             };
             let new_waypoint = Waypoint::new_any(verified_ledger_info.ledger_info());
 
-            // Try to extend our accumulator summary and check that it's consistent
-            // with the target ledger info.
-            let new_accumulator = curr_accumulator
-                .try_extend_with_proof(consistency_proof, verified_ledger_info.ledger_info())?;
-
             let new_state = TrustedState::EpochState {
                 waypoint: new_waypoint,
                 epoch_state: new_epoch_state,
-                accumulator: new_accumulator,
             };
 
             Ok(TrustedStateChange::Epoch {
@@ -279,15 +220,9 @@ impl TrustedState {
                 // Verify the target ledger info, which should be inside the current epoch.
                 curr_epoch_state.verify(latest_li)?;
 
-                // Try to extend our accumulator summary and check that it's consistent
-                // with the target ledger info.
-                let new_accumulator = curr_accumulator
-                    .try_extend_with_proof(consistency_proof, latest_li.ledger_info())?;
-
                 let new_state = Self::EpochState {
                     waypoint: new_waypoint,
                     epoch_state: curr_epoch_state.clone(),
-                    accumulator: new_accumulator,
                 };
 
                 Ok(TrustedStateChange::Version { new_state })