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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
89d9165
sweep
danielxiangzl Oct 8, 2024
da08e70
10k
danielxiangzl Oct 11, 2024
473e4ee
enable opt sig
danielxiangzl Oct 11, 2024
4cc7352
Merge branch 'main' into daniel-main
danielxiangzl Oct 11, 2024
e676e14
enable co
danielxiangzl Oct 11, 2024
2d1a173
test
danielxiangzl Oct 11, 2024
ccaf199
omit signed batchinfo verify
danielxiangzl Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/src/config/consensus_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ impl Default for ConsensusConfig {
num_bounded_executor_tasks: 16,
enable_pre_commit: true,
max_pending_rounds_in_commit_vote_cache: 100,
optimistic_sig_verification: false,
optimistic_sig_verification: true,
enable_round_timeout_msg: false,
}
}
Expand Down
6 changes: 3 additions & 3 deletions config/src/config/consensus_observer_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use serde::{Deserialize, Serialize};
use serde_yaml::Value;

// Useful constants for enabling consensus observer on different node types
const ENABLE_ON_VALIDATORS: bool = false;
const ENABLE_ON_VALIDATOR_FULLNODES: bool = false;
const ENABLE_ON_PUBLIC_FULLNODES: bool = false;
const ENABLE_ON_VALIDATORS: bool = true;
const ENABLE_ON_VALIDATOR_FULLNODES: bool = true;
const ENABLE_ON_PUBLIC_FULLNODES: bool = true;

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
#[serde(default, deny_unknown_fields)]
Expand Down
5 changes: 4 additions & 1 deletion consensus/consensus-types/src/proof_of_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,10 @@ impl SignedBatchInfo {
);
}

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

// Ok(validator.verify(self.signer, &self.info, &self.signature)?)
Comment on lines +263 to +266
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.

}

pub fn signature(&self) -> &bls12381::Signature {
Expand Down
14 changes: 5 additions & 9 deletions testsuite/forge-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,8 @@ fn get_land_blocking_test(
) -> Option<ForgeConfig> {
let test = match test_name {
"land_blocking" | "realistic_env_max_load" => {
realistic_env_max_load_test(duration, test_cmd, 7, 5)
// realistic_env_max_load_test(duration, test_cmd, 7, 5)
realistic_env_load_sweep_test()
},
"compat" => compat(),
"framework_upgrade" => framework_upgrade(),
Expand Down Expand Up @@ -1174,16 +1175,11 @@ fn background_traffic_for_sweep_with_latency(criteria: &[(f32, f32)]) -> Option<
}

fn realistic_env_load_sweep_test() -> ForgeConfig {
realistic_env_sweep_wrap(20, 10, LoadVsPerfBenchmark {
realistic_env_sweep_wrap(100, 10, LoadVsPerfBenchmark {
test: Box::new(PerformanceBenchmark),
workloads: Workloads::TPS(vec![10, 100, 1000, 3000, 5000, 7000]),
workloads: Workloads::TPS(vec![10000]),
criteria: [
(9, 0.9, 0.9, 1.2, 0),
(95, 0.9, 1.0, 1.2, 0),
(950, 1.2, 1.3, 2.0, 0),
(2900, 1.4, 2.2, 2.5, 0),
(4800, 2.0, 2.5, 3.0, 0),
(6700, 2.5, 3.5, 5.0, 0),
(9000, 5.0, 7.0, 9.0, 0),
// TODO add 9k or 10k. Allow some expired transactions (high-load)
]
.into_iter()
Expand Down
Loading