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

[test] main #14937

Closed
wants to merge 7 commits into from
Closed

[test] main #14937

wants to merge 7 commits into from

Conversation

danielxiangzl
Copy link
Contributor

Description

How Has This Been Tested?

Key Areas to Review

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)

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

@danielxiangzl danielxiangzl added the CICD:run-forge-e2e-perf Run the e2e perf forge only label Oct 11, 2024
Copy link

trunk-io bot commented Oct 11, 2024

⏱️ 2h 6m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
adhoc-forge-test / forge 33m 🟩
forge-e2e-test / forge 24m 🟩
dispatch_event 16m 🟩
dispatch_event 15m 🟩
rust-move-tests 10m 🟩
rust-move-tests 10m 🟩
test-target-determinator 8m 🟩🟩
rust-cargo-deny 3m 🟩🟩
check-dynamic-deps 2m 🟩🟩🟩
semgrep/ci 1m 🟩🟩🟩
general-lints 56s 🟩🟩
file_change_determinator 23s 🟩🟩
file_change_determinator 21s 🟩🟩
permission-check 11s 🟩🟩
permission-check 9s 🟩🟩🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

This comment has been minimized.

This comment has been minimized.

Comment on lines +263 to +266
// daniel hack
Ok(())

// Ok(validator.verify(self.signer, &self.info, &self.signature)?)
Copy link

Choose a reason for hiding this comment

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

The signature verification has been disabled, which introduces a significant security vulnerability. This change allows any signature to be considered valid, bypassing a critical security check. To maintain the integrity of the system, please restore the original verification logic by removing the Ok(()) line and uncommenting the original verification:

Ok(validator.verify(self.signer, &self.info, &self.signature)?)

If this change was made for testing purposes, consider using a feature flag or a separate test configuration to enable this behavior in controlled environments only.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

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 ccaf19944b25b33cc07d4c7007ac9ae5dc847229

Load (TPS)                     | submitted/s  | committed/s  | expired/s    | rejected/s   | chain txn/s  | latency      | p50 lat      | p90 lat      | p99 lat      | batch->pos   | pos->prop    | prop->order  | order->commit | actual dur   | idx_fn       | idx_cache    | idx_data    
10000                          | 10000.84     | 10000.84     | 0.00         | 0.00         | 10013        | 1.664        | 1.600        | 2.000        | 2.300        | 0.298        | 0.260        | 0.318        | 0.454        | 360          | 0.000        | 0.000        | 0.000       
background traffic             | submitted/s  | committed/s  | expired/s    | rejected/s   | latency      | p50 lat      | p90 lat      | p99 lat     
background with traffic 0      | 10.00        | 10.00        | 0.00         | 0.00         | 1.503        | 1.500        | 1.800        | 1.800       
Test Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD:run-forge-e2e-perf Run the e2e perf forge only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant