-
Notifications
You must be signed in to change notification settings - Fork 218
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
fix!: change grpc deny to allow #6218
fix!: change grpc deny to allow #6218
Conversation
Test Results (CI) 3 files 120 suites 49m 46s ⏱️ Results for commit e16e374. ♻️ This comment has been updated with latest results. |
Test Results (Integration tests) 2 files 11 suites 32m 49s ⏱️ For more details on these failures, see this check. Results for commit e16e374. ♻️ This comment has been updated with latest results. |
Co-authored-by: Stan Bondi <sdbondi@users.noreply.github.com>
a9fda1f
to
8b85be1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments below, thnaks.
common/config/presets/c_base_node_b_non_mining_allow_methods.toml
Outdated
Show resolved
Hide resolved
@@ -119,18 +119,18 @@ pub fn prompt_default_config() -> [&'static str; 12] { | |||
/// Returns the default configuration file template in parts from the embedded presets. If use_mining_config is true, | |||
/// the base node configuration that enables mining is returned, otherwise the non-mining configuration is returned. | |||
pub fn get_default_config(use_mining_config: bool) -> [&'static str; 12] { | |||
let base_node_deny_methods = if use_mining_config { | |||
include_str!("../../config/presets/c_base_node_b_mining_deny_methods.toml") | |||
let base_node_allow_methods = if use_mining_config { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about these methods second_layer_grpc_enabled
? I assume it should be set from initial startup.
A node could also double as mining node and second layer comms node.
Co-authored-by: Hansie Odendaal <39146854+hansieodendaal@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Description
Removes the antipattern of a deny list and puts in an allow list
Motivation and Context
A deny list is an antipattern because new methods are not automatically added to the list
Replaces: #6177