Large Package Publishing module with CLI command options (--chunked-publish) #13344

0xjunha · 2024-05-20T09:53:57Z

Description

large_packages.move module update & deployment

Updated large_packages.move module in move-examples for better data handling and supporting object code deployment & object code upgrade.
- Utilizing SmartTable in the StagingArea to store package code data
- Entry function stage_code_chunk supports 1) deploying to a standard account 2) deploying to an object 3) upgrading object code
Deployed immutable large_packages.move module both on mainnet and testnet, under account 0xa29df848eebfe5d981f708c2a5b06d31af2be53bbd8ddc94c8523f4b903f7adb

CLI

Added CLI command options to support large package publishing (chunked publish). the package will be divided into chunks and uploaded on-chain through several transactions.
added command:
- aptos move clear-staging-area: to cleanup StagingArea resource of an account.
Updated commands to include the --chunked-publish flag:
- aptos move publish [OPTIONS] --chunked-publish
- aptos move create-object-and-publish-package [OPTIONS] --address-name <ADDRESS_NAME> --chunked-publish
- aptos move upgrade-object-package [OPTIONS] --address-name <ADDRESS_NAME> --chunked-publish

e2e move tests

Added e2e-move tests for publishing large packages using large_packages.move with following test cases:
- publishing large package
- upgrading large package
- upgrading large package (fail; incompatible modules)
- upgrading large package (fail; immutable package)
- upgrading large package (fail; overlapping modules)
- publishing large package to object
- upgrading large package deployed under an object

SDK Updates required:

Ensure SDKs support serializing the Option Move type, as the entry function large_packages::stage_code_chunk receives the Option type.
Since the object address is derived from the user account and sequence number, we need a way to calculate the publisher's sequence number when the package is finally deployed, after multiple transactions of staging code. This will allow us to specify the object address as a named address. For example, if a large package requires 4 staging transactions and 1 final publishing transaction, (current sequence number) + 1 + 4 will be the sequence number when the package is finally deployed. In aptos CLI, this is done by running a preliminary package build to determine number of chunks of the package.

Type of Change

How Has This Been Tested?

cargo nextest run large_package_publishing

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

trunk-io · 2024-05-20T09:54:01Z

⏱️ 4h 45m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-move-unit-coverage	1h 20m	🟩 🟩 🟩 🟩 🟩
rust-move-tests	1h 9m	🟥 🟥 🟩 🟩 🟩
rust-targeted-unit-tests	59m	🟩 🟩 🟩 🟩 🟩
rust-lints	35m	🟥 🟩 🟩 🟥 🟩
run-tests-main-branch	21m	🟩 🟩 🟩 🟩 🟩
general-lints	9m	🟩 🟩 🟩 🟩 🟩
check-dynamic-deps	8m	🟩 🟩 🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩 🟩
file_change_determinator	58s	🟩 🟩 🟩 🟩 🟩
file_change_determinator	56s	🟩 🟩 🟩 🟩 🟩
permission-check	18s	🟩 🟩 🟩 🟩 🟩
permission-check	15s	🟩 🟩 🟩 🟩 🟩
permission-check	15s	🟩 🟩 🟩 🟩 🟩
permission-check	13s	🟩 🟩 🟩 🟩 🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
rust-move-unit-coverage	13m	16m
rust-targeted-unit-tests	10m	18m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

codecov · 2024-05-28T17:03:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.1%. Comparing base (a23e74a) to head (a3473b4).

❗ Current head a3473b4 differs from pull request most recent head 1759c80

Please upload reports for the commit 1759c80 to get more accurate results.

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #13344       +/-   ##
===========================================
- Coverage    71.4%    33.1%    -38.4%     
===========================================
  Files        2319     1753      -566     
  Lines      456030   337769   -118261     
===========================================
- Hits       325836   111808   -214028     
- Misses     130194   225961    +95767

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

aptos-move/move-examples/large_packages/sources/large_packages.move

…ent and use SmartTable to store code in StagingArea / added relevant test cases

…oy 3) object upgrade from the CLI (under `aptos move` commands)

updated immutable `large_packages.move` module deployed addresses

…and publishing removed publish only functions

davidiw

I think we're seeing an ambiguity of where to put code that's not quite in the framework.. my suggestion would be either create move-framework-ext or just merge it into the framework helpers.

beyond that it seems like the chunked api supports both the traditional and chunked publishing... my recommendation is to not let the users choose which but instead let it be decided by the size of the package being published.

we could also leverage two enums:

chunked or not
account, object, etc

whereas you have two enums that are mirrors of each other

looks really good though

I'm a little concerned this might take more time to land, so it might be worthwhile getting the Python variant fixed first, update docs, and then resume here? But let's get one other source of feedback, maybe @JohnChangUK? And your own thoughts...

davidiw · 2024-06-11T08:20:31Z

aptos-move/e2e-move-tests/Cargo.toml

@@ -14,6 +14,7 @@ rust-version = { workspace = true }

 [dependencies]
 anyhow = { workspace = true }
+aptos = { workspace = true }


This is probably an anti-pattern, I don't think the test should go here.

davidiw · 2024-06-11T08:21:22Z

...e2e-move-tests/src/tests/large_package_publishing.data/large_pack_upgrade_incompat/Move.toml

@@ -0,0 +1,9 @@
+[package]


we didn't we just take any arbitrary package and try to update rather than making another one?

davidiw · 2024-06-11T08:22:39Z

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

+// SPDX-License-Identifier: Apache-2.0
+
+use crate::{assert_move_abort, assert_success, assert_vm_status, tests::common, MoveHarness};
+use aptos::move_tool::chunked_publish::create_chunks;


let's move this to aptos_framework crate so we don't further bloat the dependencies for building e2e-move-tests

davidiw · 2024-06-11T08:23:07Z

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

+    ) -> Vec<SignedTransaction> {
+        let package = BuiltPackage::build(path.to_owned(), options.unwrap())
+            .expect("package build must succeed");
+        // let package_arc = {


don't forget to cleanup

davidiw · 2024-06-11T08:27:12Z

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

+        )
+    }
+
+    /// Create payloads from metadata and code chunks for a large package.


a lot of this code feels like it should live somewhere where both the CLI and this testing harness can access it. I don't know if the framework actually knows about Aptos types, if it does, then let's push it there...

0xjunha requested a review from davidiw May 20, 2024 09:53

0xjunha force-pushed the large_packages_e2e branch 2 times, most recently from ba4f40c to 00b8d8e Compare May 28, 2024 17:24

0xjunha changed the title ~~Add large_package_publishing e2e-move tests~~ Large Package Publishing module with e2e-move tests May 28, 2024

0xjunha force-pushed the large_packages_e2e branch from 00b8d8e to 0401e12 Compare May 28, 2024 17:51

0xjunha requested review from JohnChangUK and movekevin May 28, 2024 17:52

JohnChangUK reviewed May 28, 2024

View reviewed changes

aptos-move/move-examples/large_packages/sources/large_packages.move Outdated Show resolved Hide resolved

0xjunha added 5 commits June 10, 2024 16:22

Large Package Publishing e2e move test cases

58878aa

updated large_packages.move contract to support object code deploym…

d65717f

…ent and use SmartTable to store code in StagingArea / added relevant test cases

hotfix code_indices push logic

c99f51d

added --chunked-publish option for 1) account deploy 2) object depl…

80ef8dc

…oy 3) object upgrade from the CLI (under `aptos move` commands)

added aptos move clear-staging-area command

ed370fa

0xjunha force-pushed the large_packages_e2e branch from 0401e12 to 58c6cdc Compare June 10, 2024 07:30

0xjunha requested review from gregnazario, banool and 0xmaayan as code owners June 10, 2024 07:30

0xjunha changed the title ~~Large Package Publishing module with e2e-move tests~~ Large Package Publishing module with CLI command options (--chunked-publish) Jun 10, 2024

0xjunha force-pushed the large_packages_e2e branch from 58c6cdc to 9115ce2 Compare June 10, 2024 07:51

updated docs and README file of large_packages.move

d63226b

updated immutable `large_packages.move` module deployed addresses

0xjunha force-pushed the large_packages_e2e branch from 9115ce2 to d63226b Compare June 10, 2024 08:50

0xjunha added 2 commits June 10, 2024 22:40

large_packages.move to have separate functions for the final staging …

bc841b3

…and publishing removed publish only functions

updated e2e move tests to match updated large_packages.move module

1759c80

davidiw reviewed Jun 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Large Package Publishing module with CLI command options (--chunked-publish) #13344

Large Package Publishing module with CLI command options (--chunked-publish) #13344

0xjunha commented May 20, 2024 •

edited

trunk-io bot commented May 20, 2024 •

edited

codecov bot commented May 28, 2024 •

edited

davidiw left a comment

davidiw Jun 11, 2024

davidiw Jun 11, 2024

davidiw Jun 11, 2024

davidiw Jun 11, 2024

davidiw Jun 11, 2024

Large Package Publishing module with CLI command options (--chunked-publish) #13344

Are you sure you want to change the base?

Large Package Publishing module with CLI command options (--chunked-publish) #13344

Conversation

0xjunha commented May 20, 2024 • edited

Description

large_packages.move module update & deployment

CLI

e2e move tests

SDK Updates required:

Type of Change

How Has This Been Tested?

Checklist

trunk-io bot commented May 20, 2024 • edited

codecov bot commented May 28, 2024 • edited

Codecov Report

davidiw left a comment

Choose a reason for hiding this comment

davidiw Jun 11, 2024

Choose a reason for hiding this comment

davidiw Jun 11, 2024

Choose a reason for hiding this comment

davidiw Jun 11, 2024

Choose a reason for hiding this comment

davidiw Jun 11, 2024

Choose a reason for hiding this comment

davidiw Jun 11, 2024

Choose a reason for hiding this comment

0xjunha commented May 20, 2024 •

edited

trunk-io bot commented May 20, 2024 •

edited

codecov bot commented May 28, 2024 •

edited