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

[move-decompiler] Model import from binary and basic decompiler setup #14964

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

wrwg
Copy link
Contributor

@wrwg wrwg commented Oct 15, 2024

Description

This PR closes the loop on the needed components for the decompiler and sets up basic plumbing for the command line tool.

  • A major addition is a way to import Move bytecode into the move-model. Until now, this wasn't directly possible. The new module binary_module_loader now takes care of this. The code is mostly complete, what is missing is a way to analyze metadata to synthesize attributes. (Future PRs.)
  • The crate move-decompiler has been added, which glues the various parts which have been created in the last few PRs into a command line tool.
  • Some tests have been added together with the decompiler, which is also a good way to test the new binary_module_loader, by replicating parts of the move-stdlib. However, the decompiler isn't functional yet, the code isn't correct in multiple instances. This has to be fixed in subsequent PRs, until the tests in move-decompiler allow to compile and executed decompiled code.

How Has This Been Tested?

Tests in move-decompiler

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
  • Move Compiler
  • Other (specify)

Copy link

trunk-io bot commented Oct 15, 2024

⏱️ 2h 28m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
rust-move-unit-coverage 17m 🟩
rust-move-unit-coverage 16m 🟩
rust-cargo-deny 16m 🟩🟩🟩🟩🟩 (+1 more)
rust-move-unit-coverage 16m 🟩
rust-move-unit-coverage 12m
rust-move-tests 10m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
rust-move-tests 9m 🟩
check-dynamic-deps 7m 🟩🟩🟩🟩🟩 (+1 more)
rust-move-tests 5m
rust-move-unit-coverage 5m
general-lints 3m 🟩🟩🟩🟩🟩 (+1 more)
semgrep/ci 2m 🟩🟩🟩🟩🟩 (+1 more)

settingsfeedbackdocs ⋅ learn more about trunk.io

Copy link
Contributor Author

wrwg commented Oct 15, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @wrwg and the rest of your teammates on Graphite Graphite

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 81.86047% with 156 lines in your changes missing coverage. Please review.

Project coverage is 60.2%. Comparing base (2a0e7d6) to head (dc2d715).

Files with missing lines Patch % Lines
third_party/move/tools/move-decompiler/src/lib.rs 36.1% 69 Missing ⚠️
...ove/move-model/src/builder/binary_module_loader.rs 87.8% 61 Missing ⚠️
third_party/move/move-model/src/lib.rs 0.0% 9 Missing ⚠️
third_party/move/tools/move-decompiler/src/main.rs 0.0% 8 Missing ⚠️
third_party/move/move-model/src/model.rs 95.8% 5 Missing ⚠️
third_party/move/move-model/src/metadata.rs 50.0% 3 Missing ⚠️
third_party/move/move-model/src/sourcifier.rs 97.0% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #14964       +/-   ##
===========================================
- Coverage    71.5%    60.2%    -11.4%     
===========================================
  Files        2409      859     -1550     
  Lines      490368   211785   -278583     
===========================================
- Hits       350832   127576   -223256     
+ Misses     139536    84209    -55327     

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

@wrwg wrwg force-pushed the wrwg/decompiler branch 4 times, most recently from 10c6304 to dc2d715 Compare October 15, 2024 07:38
Copy link
Contributor

@rahxephon89 rahxephon89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Copy link
Contributor

@brmataptos brmataptos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted 2 obvious issues. Will look more later.

@wrwg wrwg requested a review from brmataptos October 20, 2024 22:21
@wrwg wrwg mentioned this pull request Oct 21, 2024
22 tasks
wrwg added 2 commits October 28, 2024 19:59
This PR closes the loop on the needed components for the decompiler and sets up basic plumbing for the command line tool.

- A major addition is a way to import Move bytecode into the move-model. Until now, this wasn't directly possible. The new module `binary_module_loader` now takes care of this. The code is mostly complete, what is missing is a way to analyze metadata to synthesize attributes. (Future PRs.)
- The crate `move-decompiler` has been added, which glues the various parts which have been created in the last few PRs into a command line tool.
- Some tests have been added together with the decompiler, which is also a good way to test the new `binary_module_loader`, by replicating parts of the move-stdlib. However, the decompiler isn't functional yet, the code isn't correct in multiple instances. This has to be fixed in subsequent PRs, until the tests in `move-decompiler` allow to compile and executed decompiled code.
@wrwg wrwg enabled auto-merge (squash) October 29, 2024 03:21

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 0f825616b601f7935b33d78a13ecd2f05187a639

two traffics test: inner traffic : committed: 14342.73 txn/s, latency: 2771.07 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3300 ms), latency samples: 5453480
two traffics test : committed: 99.94 txn/s, latency: 1421.45 ms, (p50: 1400 ms, p70: 1400, p90: 1600 ms, p99: 2900 ms), latency samples: 1720
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.007, avg: 1.587", "ConsensusProposalToOrdered: max: 0.310, avg: 0.293", "ConsensusOrderedToCommit: max: 0.356, avg: 0.348", "ConsensusProposalToCommit: max: 0.647, avg: 0.640"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.99s no progress at version 41444 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 8.43s no progress at version 2058582 (avg 7.32s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639

Compatibility test results for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639 (PR)
Upgrade the nodes to version: 0f825616b601f7935b33d78a13ecd2f05187a639
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1263.71 txn/s, submitted: 1267.71 txn/s, failed submission: 4.00 txn/s, expired: 4.00 txn/s, latency: 2397.35 ms, (p50: 2100 ms, p70: 2400, p90: 3900 ms, p99: 5400 ms), latency samples: 113740
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1297.64 txn/s, submitted: 1299.64 txn/s, failed submission: 2.00 txn/s, expired: 2.00 txn/s, latency: 2278.45 ms, (p50: 2100 ms, p70: 2400, p90: 3400 ms, p99: 5400 ms), latency samples: 116880
5. check swarm health
Compatibility test for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639 passed
Upgrade the remaining nodes to version: 0f825616b601f7935b33d78a13ecd2f05187a639
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1449.27 txn/s, submitted: 1451.79 txn/s, failed submission: 2.52 txn/s, expired: 2.52 txn/s, latency: 2134.63 ms, (p50: 1800 ms, p70: 2100, p90: 3600 ms, p99: 4900 ms), latency samples: 126440
Test Ok

Copy link
Contributor

✅ Forge suite compat success on 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639

Compatibility test results for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639 (PR)
1. Check liveness of validators at old version: 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd
compatibility::simple-validator-upgrade::liveness-check : committed: 16905.14 txn/s, latency: 2009.31 ms, (p50: 2100 ms, p70: 2100, p90: 2200 ms, p99: 2400 ms), latency samples: 544020
2. Upgrading first Validator to new version: 0f825616b601f7935b33d78a13ecd2f05187a639
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 5549.84 txn/s, latency: 5099.01 ms, (p50: 5600 ms, p70: 5900, p90: 6600 ms, p99: 6800 ms), latency samples: 113100
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5868.58 txn/s, latency: 5524.53 ms, (p50: 5900 ms, p70: 6000, p90: 7300 ms, p99: 7500 ms), latency samples: 200380
3. Upgrading rest of first batch to new version: 0f825616b601f7935b33d78a13ecd2f05187a639
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 6906.08 txn/s, latency: 4116.29 ms, (p50: 4700 ms, p70: 4900, p90: 5100 ms, p99: 5100 ms), latency samples: 128860
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 6264.19 txn/s, latency: 4730.29 ms, (p50: 5100 ms, p70: 5200, p90: 5300 ms, p99: 5600 ms), latency samples: 232920
4. upgrading second batch to new version: 0f825616b601f7935b33d78a13ecd2f05187a639
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 8198.44 txn/s, latency: 3416.93 ms, (p50: 3700 ms, p70: 4200, p90: 4500 ms, p99: 4800 ms), latency samples: 145060
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 7914.46 txn/s, latency: 3989.47 ms, (p50: 4200 ms, p70: 4400, p90: 5700 ms, p99: 6000 ms), latency samples: 258500
5. check swarm health
Compatibility test for 9c922ebe94f5ff4b58df4617f3ff003e2ce10ccd ==> 0f825616b601f7935b33d78a13ecd2f05187a639 passed
Test Ok

@wrwg wrwg merged commit d50e048 into main Oct 29, 2024
86 of 92 checks passed
@wrwg wrwg deleted the wrwg/decompiler branch October 29, 2024 03:53
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.

4 participants