Skip to content

add code download mode for comparison testing tool#20187

Draft
rahxephon89 wants to merge 1 commit into
mainfrom
teng/comparison-testing-code-download-mode
Draft

add code download mode for comparison testing tool#20187
rahxephon89 wants to merge 1 commit into
mainfrom
teng/comparison-testing-code-download-mode

Conversation

@rahxephon89

@rahxephon89 rahxephon89 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a source-code-only download mode to the comparison-testing tool and speeds up its txn scanning.

  • dump --skip-txn-execution: collects the verified source of every package called by an entry-function txn (plus transitive deps) without executing txns — no state data, write sets, or txn index are dumped, and dumped sources are not compile-validated (a dumped_packages set dedupes re-dumps instead of the compiled-package map). This avoids execution-only failure modes on historical version ranges (e.g. framework bytecode-version mismatches) and the per-txn remote read-set recording.
  • Faster scanning: get_and_filter_committed_transactions no longer fetches persisted auxiliary infos it immediately discarded, and fetches API pages concurrently via get_committed_transactions_no_aux_concurrent. The txn-fetch loop is extracted verbatim into get_committed_transactions_no_aux, so the get_committed_transactions trait method behaves exactly as before. Net: ~11× faster (2.6s → 0.23s per 500-txn batch; a 1M-version window drops from ~90 min to ~8 min).

How Has This Been Tested?

  • Live mainnet dumps with --skip-txn-execution (genesis-era and recent ranges): package sources and rewritten Move.toml dumped correctly, no state/write-set/index files, no compilation, dumped packages verified compilable with aptos move test against the framework checkout.
  • 5000-version scan benchmark confirming the speedup; default (execution) mode and online/execute paths unchanged and still build/run.

Type of Change

  • New feature
  • Performance improvement

Which Components or Systems Does This Change Impact?

  • Developer Infrastructure

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I tested both happy and unhappy path of the functionality

Copy link
Copy Markdown
Contributor Author

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

@rahxephon89 rahxephon89 force-pushed the teng/comparison-testing-code-download-mode branch from f44713b to 888104d Compare July 3, 2026 16:35

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aptos Security Bugbot has reviewed your changes and found 1 potential issue.

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

Comment on lines +343 to 357
// dump through data_manager (skipped in source-only mode)
if let (Some(state_view), Some(outputs)) =
(state_view_opt, epoch_result_opt)
{
Self::dump_txn_index(
&mut data_manager.lock().unwrap(),
version_idx,
&state_view.get_state_keys().lock().unwrap(),
Ok(outputs),
dump_write_set,
);
}

// Log version
index.lock().unwrap().add_version(version);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium severity: --skip-txn-execution skips dump_txn_index() and state snapshots here, but the code still records every version in version_index.txt. The replay path in Execution::execute_txns() only checks for the directories and index file, and execute_one_txn() silently returns Ok(()) when get_txn_index() is missing, so a source-only dump can later be consumed as if it were a valid replay corpus while executing zero transactions. That can produce false-clean comparison runs and mask compiler or VM regressions.

@rahxephon89 rahxephon89 force-pushed the teng/comparison-testing-code-download-mode branch from 888104d to 93dff04 Compare July 3, 2026 17:02

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aptos Security Bugbot has reviewed your changes, there are still 1 issues that need to be addressed from previous scan.

Open findings:

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

@rahxephon89 rahxephon89 force-pushed the teng/comparison-testing-code-download-mode branch from 93dff04 to 59d21d6 Compare July 3, 2026 22:52

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aptos Security Bugbot has reviewed your changes and found 1 potential issue.

Open in Web View Automation 

Sent by Cursor Automation: Security Review Bot

Comment on lines +653 to +661
if skip_source_compilation {
let dep_hash = package_content_hash(dep_metadata);
if let Some(canonical) =
compilation_cache.content_hash_to_package.get(&dep_hash)
{
if *canonical != package_info
&& root_dir.join(format!("{}", canonical)).exists()
{
fix_manifest_dep(dep, &format!("{}", canonical));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium severity: In --skip-txn-execution mode this redirection only checks package_content_hash(), which ignores the package manifest and package name. If two packages publish the same module text under different package identities or named-address bindings, dependents here are rewritten to the canonical directory even though Move resolves local dependencies against that dependency's own Move.toml. The resulting dump can therefore compile against the wrong manifest semantics, or stop compiling entirely when the dependency key no longer matches the canonical package name, which hides real source-level comparison failures behind a seemingly successful collection.

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