From 70db3d827677a1890b6e3146de2fe8d696df40f3 Mon Sep 17 00:00:00 2001
From: Mateusz Morusiewicz <11313015+Ruteri@users.noreply.github.com>
Date: Wed, 17 Apr 2024 08:39:21 +0200
Subject: [PATCH] storage: fixes behaviour of cursor delete_current on start
 item (#7646)

Co-authored-by: Roman Krasiuk <rokrassyuk@gmail.com>
---
 crates/storage/db/src/abstraction/cursor.rs   |  4 ++
 .../storage/db/src/implementation/mdbx/mod.rs | 40 +++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/crates/storage/db/src/abstraction/cursor.rs b/crates/storage/db/src/abstraction/cursor.rs
index 306b010f5..eb7a209da 100644
--- a/crates/storage/db/src/abstraction/cursor.rs
+++ b/crates/storage/db/src/abstraction/cursor.rs
@@ -181,6 +181,7 @@ impl<'cursor, T: Table, CURSOR: DbCursorRO<T>> Walker<'cursor, T, CURSOR> {
 impl<'cursor, T: Table, CURSOR: DbCursorRW<T> + DbCursorRO<T>> Walker<'cursor, T, CURSOR> {
     /// Delete current item that walker points to.
     pub fn delete_current(&mut self) -> Result<(), DatabaseError> {
+        self.start.take();
         self.cursor.delete_current()
     }
 }
@@ -223,6 +224,7 @@ impl<'cursor, T: Table, CURSOR: DbCursorRO<T>> ReverseWalker<'cursor, T, CURSOR>
 impl<'cursor, T: Table, CURSOR: DbCursorRW<T> + DbCursorRO<T>> ReverseWalker<'cursor, T, CURSOR> {
     /// Delete current item that walker points to.
     pub fn delete_current(&mut self) -> Result<(), DatabaseError> {
+        self.start.take();
         self.cursor.delete_current()
     }
 }
@@ -321,6 +323,7 @@ impl<'cursor, T: Table, CURSOR: DbCursorRO<T>> RangeWalker<'cursor, T, CURSOR> {
 impl<'cursor, T: Table, CURSOR: DbCursorRW<T> + DbCursorRO<T>> RangeWalker<'cursor, T, CURSOR> {
     /// Delete current item that walker points to.
     pub fn delete_current(&mut self) -> Result<(), DatabaseError> {
+        self.start.take();
         self.cursor.delete_current()
     }
 }
@@ -353,6 +356,7 @@ where
 impl<'cursor, T: DupSort, CURSOR: DbCursorRW<T> + DbDupCursorRO<T>> DupWalker<'cursor, T, CURSOR> {
     /// Delete current item that walker points to.
     pub fn delete_current(&mut self) -> Result<(), DatabaseError> {
+        self.start.take();
         self.cursor.delete_current()
     }
 }
diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs
index 244c4d5b0..53594f671 100644
--- a/crates/storage/db/src/implementation/mdbx/mod.rs
+++ b/crates/storage/db/src/implementation/mdbx/mod.rs
@@ -482,6 +482,7 @@ mod tests {
     const ERROR_APPEND: &str = "Not able to append the value to the table.";
     const ERROR_UPSERT: &str = "Not able to upsert the value to the table.";
     const ERROR_GET: &str = "Not able to get value from table.";
+    const ERROR_DEL: &str = "Not able to delete from table.";
     const ERROR_COMMIT: &str = "Not able to commit transaction.";
     const ERROR_RETURN_VALUE: &str = "Mismatching result.";
     const ERROR_INIT_TX: &str = "Failed to create a MDBX transaction.";
@@ -511,6 +512,45 @@ mod tests {
         tx.commit().expect(ERROR_COMMIT);
     }
 
+    #[test]
+    fn db_dup_cursor_delete_first() {
+        let db: Arc<DatabaseEnv> = create_test_db(DatabaseEnvKind::RW);
+        let tx = db.tx_mut().expect(ERROR_INIT_TX);
+
+        let mut dup_cursor = tx.cursor_dup_write::<PlainStorageState>().unwrap();
+
+        let entry_0 = StorageEntry { key: B256::with_last_byte(1), value: U256::from(0) };
+        let entry_1 = StorageEntry { key: B256::with_last_byte(1), value: U256::from(1) };
+
+        dup_cursor.upsert(Address::with_last_byte(1), entry_0).expect(ERROR_UPSERT);
+        dup_cursor.upsert(Address::with_last_byte(1), entry_1).expect(ERROR_UPSERT);
+
+        assert_eq!(
+            dup_cursor.walk(None).unwrap().collect::<Result<Vec<_>, _>>(),
+            Ok(vec![(Address::with_last_byte(1), entry_0), (Address::with_last_byte(1), entry_1),])
+        );
+
+        let mut walker = dup_cursor.walk(None).unwrap();
+        walker.delete_current().expect(ERROR_DEL);
+
+        assert_eq!(walker.next(), Some(Ok((Address::with_last_byte(1), entry_1))));
+
+        // Check the tx view - it correctly holds entry_1
+        assert_eq!(
+            tx.cursor_dup_read::<PlainStorageState>()
+                .unwrap()
+                .walk(None)
+                .unwrap()
+                .collect::<Result<Vec<_>, _>>(),
+            Ok(vec![
+                (Address::with_last_byte(1), entry_1), // This is ok - we removed entry_0
+            ])
+        );
+
+        // Check the remainder of walker
+        assert_eq!(walker.next(), None);
+    }
+
     #[test]
     fn db_cursor_walk() {
         let env = create_test_db(DatabaseEnvKind::RW);