[storage fee] adjustments #6816

msmouse · 2023-02-28T04:30:24Z

Description

[storage fee] take into account free txn and event bytes quota
[gas] 100x the GAS_SCALING_FACTOR, lowering execution gas charge by 100x

Test Plan

existing test suite

movekevin · 2023-02-28T12:16:31Z

aptos-move/aptos-gas/src/transaction/mod.rs

+        [
+            free_event_bytes_quota: NumBytes,
+            { 7.. => "free_event_bytes_quota" },
+            1024, // 1KB free event bytes per transaction


Are you planning to benchmark 2KB after v1.4 cut? This is still fine actually as it'll be another week or two before we upgrade the gas schedule in testnet

The potential 1KB -> 2KB change is for the state write quota (free_write_bytes_quota), orthogonal to this, to make the smart data structures more useful. In this case we don't think it's valid to allow more than 1KB free events to be emitted.

movekevin · 2023-02-28T12:16:57Z

aptos-move/aptos-gas/src/transaction/mod.rs

+        let total_bytes = events.into_iter().fold(NumBytes::zero(), |total, event| {
+            total + NumBytes::new(event.size() as u64)
+        });
+        total_bytes


Shouldn't this change be gated for replayability?

This is for the storage fee only, starting from gas feature version 7, never enabled before. In the introduction of the storage fee, we are lowering the cost for emitting events a bit, including this new free quota.

Is calculate_event_storage_fee already used? If so, wouldn't .checked_sub(self.free_event_bytes_quota) lead to new behavior?

No all the "storage_fee" related functions and parameters were new with @vgao1996 's PR.

Got it. We'll run replay anyway I guess so we'll know if any issues happen due to missed gating.

movekevin · 2023-02-28T12:17:35Z

aptos-move/aptos-gas/src/transaction/mod.rs

    }

    /// New formula to charge storage fee for transaction based on fixed APT costs.
    pub fn calculate_transaction_storage_fee(&self, txn_size: NumBytes) -> Fee {
-        self.storage_fee_per_transaction_byte * txn_size
+        txn_size
+            .checked_sub(self.large_transaction_cutoff)


Why are we enforcing large_transaction_cutoff now?

To make smaller txns storage fee -free (virtually entire txn is close to free when the gas price is low). Again this is for the new "storage fee", although in this case large_transaction_cutoff has been respected for calculating the intrinsic gas

aptos-core/aptos-move/aptos-gas/src/transaction/mod.rs

Line 251 in bcdb4f2

if transaction_size > self.large_transaction_cutoff {

movekevin · 2023-02-28T12:17:50Z

aptos-move/e2e-move-tests/src/tests/per_category_gas_limits.rs

-        gas_params.txn.max_io_gas = Gas::new(10) * gas_params.txn.gas_unit_scaling_factor
-    });
+    // Lower the max io gas to lower than a single load_resource
+    h.modify_gas_schedule(|gas_params| gas_params.txn.max_io_gas = InternalGas::new(300_000 - 1));


What is 300_000 - 1?

300_000 is the gas charge for a single load_resource (see the comment above)

msmouse · 2023-02-28T17:33:43Z

aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/call_function.exp

@@ -1,7 +1,7 @@
 processed 4 tasks

 task 1 'publish'. lines 4-30:
-Error: Transaction discarded. VM status code: MAX_GAS_UNITS_BELOW_MIN_TRANSACTION_GAS_UNITS


These are so broken but I'd fix them separately.

vgao1996

LGTM

github-actions · 2023-02-28T20:13:10Z

✅ Forge suite `framework_upgrade` success on `cb4ba0a57c998c60cbab65af31a64875d2588ca5` ==> `c2a6fc2391ef06ad0f656749749b59fa97c5b267`

Compatibility test results for cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267 (PR)
Upgrade the nodes to version: c2a6fc2391ef06ad0f656749749b59fa97c5b267
framework_upgrade::framework-upgrade::full-framework-upgrade : 7114 TPS, 5341 ms latency, 8700 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267 passed
Test Ok

github-actions · 2023-02-28T20:16:17Z

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `c2a6fc2391ef06ad0f656749749b59fa97c5b267`

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 8202 TPS, 4685 ms latency, 6800 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: c2a6fc2391ef06ad0f656749749b59fa97c5b267
compatibility::simple-validator-upgrade::single-validator-upgrade : 4822 TPS, 8100 ms latency, 12300 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: c2a6fc2391ef06ad0f656749749b59fa97c5b267
compatibility::simple-validator-upgrade::half-validator-upgrade : 4883 TPS, 8242 ms latency, 11200 ms p99 latency,no expired txns
4. upgrading second batch to new version: c2a6fc2391ef06ad0f656749749b59fa97c5b267
compatibility::simple-validator-upgrade::rest-validator-upgrade : 7152 TPS, 5367 ms latency, 10400 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267 passed
Test Ok

github-actions · 2023-02-28T20:16:30Z

✅ Forge suite `land_blocking` success on `c2a6fc2391ef06ad0f656749749b59fa97c5b267`

performance benchmark with full nodes : 6146 TPS, 6462 ms latency, 8700 ms p99 latency,(!) expired 440 out of 2625140 txns
Test Ok

hydrogenbond007 · 2023-03-01T08:10:30Z

hey great work, was curious is there a small md file or writeup on how this works?

msmouse · 2023-03-02T04:13:47Z

@hydrogenbond007 aptos-foundation/AIPs#79

msmouse requested review from vgao1996, banool, gregnazario and 0xmaayan as code owners February 28, 2023 04:30

msmouse requested review from movekevin, davidiw, a team and lightmark February 28, 2023 04:30

msmouse changed the title ~~[storage fee] take into account free txn and event bytes quota~~ [storage fee] adjustments Feb 28, 2023

msmouse force-pushed the 0227alden_storage_fee branch 3 times, most recently from 48baac9 to bcdb4f2 Compare February 28, 2023 05:18

movekevin reviewed Feb 28, 2023

View reviewed changes

msmouse force-pushed the 0227alden_storage_fee branch 3 times, most recently from b13ff0a to 0c74353 Compare February 28, 2023 17:33

msmouse commented Feb 28, 2023

View reviewed changes

movekevin approved these changes Feb 28, 2023

View reviewed changes

msmouse force-pushed the 0227alden_storage_fee branch from 0c74353 to 9367761 Compare February 28, 2023 17:49

vgao1996 approved these changes Feb 28, 2023

View reviewed changes

msmouse enabled auto-merge (rebase) February 28, 2023 18:36

This comment has been minimized.

Sign in to view

[storage fee] take into account free txn and event bytes quota

4479c19

grao1991 approved these changes Feb 28, 2023

View reviewed changes

[gas] 100x the GAS_SCALING_FACTOR, lowering execution gas charge by 100x

c2a6fc2

msmouse force-pushed the 0227alden_storage_fee branch from 9367761 to c2a6fc2 Compare February 28, 2023 19:34

This comment has been minimized.

Sign in to view

msmouse merged commit 7e5ab62 into main Feb 28, 2023

msmouse deleted the 0227alden_storage_fee branch February 28, 2023 20:16

lightmark mentioned this pull request Feb 28, 2023

Move smart vector/table to framework #6823

Merged

msmouse mentioned this pull request Mar 1, 2023

[AIP-17][Discussion] Reducing Execution Costs by Decoupling Transaction Storage and Execution Charges aptos-foundation/AIPs#79

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[storage fee] adjustments #6816

[storage fee] adjustments #6816

msmouse commented Feb 28, 2023

movekevin Feb 28, 2023

msmouse Feb 28, 2023 •

edited

Loading

movekevin Feb 28, 2023

msmouse Feb 28, 2023 •

edited

Loading

movekevin Feb 28, 2023

msmouse Feb 28, 2023

movekevin Feb 28, 2023

movekevin Feb 28, 2023

msmouse Feb 28, 2023

movekevin Feb 28, 2023

msmouse Feb 28, 2023

msmouse Feb 28, 2023

vgao1996 left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

github-actions bot commented Feb 28, 2023

hydrogenbond007 commented Mar 1, 2023

msmouse commented Mar 2, 2023

[storage fee] adjustments #6816

[storage fee] adjustments #6816

Conversation

msmouse commented Feb 28, 2023

Description

Test Plan

Choose a reason for hiding this comment

msmouse Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msmouse Feb 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vgao1996 left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Feb 28, 2023

✅ Forge suite framework_upgrade success on cb4ba0a57c998c60cbab65af31a64875d2588ca5 ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267

github-actions bot commented Feb 28, 2023

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> c2a6fc2391ef06ad0f656749749b59fa97c5b267

github-actions bot commented Feb 28, 2023

✅ Forge suite land_blocking success on c2a6fc2391ef06ad0f656749749b59fa97c5b267

hydrogenbond007 commented Mar 1, 2023

msmouse commented Mar 2, 2023

msmouse Feb 28, 2023 •

edited

Loading

msmouse Feb 28, 2023 •

edited

Loading

✅ Forge suite `framework_upgrade` success on `cb4ba0a57c998c60cbab65af31a64875d2588ca5` ==> `c2a6fc2391ef06ad0f656749749b59fa97c5b267`

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `c2a6fc2391ef06ad0f656749749b59fa97c5b267`

✅ Forge suite `land_blocking` success on `c2a6fc2391ef06ad0f656749749b59fa97c5b267`