From e2cdd9964d84dba664bb2137d27dcffbc43d2ee1 Mon Sep 17 00:00:00 2001 From: 0xpause Date: Mon, 2 Oct 2023 23:13:13 +0800 Subject: [PATCH 1/5] resolve table size from remote and keep size increment in cache --- .../moveos-stdlib/doc/raw_table.md | 6 ++++ .../moveos-stdlib/sources/raw_table.move | 2 ++ .../natives/moveos_stdlib/raw_table/mod.rs | 35 +++++++++++++++---- moveos/moveos-store/src/lib.rs | 4 +++ .../moveos-store/src/state_store/statedb.rs | 17 +++++++++ moveos/moveos-types/src/object.rs | 6 +++- moveos/moveos-types/src/state.rs | 2 ++ moveos/moveos-types/src/state_resolver.rs | 7 ++++ moveos/moveos/src/vm/data_cache.rs | 12 +++++-- .../src/vm/unit_tests/vm_arguments_tests.rs | 4 +++ 10 files changed, 86 insertions(+), 9 deletions(-) diff --git a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md index 1a7847a63d..a63ced3701 100644 --- a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md +++ b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md @@ -50,6 +50,12 @@ This type table if for design internal global storage, so all functions are frie
+
+
+size: u64 +
+
+
diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move index bd5882c7a4..2c96e8c97d 100644 --- a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move +++ b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move @@ -21,6 +21,8 @@ module moveos_std::raw_table { struct TableInfo has key { // Table SMT root state_root: address, + // Table size, number of items + size: u64, } /// Add a new entry to the table. Aborts if an entry for this diff --git a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs index 61689b690f..4bbc731207 100644 --- a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs +++ b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs @@ -178,6 +178,7 @@ pub struct Table { handle: ObjectID, key_layout: MoveTypeLayout, content: BTreeMap, TableRuntimeValue>, + size_increment: i64, } // ========================================================================================= @@ -210,6 +211,7 @@ impl TableData { handle, key_layout, content: Default::default(), + size_increment: 0, }; if log::log_enabled!(log::Level::Trace) { let key_type = type_to_type_tag(context, key_ty)?; @@ -232,6 +234,7 @@ impl TableData { handle, key_layout, content: Default::default(), + size_increment: 0, }; e.insert(table) } @@ -354,13 +357,15 @@ impl Table { ObjectID, MoveTypeLayout, BTreeMap, TableRuntimeValue>, + i64, ) { let Table { handle, key_layout, content, + size_increment, } = self; - (handle, key_layout, content) + (handle, key_layout, content, size_increment) } pub fn key_layout(&self) -> &MoveTypeLayout { @@ -470,7 +475,10 @@ fn native_add_box( let value_layout = type_to_type_layout(context, &ty_args[1])?; let value_type = type_to_type_tag(context, &ty_args[1])?; match tv.move_to(val, value_layout, value_type) { - Ok(_) => Ok(NativeResult::ok(cost, smallvec![])), + Ok(_) => { + table.size_increment += 1; + Ok(NativeResult::ok(cost, smallvec![])) + } Err(_) => Ok(NativeResult::err( cost, moveos_types::move_std::error::already_exists(E_ALREADY_EXISTS), @@ -629,7 +637,10 @@ fn native_remove_box( cost += common_gas_params.calculate_load_cost(loaded); let value_type = type_to_type_tag(context, &ty_args[1])?; match tv.move_from(value_type) { - Ok(val) => Ok(NativeResult::ok(cost, smallvec![val])), + Ok(val) => { + table.size_increment -= 1; + Ok(NativeResult::ok(cost, smallvec![val])) + } Err(_) => Ok(NativeResult::err( cost, moveos_types::move_std::error::not_found(E_NOT_FOUND), @@ -665,9 +676,21 @@ fn native_destroy_empty_box( let mut table_data = table_context.table_data.write(); let handle = get_table_handle(pop_arg!(args, StructRef))?; - if table_data.tables.contains_key(&handle) - && !table_data.tables.get(&handle).unwrap().content.is_empty() - { + + let remote_table_size = table_context + .resolver + .resolve_size(&handle) + .map_err(|err| { + partial_extension_error(format!("remote table resolver failure: {}", err)) + })?; + let size_increment = if table_data.exist_table(&handle) { + table_data.borrow_table(&handle).unwrap().size_increment + } else { + 0i64 + }; + let updated_table_size = (remote_table_size as i64) + size_increment; + debug_assert!(updated_table_size >= 0); + if updated_table_size > 0 { return Ok(NativeResult::err( gas_params.base, moveos_types::move_std::error::invalid_state(E_NOT_EMPTY), diff --git a/moveos/moveos-store/src/lib.rs b/moveos/moveos-store/src/lib.rs index 9ccce1f383..2581015503 100644 --- a/moveos/moveos-store/src/lib.rs +++ b/moveos/moveos-store/src/lib.rs @@ -303,4 +303,8 @@ impl StateResolver for MoveOSStore { ) -> std::result::Result, State)>>, Error> { self.statedb.resolve_list_state(handle, cursor, limit) } + + fn resolve_size(&self, handle: &ObjectID) -> Result { + self.statedb.resolve_size(handle) + } } diff --git a/moveos/moveos-store/src/state_store/statedb.rs b/moveos/moveos-store/src/state_store/statedb.rs index 3531aca1e7..658d829c8b 100644 --- a/moveos/moveos-store/src/state_store/statedb.rs +++ b/moveos/moveos-store/src/state_store/statedb.rs @@ -256,10 +256,15 @@ impl StateDBStore { if table_handle == storage_context::GLOBAL_OBJECT_STORAGE_HANDLE { self.global_table .put_changes(table_change.entries.into_iter())?; + // TODO: do we need to update the size of global table? } else { let (mut object, table) = self.get_as_table_or_create(table_handle)?; let new_state_root = table.put_changes(table_change.entries.into_iter())?; object.value.state_root = AccountAddress::new(new_state_root.into()); + let curr_table_size: i64 = object.value.size as i64; + let updated_table_size = curr_table_size + table_change.size_increment; + debug_assert!(updated_table_size >= 0); + object.value.size = updated_table_size as u64; changed_objects.put(table_handle.to_bytes(), object.into()); } } @@ -306,6 +311,14 @@ impl StateDBStore { } } + pub fn resolve_size(&self, handle: &ObjectID) -> Result { + match self.get_as_table(*handle)? { + Some((table_info, _)) => Ok(table_info.value.size), + // TODO: table not exist, should we return 0 or error? + None => Ok(0u64), + } + } + // rebuild statedb via StateSet from dump pub fn apply(&self, state_set: StateSet) -> Result { let mut state_root = H256::zero(); @@ -383,4 +396,8 @@ impl StateResolver for StateDBStore { ) -> std::result::Result>, Error> { self.resolve_list_state(handle, cursor, limit) } + + fn resolve_size(&self, handle: &ObjectID) -> Result { + self.resolve_size(handle) + } } diff --git a/moveos/moveos-types/src/object.rs b/moveos/moveos-types/src/object.rs index 7ebb631b7f..9862a7e1d2 100644 --- a/moveos/moveos-types/src/object.rs +++ b/moveos/moveos-types/src/object.rs @@ -248,11 +248,15 @@ pub struct TableInfo { //TODO use u256? pub state_root: AccountAddress, //TODO keep Table Key TypeTag at here + pub size: u64, } impl TableInfo { pub fn new(state_root: AccountAddress) -> Self { - TableInfo { state_root } + TableInfo { + state_root, + size: 0u64, + } } } diff --git a/moveos/moveos-types/src/state.rs b/moveos/moveos-types/src/state.rs index 33d0a4276e..fe1fdc5002 100644 --- a/moveos/moveos-types/src/state.rs +++ b/moveos/moveos-types/src/state.rs @@ -425,6 +425,8 @@ impl StateChangeSet { pub struct TableChange { //TODO should we keep the key's type here? pub entries: BTreeMap, Op>, + /// The size increment of the table, may be negtive which means more deleting than inserting. + pub size_increment: i64, } /// StateSet is represent state dump result. Not include events and other stores diff --git a/moveos/moveos-types/src/state_resolver.rs b/moveos/moveos-types/src/state_resolver.rs index 6889d38dc3..4bdd9c1ee0 100644 --- a/moveos/moveos-types/src/state_resolver.rs +++ b/moveos/moveos-types/src/state_resolver.rs @@ -34,6 +34,9 @@ pub trait StateResolver { cursor: Option>, limit: usize, ) -> Result>, anyhow::Error>; + + /// Resolve the table size if the handle is a table. + fn resolve_size(&self, handle: &ObjectID) -> Result; } /// A proxy type for proxy the StateResolver to MoveResolver @@ -103,6 +106,10 @@ where ) -> Result>, anyhow::Error> { self.0.resolve_list_state(handle, cursor, limit) } + + fn resolve_size(&self, handle: &ObjectID) -> Result { + self.0.resolve_size(handle) + } } pub trait MoveOSResolver: MoveResolver + StateResolver {} diff --git a/moveos/moveos/src/vm/data_cache.rs b/moveos/moveos/src/vm/data_cache.rs index 26170f0358..dfd9ce0bd5 100644 --- a/moveos/moveos/src/vm/data_cache.rs +++ b/moveos/moveos/src/vm/data_cache.rs @@ -270,7 +270,7 @@ pub fn into_change_set(table_data: Arc>) -> PartialVMResult>) -> PartialVMResult anyhow::Result>, anyhow::Error> { todo!() } + + fn resolve_size(&self, handle: &ObjectID) -> anyhow::Result { + todo!() + } } fn combine_signers_and_args( From cac3a854978e70f38e403c4ea5fd12d855105d0c Mon Sep 17 00:00:00 2001 From: 0xpause Date: Wed, 4 Oct 2023 11:16:53 +0800 Subject: [PATCH 2/5] fix and add test --- .../tests/cases/table/test_destroy.exp | 16 ++-- .../tests/cases/table/test_destroy.move | 51 +++++++++++- .../natives/gas_parameter/table_extension.rs | 3 +- .../moveos-stdlib/doc/raw_table.md | 83 ++++++++++++++++++- .../moveos-stdlib/moveos-stdlib/doc/table.md | 80 ++++++++++++++++++ .../moveos-stdlib/sources/raw_table.move | 22 +++-- .../moveos-stdlib/sources/table.move | 16 ++++ .../natives/moveos_stdlib/raw_table/mod.rs | 69 +++++++++++---- moveos/moveos/src/gas/parameter.rs | 1 + 9 files changed, 308 insertions(+), 33 deletions(-) diff --git a/crates/rooch-framework-tests/tests/cases/table/test_destroy.exp b/crates/rooch-framework-tests/tests/cases/table/test_destroy.exp index 286a73e037..b1b549227d 100644 --- a/crates/rooch-framework-tests/tests/cases/table/test_destroy.exp +++ b/crates/rooch-framework-tests/tests/cases/table/test_destroy.exp @@ -1,13 +1,19 @@ -processed 5 tasks +processed 7 tasks -task 1 'publish'. lines 3-73: +task 1 'publish'. lines 3-85: status EXECUTED -task 2 'run'. lines 75-88: +task 2 'run'. lines 87-101: status EXECUTED -task 3 'run'. lines 89-104: +task 3 'run'. lines 102-117: status EXECUTED -task 4 'run'. lines 105-115: +task 4 'run'. lines 118-133: +status EXECUTED + +task 5 'run'. lines 134-146: +status ABORTED with code 3 in 0000000000000000000000000000000000000000000000000000000000000002::raw_table + +task 6 'run'. lines 147-160: status EXECUTED diff --git a/crates/rooch-framework-tests/tests/cases/table/test_destroy.move b/crates/rooch-framework-tests/tests/cases/table/test_destroy.move index 2608de05be..0b5a5fa1fa 100644 --- a/crates/rooch-framework-tests/tests/cases/table/test_destroy.move +++ b/crates/rooch-framework-tests/tests/cases/table/test_destroy.move @@ -26,6 +26,10 @@ module test::m { table::add(&mut store.table, key, value); } + public fun remove(store: &mut KVStore, key: String): vector { + table::remove(&mut store.table, key) + } + public fun contains(store: &KVStore, key: String): bool { table::contains(&store.table, key) } @@ -66,6 +70,14 @@ module test::m { account_storage::global_move_from(ctx, account) } + public fun length(kv: &KVStore): u64 { + table::length(&kv.table) + } + + public fun is_empty(kv: &KVStore): bool { + table::length(&kv.table) == 0 + } + public fun destroy(kv: KVStore){ let KVStore{table} = kv; table::destroy_empty(table); @@ -81,6 +93,7 @@ script { fun main(ctx: &mut StorageContext, sender: signer) { let kv = m::make_kv_store(ctx); m::add(&mut kv, string::utf8(b"test"), b"value"); + assert!(m::length(&kv) == 1, 1000); // check length is correct when data in table cache m::save_to_account_storage(ctx, &sender, kv); } } @@ -95,21 +108,53 @@ script { fun main(ctx: &mut StorageContext) { let sender = storage_context::sender(ctx); let kv = m::borrow_from_account_storage(ctx, sender); - assert!(m::contains(kv, string::utf8(b"test")), 1000); + assert!(m::contains(kv, string::utf8(b"test")), 1001); let v = m::borrow(kv, string::utf8(b"test")); - assert!(v == &b"value", 1001); + assert!(v == &b"value", 1002); } } -//FIXME destroy no empty table, should failed. +// check length when data is in both remote storage and cache //# run --signers test script { + use std::string; use moveos_std::storage_context::{Self, StorageContext}; use test::m; + fun main(ctx: &mut StorageContext) { + let sender = storage_context::sender(ctx); + let kv = m::borrow_mut_from_account_storage(ctx, sender); + m::add(kv, string::utf8(b"test1"), b"value1"); + assert!(m::length(kv) == 2, 1003); + let _value = m::remove(kv, string::utf8(b"test1")); + } +} + +// destroy none empty table, should failed. +//# run --signers test +script { + use moveos_std::storage_context::{Self, StorageContext}; + use test::m; + + fun main(ctx: &mut StorageContext) { + let sender = storage_context::sender(ctx); + let kv = m::move_from_account_storage(ctx, sender); + m::destroy(kv); + } +} + +// destroy empty table, should success. +//# run --signers test +script { + use std::string; + use moveos_std::storage_context::{Self, StorageContext}; + use test::m; + fun main(ctx: &mut StorageContext) { let sender = storage_context::sender(ctx); let kv = m::move_from_account_storage(ctx, sender); + let v = m::remove(&mut kv, string::utf8(b"test")); + assert!(v == b"value", 1004); m::destroy(kv); } } \ No newline at end of file diff --git a/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs b/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs index 0d570e01ea..087b48908e 100644 --- a/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs +++ b/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs @@ -16,6 +16,7 @@ crate::natives::gas_parameter::native::define_gas_parameters_for_natives!(GasPar [.contains_box.per_byte_serialized, "contains_box.per_byte_serialized", (5 + 1) * MUL], [.remove_box.base, "remove_box.base", (5 + 1) * MUL], [.remove_box.per_byte_serialized, "remove_box.per_byte_serialized", (5 + 1) * MUL], - [.destroy_empty_box.base, "destroy_empty_box.base", (5 + 1) * MUL], + [.destroy_empty_box_unchecked.base, "destroy_empty_box_unchecked.base", (5 + 1) * MUL], [.drop_unchecked_box.base, "drop_unchecked_box.base", (5 + 1) * MUL], + [.box_length.base, "box_length.base", (5 + 1) * MUL], ]); diff --git a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md index a63ced3701..e4c11a9a76 100644 --- a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md +++ b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md @@ -19,6 +19,9 @@ This type table if for design internal global storage, so all functions are frie - [Function `upsert`](#0x2_raw_table_upsert) - [Function `remove`](#0x2_raw_table_remove) - [Function `contains`](#0x2_raw_table_contains) +- [Function `length`](#0x2_raw_table_length) +- [Function `is_empty`](#0x2_raw_table_is_empty) +- [Function `drop_unchecked`](#0x2_raw_table_drop_unchecked) - [Function `destroy_empty`](#0x2_raw_table_destroy_empty) - [Function `new_table_handle`](#0x2_raw_table_new_table_handle) @@ -345,13 +348,88 @@ Returns true if table contains an + + + + +## Function `length` + +Returns the size of the table, the number of key-value pairs + + +
public fun length(table_handle: &object_id::ObjectID): u64
+
+ + + +
+Implementation + + +
public fun length(table_handle: &ObjectID): u64 {
+    box_length(table_handle)
+}
+
+ + + +
+ + + +## Function `is_empty` + +Returns true iff the table is empty (if length returns 0) + + +
public fun is_empty(table_handle: &object_id::ObjectID): bool
+
+ + + +
+Implementation + + +
public fun is_empty(table_handle: &ObjectID): bool {
+    length(table_handle) == 0
+}
+
+ + + +
+ + + +## Function `drop_unchecked` + +Testing only: allows to drop a table even if it is not empty. + + +
public(friend) fun drop_unchecked(table_handle: &object_id::ObjectID)
+
+ + + +
+Implementation + + +
public(friend) fun drop_unchecked(table_handle: &ObjectID) {
+    destroy_empty_box_unchecked(table_handle)
+}
+
+ + +
## Function `destroy_empty` -Destroy a table. The table must be empty to succeed. +Destroy a table. Aborts if the table is not empty 
public(friend) fun destroy_empty(table_handle: &object_id::ObjectID)
@@ -364,7 +442,8 @@ Destroy a table. The table must be empty to succeed.
 
 
 
public(friend) fun destroy_empty(table_handle: &ObjectID) {
-    destroy_empty_box(table_handle)
+    assert!(is_empty(table_handle), ErrorNotEmpty);
+    destroy_empty_box_unchecked(table_handle)
 }
 

 
diff --git a/moveos/moveos-stdlib/moveos-stdlib/doc/table.md b/moveos/moveos-stdlib/moveos-stdlib/doc/table.md
index bf465beae6..3cf4461700 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/doc/table.md
+++ b/moveos/moveos-stdlib/moveos-stdlib/doc/table.md
@@ -23,6 +23,9 @@ struct itself, while the operations are implemented as native functions. No trav
 -  [Function `remove`](#0x2_table_remove)
 -  [Function `contains`](#0x2_table_contains)
 -  [Function `destroy_empty`](#0x2_table_destroy_empty)
+-  [Function `length`](#0x2_table_length)
+-  [Function `is_empty`](#0x2_table_is_empty)
+-  [Function `drop`](#0x2_table_drop)
 
 
 
use 0x2::object_id;
@@ -346,4 +349,81 @@ Destroy a table. The table must be empty to succeed.
 
 
 
+
+
+
+
+## Function `length`
+
+Returns the size of the table, the number of key-value pairs
+
+
+
public fun length<K: copy, drop, V>(table: &table::Table<K, V>): u64
+

+
+
+
+

+Implementation
+
+
+
public fun length<K: copy + drop, V>(table: &Table<K, V>): u64 {
+    raw_table::length(&table.handle)
+}
+

+
+
+
+

+
+
+
+## Function `is_empty`
+
+Returns true iff the table is empty (if length returns 0)
+
+
+
public fun is_empty<K: copy, drop, V>(table: &table::Table<K, V>): bool
+

+
+
+
+

+Implementation
+
+
+
public fun is_empty<K: copy + drop, V>(table: &Table<K, V>): bool {
+    raw_table::length(&table.handle) == 0
+}
+

+
+
+
+

+
+
+
+## Function `drop`
+
+Drop a possibly non-empty table.
+Usable only if the value type V has the drop ability
+
+
+
public fun drop<K: copy, drop, V: drop>(table: table::Table<K, V>)
+

+
+
+
+

+Implementation
+
+
+
public fun drop<K: copy + drop , V: drop>(table: Table<K, V>) {
+    let Table { handle } = table;
+    raw_table::drop_unchecked(&handle)
+}
+

+
+
+
 

diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
index 2c96e8c97d..ca1eab438a 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
+++ b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
@@ -86,15 +86,25 @@ module moveos_std::raw_table {
         contains_box(table_handle, key)
     }
 
-    #[test_only]
+    /// Returns the size of the table, the number of key-value pairs
+    public fun length(table_handle: &ObjectID): u64 {
+        box_length(table_handle)
+    }
+
+    /// Returns true iff the table is empty (if `length` returns `0`)
+    public fun is_empty(table_handle: &ObjectID): bool {
+        length(table_handle) == 0
+    }
+
     /// Testing only: allows to drop a table even if it is not empty.
     public(friend) fun drop_unchecked(table_handle: &ObjectID) {
-        drop_unchecked_box(table_handle)
+        destroy_empty_box_unchecked(table_handle)
     }
     
-    /// Destroy a table. The table must be empty to succeed.
+    /// Destroy a table. Aborts if the table is not empty
     public(friend) fun destroy_empty(table_handle: &ObjectID) {
-        destroy_empty_box(table_handle)
+        assert!(is_empty(table_handle), ErrorNotEmpty);
+        destroy_empty_box_unchecked(table_handle)
     }
 
     // ======================================================================================================
@@ -120,7 +130,9 @@ module moveos_std::raw_table {
 
     native fun remove_box(table_handle: &ObjectID, key: K): Box;
 
-    native fun destroy_empty_box(table_handle: &ObjectID);
+    native fun destroy_empty_box_unchecked(table_handle: &ObjectID);
 
     native fun drop_unchecked_box(table_handle: &ObjectID);
+
+    native fun box_length(table_handle: &ObjectID): u64;
 }
diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
index ef2afe5da7..6e41ec310c 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
+++ b/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
@@ -92,6 +92,22 @@ module moveos_std::table {
         raw_table::destroy_empty(&handle)
     }
 
+    /// Returns the size of the table, the number of key-value pairs
+    public fun length(table: &Table): u64 {
+        raw_table::length(&table.handle)
+    }
+
+    /// Returns true iff the table is empty (if `length` returns `0`)
+    public fun is_empty(table: &Table): bool {
+        raw_table::length(&table.handle) == 0
+    }
+
+    /// Drop a possibly non-empty table.
+    /// Usable only if the value type `V` has the `drop` ability
+    public fun drop(table: Table) {
+        let Table { handle } = table;
+        raw_table::drop_unchecked(&handle)
+    }
 
     #[test_only]
     struct TableHolder has key {
diff --git a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
index 4bbc731207..c20250cbef 100644
--- a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
+++ b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
@@ -47,7 +47,6 @@ pub struct NativeTableContext<'a> {
 /// Ensure the error codes in this file is consistent with the error code in raw_table.move
 const E_ALREADY_EXISTS: u64 = 1;
 const E_NOT_FOUND: u64 = 2;
-const E_NOT_EMPTY: u64 = 3;
 
 // ===========================================================================================
 // Private Data Structures and Constants
@@ -378,7 +377,7 @@ impl Table {
 
 /// Returns all natives for tables.
 pub fn table_natives(table_addr: AccountAddress, gas_params: GasParameters) -> NativeFunctionTable {
-    let natives: [(&str, &str, NativeFunction); 7] = [
+    let natives: [(&str, &str, NativeFunction); 8] = [
         (
             "raw_table",
             "add_box",
@@ -406,14 +405,19 @@ pub fn table_natives(table_addr: AccountAddress, gas_params: GasParameters) -> N
         ),
         (
             "raw_table",
-            "destroy_empty_box",
-            make_native_destroy_empty_box(gas_params.destroy_empty_box),
+            "destroy_empty_box_unchecked",
+            make_native_destroy_empty_box_unchecked(gas_params.destroy_empty_box_unchecked),
         ),
         (
             "raw_table",
             "drop_unchecked_box",
             make_native_drop_unchecked_box(gas_params.drop_unchecked_box),
         ),
+        (
+            "raw_table",
+            "box_length",
+            make_native_box_length(gas_params.box_length),
+        ),
     ];
 
     native_functions::make_table_from_iter(table_addr, natives)
@@ -664,7 +668,7 @@ pub struct DestroyEmptyBoxGasParameters {
     pub base: InternalGas,
 }
 
-fn native_destroy_empty_box(
+fn native_destroy_empty_box_unchecked(
     gas_params: &DestroyEmptyBoxGasParameters,
     context: &mut NativeContext,
     _ty_args: Vec,
@@ -677,6 +681,39 @@ fn native_destroy_empty_box(
 
     let handle = get_table_handle(pop_arg!(args, StructRef))?;
 
+    assert!(table_data.removed_tables.insert(handle));
+
+    Ok(NativeResult::ok(gas_params.base, smallvec![]))
+}
+
+pub fn make_native_destroy_empty_box_unchecked(
+    gas_params: DestroyEmptyBoxGasParameters,
+) -> NativeFunction {
+    Arc::new(
+        move |context, ty_args, args| -> PartialVMResult {
+            native_destroy_empty_box_unchecked(&gas_params, context, ty_args, args)
+        },
+    )
+}
+
+#[derive(Debug, Clone)]
+pub struct BoxLengthGasParameters {
+    pub base: InternalGas,
+}
+
+fn native_box_length(
+    gas_params: &BoxLengthGasParameters,
+    context: &mut NativeContext,
+    _ty_args: Vec,
+    mut args: VecDeque,
+) -> PartialVMResult {
+    assert_eq!(args.len(), 1);
+
+    let table_context = context.extensions().get::();
+    let table_data = table_context.table_data.write();
+
+    let handle = get_table_handle(pop_arg!(args, StructRef))?;
+
     let remote_table_size = table_context
         .resolver
         .resolve_size(&handle)
@@ -690,21 +727,17 @@ fn native_destroy_empty_box(
     };
     let updated_table_size = (remote_table_size as i64) + size_increment;
     debug_assert!(updated_table_size >= 0);
-    if updated_table_size > 0 {
-        return Ok(NativeResult::err(
-            gas_params.base,
-            moveos_types::move_std::error::invalid_state(E_NOT_EMPTY),
-        ));
-    }
-    assert!(table_data.removed_tables.insert(handle));
 
-    Ok(NativeResult::ok(gas_params.base, smallvec![]))
+    let length = Value::u64(updated_table_size as u64);
+    let cost = gas_params.base;
+
+    Ok(NativeResult::ok(cost, smallvec![length]))
 }
 
-pub fn make_native_destroy_empty_box(gas_params: DestroyEmptyBoxGasParameters) -> NativeFunction {
+pub fn make_native_box_length(gas_params: BoxLengthGasParameters) -> NativeFunction {
     Arc::new(
         move |context, ty_args, args| -> PartialVMResult {
-            native_destroy_empty_box(&gas_params, context, ty_args, args)
+            native_box_length(&gas_params, context, ty_args, args)
         },
     )
 }
@@ -740,8 +773,9 @@ pub struct GasParameters {
     pub borrow_box: BorrowBoxGasParameters,
     pub contains_box: ContainsBoxGasParameters,
     pub remove_box: RemoveGasParameters,
-    pub destroy_empty_box: DestroyEmptyBoxGasParameters,
+    pub destroy_empty_box_unchecked: DestroyEmptyBoxGasParameters,
     pub drop_unchecked_box: DropUncheckedBoxGasParameters,
+    pub box_length: BoxLengthGasParameters,
 }
 
 impl GasParameters {
@@ -768,8 +802,9 @@ impl GasParameters {
                 base: 0.into(),
                 per_byte_serialized: 0.into(),
             },
-            destroy_empty_box: DestroyEmptyBoxGasParameters { base: 0.into() },
+            destroy_empty_box_unchecked: DestroyEmptyBoxGasParameters { base: 0.into() },
             drop_unchecked_box: DropUncheckedBoxGasParameters { base: 0.into() },
+            box_length: BoxLengthGasParameters { base: 0.into() },
         }
     }
 }
diff --git a/moveos/moveos/src/gas/parameter.rs b/moveos/moveos/src/gas/parameter.rs
index 5aadb9eaea..7f9bbaaec1 100644
--- a/moveos/moveos/src/gas/parameter.rs
+++ b/moveos/moveos/src/gas/parameter.rs
@@ -496,6 +496,7 @@ static G_NATIVE_STRS: Lazy> = Lazy::new(|| {
         "table.contains_box.per_byte_serialized",
         "table.destroy_empty_box.base",
         "table.drop_unchecked_box.base",
+        "table.box_length.base",
         "move_stdlib.string.check_utf8.per_byte",
         "move_stdlib.string.sub_string.per_byte",
         "move_stdlib.string.is_char_boundary.base",

From 3cd738fa848f2780b2f97500cc0f885eecf5453e Mon Sep 17 00:00:00 2001
From: 0xpause 
Date: Wed, 4 Oct 2023 11:49:54 +0800
Subject: [PATCH 3/5] fix

---
 moveos/moveos/src/vm/unit_tests/vm_arguments_tests.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/moveos/moveos/src/vm/unit_tests/vm_arguments_tests.rs b/moveos/moveos/src/vm/unit_tests/vm_arguments_tests.rs
index b3f7307c1b..e15aa0dc52 100644
--- a/moveos/moveos/src/vm/unit_tests/vm_arguments_tests.rs
+++ b/moveos/moveos/src/vm/unit_tests/vm_arguments_tests.rs
@@ -297,7 +297,7 @@ impl StateResolver for RemoteStore {
         todo!()
     }
 
-    fn resolve_size(&self, handle: &ObjectID) -> anyhow::Result {
+    fn resolve_size(&self, _handle: &ObjectID) -> anyhow::Result {
         todo!()
     }
 }

From 65b4ac4e5b0396a32a26e13a3de2920b18223591 Mon Sep 17 00:00:00 2001
From: 0xpause 
Date: Wed, 4 Oct 2023 12:35:24 +0800
Subject: [PATCH 4/5] Merge the drop_unchecked_box and
 destroy_empty_box_unchecked

---
 .../natives/gas_parameter/table_extension.rs  |  1 -
 .../moveos-stdlib/doc/raw_table.md            |  4 +-
 .../moveos-stdlib/sources/raw_table.move      |  6 +-
 .../natives/moveos_stdlib/raw_table/mod.rs    | 55 ++++---------------
 4 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs b/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs
index 087b48908e..1a9131084e 100644
--- a/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs
+++ b/crates/rooch-framework/src/natives/gas_parameter/table_extension.rs
@@ -16,7 +16,6 @@ crate::natives::gas_parameter::native::define_gas_parameters_for_natives!(GasPar
     [.contains_box.per_byte_serialized, "contains_box.per_byte_serialized", (5 + 1) * MUL],
     [.remove_box.base, "remove_box.base", (5 + 1) * MUL],
     [.remove_box.per_byte_serialized, "remove_box.per_byte_serialized", (5 + 1) * MUL],
-    [.destroy_empty_box_unchecked.base, "destroy_empty_box_unchecked.base", (5 + 1) * MUL],
     [.drop_unchecked_box.base, "drop_unchecked_box.base", (5 + 1) * MUL],
     [.box_length.base, "box_length.base", (5 + 1) * MUL],
 ]);
diff --git a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
index e4c11a9a76..537b6c5927 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
+++ b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
@@ -417,7 +417,7 @@ Testing only: allows to drop a table even if it is not empty.
 
 
 
public(friend) fun drop_unchecked(table_handle: &ObjectID) {
-    destroy_empty_box_unchecked(table_handle)
+    drop_unchecked_box(table_handle)
 }
 

 
@@ -443,7 +443,7 @@ Destroy a table. Aborts if the table is not empty
 
 
public(friend) fun destroy_empty(table_handle: &ObjectID) {
     assert!(is_empty(table_handle), ErrorNotEmpty);
-    destroy_empty_box_unchecked(table_handle)
+    drop_unchecked_box(table_handle)
 }
 

 
diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
index ca1eab438a..08902a25c2 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
+++ b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
@@ -98,13 +98,13 @@ module moveos_std::raw_table {
 
     /// Testing only: allows to drop a table even if it is not empty.
     public(friend) fun drop_unchecked(table_handle: &ObjectID) {
-        destroy_empty_box_unchecked(table_handle)
+        drop_unchecked_box(table_handle)
     }
     
     /// Destroy a table. Aborts if the table is not empty
     public(friend) fun destroy_empty(table_handle: &ObjectID) {
         assert!(is_empty(table_handle), ErrorNotEmpty);
-        destroy_empty_box_unchecked(table_handle)
+        drop_unchecked_box(table_handle)
     }
 
     // ======================================================================================================
@@ -130,8 +130,6 @@ module moveos_std::raw_table {
 
     native fun remove_box(table_handle: &ObjectID, key: K): Box;
 
-    native fun destroy_empty_box_unchecked(table_handle: &ObjectID);
-
     native fun drop_unchecked_box(table_handle: &ObjectID);
 
     native fun box_length(table_handle: &ObjectID): u64;
diff --git a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
index c20250cbef..05e33a03c0 100644
--- a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
+++ b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
@@ -377,7 +377,7 @@ impl Table {
 
 /// Returns all natives for tables.
 pub fn table_natives(table_addr: AccountAddress, gas_params: GasParameters) -> NativeFunctionTable {
-    let natives: [(&str, &str, NativeFunction); 8] = [
+    let natives: [(&str, &str, NativeFunction); 7] = [
         (
             "raw_table",
             "add_box",
@@ -403,11 +403,6 @@ pub fn table_natives(table_addr: AccountAddress, gas_params: GasParameters) -> N
             "contains_box",
             make_native_contains_box(gas_params.common, gas_params.contains_box),
         ),
-        (
-            "raw_table",
-            "destroy_empty_box_unchecked",
-            make_native_destroy_empty_box_unchecked(gas_params.destroy_empty_box_unchecked),
-        ),
         (
             "raw_table",
             "drop_unchecked_box",
@@ -663,39 +658,6 @@ pub fn make_native_remove_box(
     )
 }
 
-#[derive(Debug, Clone)]
-pub struct DestroyEmptyBoxGasParameters {
-    pub base: InternalGas,
-}
-
-fn native_destroy_empty_box_unchecked(
-    gas_params: &DestroyEmptyBoxGasParameters,
-    context: &mut NativeContext,
-    _ty_args: Vec,
-    mut args: VecDeque,
-) -> PartialVMResult {
-    assert_eq!(args.len(), 1);
-
-    let table_context = context.extensions().get::();
-    let mut table_data = table_context.table_data.write();
-
-    let handle = get_table_handle(pop_arg!(args, StructRef))?;
-
-    assert!(table_data.removed_tables.insert(handle));
-
-    Ok(NativeResult::ok(gas_params.base, smallvec![]))
-}
-
-pub fn make_native_destroy_empty_box_unchecked(
-    gas_params: DestroyEmptyBoxGasParameters,
-) -> NativeFunction {
-    Arc::new(
-        move |context, ty_args, args| -> PartialVMResult {
-            native_destroy_empty_box_unchecked(&gas_params, context, ty_args, args)
-        },
-    )
-}
-
 #[derive(Debug, Clone)]
 pub struct BoxLengthGasParameters {
     pub base: InternalGas,
@@ -749,12 +711,19 @@ pub struct DropUncheckedBoxGasParameters {
 
 fn native_drop_unchecked_box(
     gas_params: &DropUncheckedBoxGasParameters,
-    _context: &mut NativeContext,
+    context: &mut NativeContext,
     _ty_args: Vec,
-    args: VecDeque,
+    mut args: VecDeque,
 ) -> PartialVMResult {
     assert_eq!(args.len(), 1);
-    //TODO remove the droped table from the table_data
+
+    let table_context = context.extensions().get::();
+    let mut table_data = table_context.table_data.write();
+
+    let handle = get_table_handle(pop_arg!(args, StructRef))?;
+
+    assert!(table_data.removed_tables.insert(handle));
+
     Ok(NativeResult::ok(gas_params.base, smallvec![]))
 }
 
@@ -773,7 +742,6 @@ pub struct GasParameters {
     pub borrow_box: BorrowBoxGasParameters,
     pub contains_box: ContainsBoxGasParameters,
     pub remove_box: RemoveGasParameters,
-    pub destroy_empty_box_unchecked: DestroyEmptyBoxGasParameters,
     pub drop_unchecked_box: DropUncheckedBoxGasParameters,
     pub box_length: BoxLengthGasParameters,
 }
@@ -802,7 +770,6 @@ impl GasParameters {
                 base: 0.into(),
                 per_byte_serialized: 0.into(),
             },
-            destroy_empty_box_unchecked: DestroyEmptyBoxGasParameters { base: 0.into() },
             drop_unchecked_box: DropUncheckedBoxGasParameters { base: 0.into() },
             box_length: BoxLengthGasParameters { base: 0.into() },
         }

From 1fa9684f3489c0d567da547f150196f0ed1c0dba Mon Sep 17 00:00:00 2001
From: 0xpause 
Date: Wed, 4 Oct 2023 14:19:53 +0800
Subject: [PATCH 5/5] fix

---
 moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md  |  8 ++++----
 .../moveos-stdlib/sources/raw_table.move             |  4 ++--
 .../moveos-stdlib/moveos-stdlib/sources/table.move   | 10 +++++-----
 .../src/natives/moveos_stdlib/raw_table/mod.rs       | 12 +++++++++---
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
index 537b6c5927..7f5fd5871f 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
+++ b/moveos/moveos-stdlib/moveos-stdlib/doc/raw_table.md
@@ -357,7 +357,7 @@ Returns true if table contains an
 Returns the size of the table, the number of key-value pairs
 
 
-
public fun length(table_handle: &object_id::ObjectID): u64
+
public(friend) fun length(table_handle: &object_id::ObjectID): u64
 

 
 
@@ -366,7 +366,7 @@ Returns the size of the table, the number of key-value pairs
 Implementation
 
 
-
public fun length(table_handle: &ObjectID): u64 {
+
public(friend) fun length(table_handle: &ObjectID): u64 {
     box_length(table_handle)
 }
 

@@ -382,7 +382,7 @@ Returns the size of the table, the number of key-value pairs
 Returns true iff the table is empty (if length returns 0)
 
 
-
public fun is_empty(table_handle: &object_id::ObjectID): bool
+
public(friend) fun is_empty(table_handle: &object_id::ObjectID): bool
 

 
 
@@ -391,7 +391,7 @@ Returns true iff the table is empty (if length returns 0Implementation
 
 
-
public fun is_empty(table_handle: &ObjectID): bool {
+
public(friend) fun is_empty(table_handle: &ObjectID): bool {
     length(table_handle) == 0
 }
 

diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
index 08902a25c2..a828157c13 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
+++ b/moveos/moveos-stdlib/moveos-stdlib/sources/raw_table.move
@@ -87,12 +87,12 @@ module moveos_std::raw_table {
     }
 
     /// Returns the size of the table, the number of key-value pairs
-    public fun length(table_handle: &ObjectID): u64 {
+    public(friend) fun length(table_handle: &ObjectID): u64 {
         box_length(table_handle)
     }
 
     /// Returns true iff the table is empty (if `length` returns `0`)
-    public fun is_empty(table_handle: &ObjectID): bool {
+    public(friend) fun is_empty(table_handle: &ObjectID): bool {
         length(table_handle) == 0
     }
 
diff --git a/moveos/moveos-stdlib/moveos-stdlib/sources/table.move b/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
index 6e41ec310c..0af0f19fdd 100644
--- a/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
+++ b/moveos/moveos-stdlib/moveos-stdlib/sources/table.move
@@ -230,15 +230,15 @@ module moveos_std::table {
 
         let t2 = new_with_id(copy t2_id);
         assert!(contains(&t2, t2_key), 2);
-        drop_unchecked(t2);
+        let Table { handle: _ } = t2;
 
         let borrowed_mut_t2 = borrow_mut(&mut t1, t1_key);
         remove(borrowed_mut_t2, t2_key);
 
-        let t2 = new_with_id(t2_id);
-        assert!(!contains(&t2, t2_key), 2);
-        drop_unchecked(t2);
-
+        let t3 = new_with_id(t2_id);
+        assert!(!contains(&t3, t2_key), 2);
+        
+        drop_unchecked(t3); // No need to drop t2 as t2 shares same handle with t3
         drop_unchecked(t1);
     }
 
diff --git a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
index 05e33a03c0..14be1a7ec4 100644
--- a/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
+++ b/moveos/moveos-stdlib/src/natives/moveos_stdlib/raw_table/mod.rs
@@ -47,6 +47,7 @@ pub struct NativeTableContext<'a> {
 /// Ensure the error codes in this file is consistent with the error code in raw_table.move
 const E_ALREADY_EXISTS: u64 = 1;
 const E_NOT_FOUND: u64 = 2;
+const E_DUPLICATE_OPERATION: u64 = 3;
 
 // ===========================================================================================
 // Private Data Structures and Constants
@@ -722,9 +723,14 @@ fn native_drop_unchecked_box(
 
     let handle = get_table_handle(pop_arg!(args, StructRef))?;
 
-    assert!(table_data.removed_tables.insert(handle));
-
-    Ok(NativeResult::ok(gas_params.base, smallvec![]))
+    if table_data.removed_tables.insert(handle) {
+        Ok(NativeResult::ok(gas_params.base, smallvec![]))
+    } else {
+        Ok(NativeResult::err(
+            gas_params.base,
+            moveos_types::move_std::error::not_found(E_DUPLICATE_OPERATION),
+        ))
+    }
 }
 
 pub fn make_native_drop_unchecked_box(gas_params: DropUncheckedBoxGasParameters) -> NativeFunction {