chore(db): clarify upsert (#2216)

Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

crates/storage/db/src/implementation/mdbx/cursor.rs

@@ -204,6 +204,11 @@ impl<'tx, K: TransactionKind, T: DupSort> DbDupCursorRO<'tx, T> for Cursor<'tx,
 impl<'tx, T: Table> DbCursorRW<'tx, T> for Cursor<'tx, RW, T> {
     /// Database operation that will update an existing row if a specified value already
     /// exists in a table, and insert a new row if the specified value doesn't already exist
+    ///
+    /// For a DUPSORT table, `upsert` will not actually update-or-insert. If the key already exists,
+    /// it will append the value to the subkey, even if the subkeys are the same. So if you want
+    /// to properly upsert, you'll need to `seek_exact` & `delete_current` if the key+subkey was
+    /// found, before calling `upsert`.
     fn upsert(&mut self, key: T::Key, value: T::Value) -> Result<(), Error> {
         // Default `WriteFlags` is UPSERT
         self.inner

crates/storage/db/src/implementation/mdbx/mod.rs

@@ -160,6 +160,7 @@ mod tests {
     const ERROR_DB_CREATION: &str = "Not able to create the mdbx file.";
     const ERROR_PUT: &str = "Not able to insert value into table.";
     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_COMMIT: &str = "Not able to commit transaction.";
     const ERROR_RETURN_VALUE: &str = "Mismatching result.";
@@ -555,6 +556,41 @@ mod tests {
         tx.commit().expect(ERROR_COMMIT);
     }
 
+    #[test]
+    fn db_cursor_upsert() {
+        let db: Arc<Env<WriteMap>> = test_utils::create_test_db(EnvKind::RW);
+        let tx = db.tx_mut().expect(ERROR_INIT_TX);
+
+        let mut cursor = tx.cursor_write::<PlainAccountState>().unwrap();
+        let key = Address::random();
+
+        let account = Account::default();
+        cursor.upsert(key, account).expect(ERROR_UPSERT);
+        assert_eq!(cursor.seek_exact(key), Ok(Some((key, account))));
+
+        let account = Account { nonce: 1, ..Default::default() };
+        cursor.upsert(key, account).expect(ERROR_UPSERT);
+        assert_eq!(cursor.seek_exact(key), Ok(Some((key, account))));
+
+        let account = Account { nonce: 2, ..Default::default() };
+        cursor.upsert(key, account).expect(ERROR_UPSERT);
+        assert_eq!(cursor.seek_exact(key), Ok(Some((key, account))));
+
+        let mut dup_cursor = tx.cursor_dup_write::<PlainStorageState>().unwrap();
+        let subkey = H256::random();
+
+        let value = U256::from(1);
+        let entry1 = StorageEntry { key: subkey, value };
+        dup_cursor.upsert(key, entry1).expect(ERROR_UPSERT);
+        assert_eq!(dup_cursor.seek_by_key_subkey(key, subkey), Ok(Some(entry1)));
+
+        let value = U256::from(2);
+        let entry2 = StorageEntry { key: subkey, value };
+        dup_cursor.upsert(key, entry2).expect(ERROR_UPSERT);
+        assert_eq!(dup_cursor.seek_by_key_subkey(key, subkey), Ok(Some(entry1)));
+        assert_eq!(dup_cursor.next_dup_val(), Ok(Some(entry2)));
+    }
+
     #[test]
     fn db_cursor_dupsort_append() {
         let db: Arc<Env<WriteMap>> = test_utils::create_test_db(EnvKind::RW);