fuel-core icon indicating copy to clipboard operation
fuel-core copied to clipboard

Move the `DatabaseTransaction` logic into `StorageTransaction`

Open xgreenx opened this issue 1 year ago • 0 comments

Problem overview

The current logic of the executor works with the Transaction trait that hides the implementation and how database transaction works.

/// The type is storage transaction and holds uncommitted state.
pub trait Transaction<Storage: ?Sized>:
    AsRef<Storage> + AsMut<Storage> + Send + Sync
{
    /// Commits the pending state changes into the storage.
    fn commit(&mut self) -> StorageResult<()>;
}

/// The storage transaction for the `Storage` type.
pub struct StorageTransaction<Storage: ?Sized> {
    transaction: Box<dyn Transaction<Storage>>,
}

impl<Storage: ?Sized> Transaction<Storage> for StorageTransaction<Storage> {
    fn commit(&mut self) -> StorageResult<()> {
        self.transaction.commit()
    }
}

Because implementation is hidden, the executor can't be sure that StorageMutate methods don't update the underlying storage. It is crucial for the fraud proofs, and it will be useful to have control over that for forkless upgrades.

Solution

The solution is to move the logic of the DatabaseTransaction to the fuel-core-storage crate. But because it is still up to the database implementation of how the final commitment is done, we will add several requirements:

  • The database may be only modified through the commit_changes method.
    #[impl_tools::autoimpl(for<T: trait> &mut T, Box<T>)]
    pub trait Modifiable {
        fn commit_changes(&mut self, changes: Changes) -> StorageResult<()>;
    }
    
    pub type Changes = HashMap<(u32, Vec<u8>), WriteOperation>;
    
    pub struct StorageTransaction<Storage> {
        changes: Changes,
        storage: Storage,
    }
    
    impl<Storage> StorageTransaction<Storage>
    where
        Storage: Modifiable,
    {
        pub fn commit(&mut self) -> StorageResult<()> {
            self.storage
                .commit_changes(core::mem::take(&mut self.changes))
        }
    }
    
  • The StorageMutate is only implemented for the StorageTransaction, and the Storage should only implement read methods(and only one mutate commit_changes method).
  • The commit_changes requires &mut self, meaning that only one service will exclusively own the database. Consiquencly, all other services should only have read access to the database. We haven't trouble us with that before, but we need to guarantee a consistent view of the database during other service operations. The AtomicView from https://github.com/FuelLabs/fuel-core/pull/1579 helps with doing that:
    /// Provides a view of the storage at the given height.
    /// It guarantees to be atomic, meaning the view is immutable to outside modifications.
    pub trait AtomicView: Send + Sync {
        /// The type of the storage view.
        type View;
    
        /// Returns the view of the storage at the given `height`.
        fn view_at(&self, height: BlockHeight) -> StorageResult<Self::View>;
    
        /// Returns the view of the storage for the latest block height.
        fn latest_view(&self) -> Self::View;
    }
    

That solution:

  • Moves the logic of the storage transaction to the fuel-core-storage, providing control over that to the executor for fraud proofs and allowing upgrades via forkless upgrades.
  • It introduces database ownership that guarantees that commitment is done only by one service on a type system level.
  • It adds a consistent state view for read-only services.
  • The decision of how commit_changes works is done on the database level, instead of the transaction level. It allows implementing the https://github.com/FuelLabs/fuel-core/issues/451 a much easier way because we have only one place where it may happen(The Transaction trait is too abstract, and we need to create different database transactions types for different underlying database).

xgreenx avatar Jan 08 '24 01:01 xgreenx