Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[block-stm] Integrate script caches #14472

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

georgemitenkov
Copy link
Contributor

@georgemitenkov georgemitenkov commented Aug 30, 2024

Description

This completes Block-STM integration for the code cache by adding an implementation to deserialize/verify and cache scripts. Right now, we opt for a very basic policy of:

any module published ==> flush the script cache

We can revisit this in later PRs, but for now it suffices to get things running. Correctness is guaranteed because we publish at commit time, and so if there is a txn with a script that used older versions of code, it will be invalidated since there will be at least a single module dependency that gets loaded and is invalidated.

Other:

  • This PR also fixed a bug in sponsored account creation introduced by one of the earlier PRs where is_none() got replaced by is_some().

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

How Has This Been Tested?

For tests, see the PR stacked on top.

Key Areas to Review

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Copy link

trunk-io bot commented Aug 30, 2024

⏱️ 4h total CI duration on this PR
Job Cumulative Duration Recent Runs
general-lints 24m 🟩🟩🟩🟩🟩 (+8 more)
rust-cargo-deny 23m 🟩🟩🟩🟩🟩 (+8 more)
rust-move-unit-coverage 14m 🟩
rust-move-unit-coverage 13m 🟩
check-dynamic-deps 13m 🟩🟩🟩🟩🟩 (+8 more)
rust-move-unit-coverage 13m 🟩
rust-move-unit-coverage 12m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟥
rust-move-tests 9m 🟩
rust-move-tests 8m 🟩
rust-move-unit-coverage 8m 🟩
rust-move-tests 8m 🟥
rust-move-tests 8m 🟩
semgrep/ci 5m 🟩🟩🟩🟩🟩 (+8 more)
rust-move-unit-coverage 5m 🟥
rust-move-unit-coverage 5m 🟥
rust-move-unit-coverage 4m 🟥
rust-move-unit-coverage 4m 🟥
rust-move-unit-coverage 4m
rust-move-unit-coverage 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 3m 🟥
rust-move-tests 2m 🟥
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+8 more)
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+8 more)
rust-move-unit-coverage 2m
permission-check 39s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 38s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 37s 🟩🟩🟩🟩🟩 (+8 more)
permission-check 31s 🟩🟩🟩🟩🟩 (+8 more)

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link

codecov bot commented Aug 30, 2024

Codecov Report

Attention: Patch coverage is 12.03704% with 95 lines in your changes missing coverage. Please review.

Please upload report for BASE (george/main@e730a3c). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...tos-move/block-executor/src/modules_and_scripts.rs 0.0% 69 Missing ⚠️
aptos-move/block-executor/src/executor.rs 0.0% 20 Missing ⚠️
...ty/move/move-vm/runtime/src/storage/environment.rs 0.0% 5 Missing ⚠️
aptos-move/aptos-vm/src/aptos_vm.rs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##             george/main   #14472   +/-   ##
==============================================
  Coverage               ?    59.4%           
==============================================
  Files                  ?      857           
  Lines                  ?   209210           
  Branches               ?        0           
==============================================
  Hits                   ?   124434           
  Misses                 ?    84776           
  Partials               ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@georgemitenkov georgemitenkov force-pushed the george/script-mv-cache branch 5 times, most recently from 03515a0 to a5e7ca6 Compare September 4, 2024 18:26
Base automatically changed from george/module-mv-storage to george/main September 6, 2024 09:43
@georgemitenkov georgemitenkov merged commit e9f53e6 into george/main Sep 6, 2024
40 checks passed
@georgemitenkov georgemitenkov deleted the george/script-mv-cache branch September 6, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant