diff --git a/aptos-move/framework/aptos-framework/doc/aggregator_v2.md b/aptos-move/framework/aptos-framework/doc/aggregator_v2.md index 8b9f822d0b515..9fd3230df6d94 100644 --- a/aptos-move/framework/aptos-framework/doc/aggregator_v2.md +++ b/aptos-move/framework/aptos-framework/doc/aggregator_v2.md @@ -11,9 +11,8 @@ across multiple transactions, enabling parallel execution. For example, if the first transaction is doing try_add(X, 1) for aggregator X, and the second is doing try_sub(X,3), they can be executed in parallel avoiding a read-modify-write dependency. -However, reading the aggregator value (i.e. calling read(X)) is an expensive -operation and should be avoided as much as possible because it reduces the -parallelism. +However, reading the aggregator value (i.e. calling read(X)) is a resource-intensive +operation that also reduced parallelism, and should be avoided as much as possible. - [Struct `Aggregator`](#0x1_aggregator_v2_Aggregator) @@ -32,6 +31,7 @@ parallelism. - [Function `copy_snapshot`](#0x1_aggregator_v2_copy_snapshot) - [Function `read_snapshot`](#0x1_aggregator_v2_read_snapshot) - [Function `string_concat`](#0x1_aggregator_v2_string_concat) +- [Function `test_aggregator_valid_type`](#0x1_aggregator_v2_test_aggregator_valid_type) - [Specification](#@Specification_1) - [Function `create_aggregator`](#@Specification_1_create_aggregator) - [Function `try_add`](#@Specification_1_try_add) @@ -59,7 +59,7 @@ across multiple transactions. See the module description for more details. Currently supported types for Element are u64 and u128. -
struct Aggregator<IntElement> has store
+
struct Aggregator<IntElement> has drop, store
 

 
 
@@ -141,23 +141,23 @@ The value of aggregator underflows (goes below zero). Raised by uncoditional sub
 
 
 
-
+
 
-The native aggregator function, that is in the move file, is not yet supported.
-and any calls will raise this error.
+The aggregator api v2 feature flag is not enabled.
 
 
-
const EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED: u64 = 9;
+
const EAGGREGATOR_API_V2_NOT_ENABLED: u64 = 6;
 

 
 
 
-
+
 
-The aggregator snapshots feature flag is not enabled.
+The native aggregator function, that is in the move file, is not yet supported.
+and any calls will raise this error.
 
 
-
const EAGGREGATOR_SNAPSHOTS_NOT_ENABLED: u64 = 6;
+
const EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED: u64 = 9;
 

 
 
@@ -371,7 +371,9 @@ If subtraction would result in a negative value, false is re
 ## Function `read`
 
 Returns a value stored in this aggregator.
-Note: This operation prevents parallelism of the transaction that calls it.
+Note: This operation is resource-intensive, and reduces parallelism.
+(Especially if called in a transaction that also modifies the aggregator,
+or has other read/write conflicts)
 
 
 
public fun read<IntElement>(aggregator: &aggregator_v2::Aggregator<IntElement>): IntElement
@@ -395,7 +397,7 @@ Note: This operation prevents parallelism of the transaction that calls it.
 ## Function `snapshot`
 
 Returns a wrapper of a current value of an aggregator
-Unlike read(), this enables parallel execution of transactions.
+Unlike read(), it is fast and avoids sequential dependencies.
 
 
 
public fun snapshot<IntElement>(aggregator: &aggregator_v2::Aggregator<IntElement>): aggregator_v2::AggregatorSnapshot<IntElement>
@@ -466,7 +468,9 @@ NOT YET IMPLEMENTED, always raises EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED.
 ## Function `read_snapshot`
 
 Returns a value stored in this snapshot.
-Note: This operation prevents parallelism of the transaction that calls it.
+Note: This operation is resource-intensive, and reduces parallelism.
+(Especially if called in a transaction that also modifies the aggregator,
+or has other read/write conflicts)
 
 
 
public fun read_snapshot<Element>(snapshot: &aggregator_v2::AggregatorSnapshot<Element>): Element
@@ -509,6 +513,33 @@ If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_
 
 
 
+
+
+
+
+## Function `test_aggregator_valid_type`
+
+
+
+
public fun test_aggregator_valid_type()
+

+
+
+
+

+Implementation
+
+
+
public fun test_aggregator_valid_type() {
+    create_unbounded_aggregator<u64>();
+    create_unbounded_aggregator<u128>();
+    create_aggregator<u64>(5);
+    create_aggregator<u128>(5);
+}
+

+
+
+
 

 
 
diff --git a/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move b/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move
index 37f1d46a82161..df077b183be0e 100644
--- a/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move
+++ b/aptos-move/framework/aptos-framework/sources/aggregator_v2/aggregator_v2.move
@@ -6,9 +6,8 @@
 /// first transaction is doing `try_add(X, 1)` for aggregator `X`, and the second is
 /// doing `try_sub(X,3)`, they can be executed in parallel avoiding a read-modify-write
 /// dependency.
-/// However, reading the aggregator value (i.e. calling `read(X)`) is an expensive
-/// operation and should be avoided as much as possible because it reduces the
-/// parallelism.
+/// However, reading the aggregator value (i.e. calling `read(X)`) is a resource-intensive
+/// operation that also reduced parallelism, and should be avoided as much as possible.
 module aptos_framework::aggregator_v2 {
     use std::error;
     use std::string::String;
@@ -22,8 +21,8 @@ module aptos_framework::aggregator_v2 {
     /// The generic type supplied to the aggregator snapshot is not supported.
     const EUNSUPPORTED_AGGREGATOR_SNAPSHOT_TYPE: u64 = 5;
 
-    /// The aggregator snapshots feature flag is not enabled.
-    const EAGGREGATOR_SNAPSHOTS_NOT_ENABLED: u64 = 6;
+    /// The aggregator api v2 feature flag is not enabled.
+    const EAGGREGATOR_API_V2_NOT_ENABLED: u64 = 6;
 
     /// The generic type supplied to the aggregator is not supported.
     const EUNSUPPORTED_AGGREGATOR_TYPE: u64 = 7;
@@ -39,7 +38,7 @@ module aptos_framework::aggregator_v2 {
     /// across multiple transactions. See the module description for more details.
     ///
     /// Currently supported types for Element are u64 and u128.
-    struct Aggregator has store {
+    struct Aggregator has store, drop {
         value: IntElement,
         max_value: IntElement,
     }
@@ -90,11 +89,13 @@ module aptos_framework::aggregator_v2 {
     }
 
     /// Returns a value stored in this aggregator.
-    /// Note: This operation prevents parallelism of the transaction that calls it.
+    /// Note: This operation is resource-intensive, and reduces parallelism.
+    /// (Especially if called in a transaction that also modifies the aggregator,
+    /// or has other read/write conflicts)
     public native fun read(aggregator: &Aggregator): IntElement;
 
     /// Returns a wrapper of a current value of an aggregator
-    /// Unlike read(), this enables parallel execution of transactions.
+    /// Unlike read(), it is fast and avoids sequential dependencies.
     public native fun snapshot(aggregator: &Aggregator): AggregatorSnapshot;
 
     /// Creates a snapshot of a given value.
@@ -105,7 +106,9 @@ module aptos_framework::aggregator_v2 {
     public native fun copy_snapshot(snapshot: &AggregatorSnapshot): AggregatorSnapshot;
 
     /// Returns a value stored in this snapshot.
-    /// Note: This operation prevents parallelism of the transaction that calls it.
+    /// Note: This operation is resource-intensive, and reduces parallelism.
+    /// (Especially if called in a transaction that also modifies the aggregator,
+    /// or has other read/write conflicts)
     public native fun read_snapshot(snapshot: &AggregatorSnapshot): Element;
 
     /// Concatenates `before`, `snapshot` and `after` into a single string.
@@ -114,85 +117,98 @@ module aptos_framework::aggregator_v2 {
     /// If length of prefix and suffix together exceed 256 bytes, ECONCAT_STRING_LENGTH_TOO_LARGE is raised.
     public native fun string_concat(before: String, snapshot: &AggregatorSnapshot, after: String): AggregatorSnapshot;
 
-    #[test(_fx = @std)]
-    public fun test_correct_read(_fx: &signer) {
-        // use std::features;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
+    #[test]
+    public fun test_aggregator() {
+        let agg = create_aggregator(10);
+        assert!(try_add(&mut agg, 5), 1);
+        assert!(try_add(&mut agg, 5), 2);
+        assert!(read(&agg) == 10, 3);
+        assert!(!try_add(&mut agg, 5), 4);
+        assert!(read(&agg) == 10, 5);
+        assert!(try_sub(&mut agg, 5), 6);
+        assert!(read(&agg) == 5, 7);
+
+        let snap = snapshot(&agg);
+        assert!(try_add(&mut agg, 2), 8);
+        assert!(read(&agg) == 7, 9);
+        assert!(read_snapshot(&snap) == 5, 10);
+    }
 
+    #[test]
+    public fun test_correct_read() {
         let snapshot = create_snapshot(42);
-        // copy not yet supported
-        // let snapshot2 = copy_snapshot(&snapshot);
         assert!(read_snapshot(&snapshot) == 42, 0);
-        // assert!(read_snapshot(&snapshot2) == 42, 0);
-    }
-
-    #[test(_fx = @std)]
-    public fun test_correct_read_string(_fx: &signer) {
-        // use std::features;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
 
         let snapshot = create_snapshot(std::string::utf8(b"42"));
-        // copy not yet supported
-        // let snapshot2 = copy_snapshot(&snapshot);
         assert!(read_snapshot(&snapshot) == std::string::utf8(b"42"), 0);
-        // assert!(read_snapshot(&snapshot2) == std::string::utf8(b"42"), 0);
     }
 
-    #[test(_fx = @std)]
-    public fun test_string_concat1(_fx: &signer) {
-        // use std::features;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
+    #[test]
+    #[expected_failure(abort_code = 0x030009, location = Self)]
+    public fun test_copy_not_yet_supported() {
+        let snapshot = create_snapshot(42);
+        copy_snapshot(&snapshot);
+    }
 
+    #[test]
+    public fun test_string_concat1() {
         let snapshot = create_snapshot(42);
         let snapshot2 = string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
         assert!(read_snapshot(&snapshot2) == std::string::utf8(b"before42after"), 0);
     }
 
-    #[test(_fx = @std)]
+    #[test]
     #[expected_failure(abort_code = 0x030005, location = Self)]
-    public fun test_string_concat2(_fx: &signer) {
-        // use std::features;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
-
+    public fun test_string_concat_from_string_not_supported() {
         let snapshot = create_snapshot(std::string::utf8(b"42"));
-        let snapshot2 = string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
-        read_snapshot(&snapshot2);
-        // String concatenation not supported
-        // assert!(read_snapshot(&snapshot2) == std::string::utf8(b"before42after"), 0);
+        string_concat(std::string::utf8(b"before"), &snapshot, std::string::utf8(b"after"));
     }
 
     // #[test(fx = @std)]
     // #[expected_failure(abort_code = 0x030006, location = Self)]
     // public fun test_snapshot_feature_not_enabled(fx: &signer) {
     //     use std::features;
+    //     use aptos_framework::reconfiguration;
     //     let feature = features::get_aggregator_v2_api_feature();
     //     features::change_feature_flags(fx, vector[], vector[feature]);
-
+    //     reconfiguration::reconfigure_for_test();
     //     create_snapshot(42);
     // }
 
-    #[test(_fx = @std)]
+    // #[test(fx = @std)]
+    // #[expected_failure(abort_code = 0x030006, location = Self)]
+    // public fun test_aggregator_feature_not_enabled(fx: &signer) {
+    //     use std::features;
+    //     use aptos_framework::reconfiguration;
+    //     let feature = features::get_aggregator_v2_api_feature();
+    //     features::change_feature_flags(fx, vector[], vector[feature]);
+    //     reconfiguration::reconfigure_for_test();
+    //     create_aggregator(42);
+    // }
+
+    #[test]
+    #[expected_failure(abort_code = 0x030007, location = Self)]
+    public fun test_aggregator_invalid_type1() {
+        create_unbounded_aggregator();
+    }
+
+    public fun test_aggregator_valid_type() {
+        create_unbounded_aggregator();
+        create_unbounded_aggregator();
+        create_aggregator(5);
+        create_aggregator(5);
+    }
+
+    #[test]
     #[expected_failure(abort_code = 0x030005, location = Self)]
-    public fun test_snpashot_invalid_type1(_fx: &signer) {
-        // use std::features;
+    public fun test_snpashot_invalid_type1() {
         use std::option;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
-
         create_snapshot(option::some(42));
     }
 
-    #[test(_fx = @std)]
+    #[test]
     #[expected_failure(abort_code = 0x030005, location = Self)]
-    public fun test_snpashot_invalid_type2(_fx: &signer) {
-        // use std::features;
-        // let feature = features::get_aggregator_v2_api_feature();
-        // features::change_feature_flags(fx, vector[feature], vector[]);
-
+    public fun test_snpashot_invalid_type2() {
         create_snapshot(vector[42]);
     }
 }
diff --git a/aptos-move/framework/src/natives/aggregator_natives/aggregator_v2.rs b/aptos-move/framework/src/natives/aggregator_natives/aggregator_v2.rs
index 3b63ab2b00c27..5d1730026658b 100644
--- a/aptos-move/framework/src/natives/aggregator_natives/aggregator_v2.rs
+++ b/aptos-move/framework/src/natives/aggregator_natives/aggregator_v2.rs
@@ -244,6 +244,16 @@ impl BoundedMath {
 
 // ================= END TEMPORARY CODE =================
 
+macro_rules! abort_if_not_enabled {
+    ($context:expr) => {
+        if !$context.aggregator_v2_api_enabled() {
+            return Err(SafeNativeError::Abort {
+                abort_code: EAGGREGATOR_API_NOT_ENABLED,
+            });
+        }
+    };
+}
+
 /***************************************************************************************************
  * native fun create_aggregator(max_value: Element): Aggregator;
  **************************************************************************************************/
@@ -253,11 +263,7 @@ fn native_create_aggregator(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
-    if !context.aggregator_v2_api_enabled() {
-        return Err(SafeNativeError::Abort {
-            abort_code: EAGGREGATOR_API_NOT_ENABLED,
-        });
-    }
+    abort_if_not_enabled!(context);
 
     debug_assert_eq!(args.len(), 1);
 
@@ -281,11 +287,7 @@ fn native_create_unbounded_aggregator(
     ty_args: Vec,
     args: VecDeque,
 ) -> SafeNativeResult> {
-    if !context.aggregator_v2_api_enabled() {
-        return Err(SafeNativeError::Abort {
-            abort_code: EAGGREGATOR_API_NOT_ENABLED,
-        });
-    }
+    abort_if_not_enabled!(context);
 
     debug_assert_eq!(args.len(), 0);
 
@@ -318,6 +320,8 @@ fn native_try_add(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(args.len(), 2);
     context.charge(AGGREGATOR_V2_TRY_ADD_BASE)?;
 
@@ -347,6 +351,8 @@ fn native_try_sub(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(args.len(), 2);
     context.charge(AGGREGATOR_V2_TRY_SUB_BASE)?;
 
@@ -376,6 +382,8 @@ fn native_read(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(args.len(), 1);
     context.charge(AGGREGATOR_V2_READ_BASE)?;
 
@@ -401,6 +409,8 @@ fn native_snapshot(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(args.len(), 1);
     context.charge(AGGREGATOR_V2_SNAPSHOT_BASE)?;
 
@@ -423,11 +433,7 @@ fn native_create_snapshot(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
-    if !context.aggregator_v2_api_enabled() {
-        return Err(SafeNativeError::Abort {
-            abort_code: EAGGREGATOR_API_NOT_ENABLED,
-        });
-    }
+    abort_if_not_enabled!(context);
 
     debug_assert_eq!(ty_args.len(), 1);
     debug_assert_eq!(args.len(), 1);
@@ -448,10 +454,11 @@ fn native_create_snapshot(
  **************************************************************************************************/
 
 fn native_copy_snapshot(
-    _context: &mut SafeNativeContext,
+    context: &mut SafeNativeContext,
     _ty_args: Vec,
     _args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
     Err(SafeNativeError::Abort {
         abort_code: EAGGREGATOR_FUNCTION_NOT_YET_SUPPORTED,
     })
@@ -479,6 +486,8 @@ fn native_read_snapshot(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(ty_args.len(), 1);
     debug_assert_eq!(args.len(), 1);
     context.charge(AGGREGATOR_V2_READ_SNAPSHOT_BASE)?;
@@ -502,6 +511,8 @@ fn native_string_concat(
     ty_args: Vec,
     mut args: VecDeque,
 ) -> SafeNativeResult> {
+    abort_if_not_enabled!(context);
+
     debug_assert_eq!(ty_args.len(), 1);
     debug_assert_eq!(args.len(), 3);
     context.charge(AGGREGATOR_V2_STRING_CONCAT_BASE)?;