[Traffic Control] Fully remove client IP param injection (#17803)

## Description 

This removes the plumbing to get client IP through json rpc requests by
injecting into the request param.

Things NOT touched:

1. Did not remove plumbing of the client IP from transaction
orchestrator forward. This allows for us to later route client IP
through other interfaces such as graphql, which has far bettewr support
for such things. This is fine as these fields are all optional and
should not change anything
2. We still accept and segment proxied requests in traffic control
policies, again, so that other interfaces may forward and everything
will work as before.

## Test plan 

Existing tests

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

crates/sui-config/src/node.rs

@@ -180,12 +180,6 @@ pub struct NodeConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub firewall_config: Option<RemoteFirewallConfig>,
 
-    /// Only applicable to nodes running json rpc server.
-    /// If true, node will inject client IP into headers
-    /// in validator requests for eligible rpc endpoints.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub with_client_ip_injection: Option<bool>,
-
     #[serde(default)]
     pub execution_cache: ExecutionCacheConfig,
 }

crates/sui-core/src/traffic_controller/policies.rs

@@ -174,7 +174,6 @@ pub enum TrafficControlPolicy {
     NoOp(NoOpPolicy),
     // Test policies below this point
     TestNConnIP(TestNConnIPPolicy),
-    TestInspectIp(TestInspectIpPolicy),
     TestPanicOnInvocation(TestPanicOnInvocationPolicy),
 }
 
@@ -184,7 +183,6 @@ impl Policy for TrafficControlPolicy {
             TrafficControlPolicy::NoOp(policy) => policy.handle_tally(tally),
             TrafficControlPolicy::FreqThreshold(policy) => policy.handle_tally(tally),
             TrafficControlPolicy::TestNConnIP(policy) => policy.handle_tally(tally),
-            TrafficControlPolicy::TestInspectIp(policy) => policy.handle_tally(tally),
             TrafficControlPolicy::TestPanicOnInvocation(policy) => policy.handle_tally(tally),
         }
     }
@@ -194,7 +192,6 @@ impl Policy for TrafficControlPolicy {
             TrafficControlPolicy::NoOp(policy) => policy.policy_config(),
             TrafficControlPolicy::FreqThreshold(policy) => policy.policy_config(),
             TrafficControlPolicy::TestNConnIP(policy) => policy.policy_config(),
-            TrafficControlPolicy::TestInspectIp(policy) => policy.policy_config(),
             TrafficControlPolicy::TestPanicOnInvocation(policy) => policy.policy_config(),
         }
     }
@@ -216,9 +213,6 @@ impl TrafficControlPolicy {
             PolicyType::TestNConnIP(n) => {
                 Self::TestNConnIP(TestNConnIPPolicy::new(policy_config, n).await)
             }
-            PolicyType::TestInspectIp => {
-                Self::TestInspectIp(TestInspectIpPolicy::new(policy_config))
-            }
             PolicyType::TestPanicOnInvocation => {
                 Self::TestPanicOnInvocation(TestPanicOnInvocationPolicy::new(policy_config))
             }
@@ -373,29 +367,6 @@ async fn run_clear_frequencies(frequencies: Arc<RwLock<HashMap<IpAddr, u64>>>, w
     }
 }
 
-#[derive(Clone)]
-pub struct TestInspectIpPolicy {
-    config: PolicyConfig,
-}
-
-impl TestInspectIpPolicy {
-    pub fn new(config: PolicyConfig) -> Self {
-        Self { config }
-    }
-
-    fn handle_tally(&mut self, tally: TrafficTally) -> PolicyResponse {
-        assert!(tally.proxy_ip.is_some(), "Expected proxy_ip to be present");
-        PolicyResponse {
-            block_connection_ip: None,
-            block_proxy_ip: None,
-        }
-    }
-
-    fn policy_config(&self) -> &PolicyConfig {
-        &self.config
-    }
-}
-
 #[derive(Clone)]
 pub struct TestPanicOnInvocationPolicy {
     config: PolicyConfig,

crates/sui-e2e-tests/tests/traffic_control_tests.rs

@@ -27,13 +27,56 @@ use sui_types::{
 };
 use test_cluster::{TestCluster, TestClusterBuilder};
 
+#[tokio::test]
+async fn test_validator_traffic_control_noop() -> Result<(), anyhow::Error> {
+    let policy_config = PolicyConfig {
+        connection_blocklist_ttl_sec: 1,
+        proxy_blocklist_ttl_sec: 5,
+        spam_policy_type: PolicyType::NoOp,
+        // This should never be invoked when set as an error policy
+        // as we are not sending requests that error
+        error_policy_type: PolicyType::TestPanicOnInvocation,
+        channel_capacity: 100,
+        dry_run: false,
+        spam_sample_rate: Weight::one(),
+    };
+    let network_config = ConfigBuilder::new_with_temp_dir()
+        .with_policy_config(Some(policy_config))
+        .build();
+    let test_cluster = TestClusterBuilder::new()
+        .set_network_config(network_config)
+        .build()
+        .await;
+
+    assert_traffic_control_ok(test_cluster).await
+}
+
+#[tokio::test]
+async fn test_fullnode_traffic_control_noop() -> Result<(), anyhow::Error> {
+    let policy_config = PolicyConfig {
+        connection_blocklist_ttl_sec: 1,
+        proxy_blocklist_ttl_sec: 5,
+        spam_policy_type: PolicyType::NoOp,
+        // This should never be invoked when set as an error policy
+        // as we are not sending requests that error
+        error_policy_type: PolicyType::TestPanicOnInvocation,
+        channel_capacity: 100,
+        spam_sample_rate: Weight::one(),
+        dry_run: false,
+    };
+    let test_cluster = TestClusterBuilder::new()
+        .with_fullnode_policy_config(Some(policy_config))
+        .build()
+        .await;
+    assert_traffic_control_ok(test_cluster).await
+}
+
 #[tokio::test]
 async fn test_validator_traffic_control_ok() -> Result<(), anyhow::Error> {
     let policy_config = PolicyConfig {
         connection_blocklist_ttl_sec: 1,
         proxy_blocklist_ttl_sec: 5,
-        // Test that IP forwarding works through this policy
-        spam_policy_type: PolicyType::TestInspectIp,
+        spam_policy_type: PolicyType::TestNConnIP(5),
         // This should never be invoked when set as an error policy
         // as we are not sending requests that error
         error_policy_type: PolicyType::TestPanicOnInvocation,
@@ -46,7 +89,6 @@ async fn test_validator_traffic_control_ok() -> Result<(), anyhow::Error> {
         .build();
     let test_cluster = TestClusterBuilder::new()
         .set_network_config(network_config)
-        .with_fullnode_client_ip_injection(Some(true))
         .build()
         .await;
 
@@ -58,13 +100,13 @@ async fn test_fullnode_traffic_control_ok() -> Result<(), anyhow::Error> {
     let policy_config = PolicyConfig {
         connection_blocklist_ttl_sec: 1,
         proxy_blocklist_ttl_sec: 5,
+        spam_policy_type: PolicyType::TestNConnIP(10),
         // This should never be invoked when set as an error policy
         // as we are not sending requests that error
         error_policy_type: PolicyType::TestPanicOnInvocation,
         channel_capacity: 100,
         spam_sample_rate: Weight::one(),
         dry_run: false,
-        ..Default::default()
     };
     let test_cluster = TestClusterBuilder::new()
         .with_fullnode_policy_config(Some(policy_config))

crates/sui-indexer/src/apis/write_api.rs

@@ -7,7 +7,6 @@ use jsonrpsee::core::RpcResult;
 use jsonrpsee::http_client::HttpClient;
 use jsonrpsee::RpcModule;
 
-use std::net::SocketAddr;
 use sui_json_rpc::SuiRpcModule;
 use sui_json_rpc_api::{WriteApiClient, WriteApiServer};
 use sui_json_rpc_types::{
@@ -53,20 +52,6 @@ impl WriteApiServer for WriteApi {
         .into())
     }
 
-    async fn monitored_execute_transaction_block(
-        &self,
-        tx_bytes: Base64,
-        signatures: Vec<Base64>,
-        options: Option<SuiTransactionBlockResponseOptions>,
-        request_type: Option<ExecuteTransactionRequestType>,
-        _client_addr: Option<SocketAddr>,
-    ) -> RpcResult<SuiTransactionBlockResponse> {
-        // TODO(william) do we nee to do anything in terms of client IP tracking
-        // for indexer case?
-        self.execute_transaction_block(tx_bytes, signatures, options, request_type)
-            .await
-    }
-
     async fn dev_inspect_transaction_block(
         &self,
         sender_address: SuiAddress,

crates/sui-indexer/src/lib.rs

@@ -158,13 +158,8 @@ pub async fn build_json_rpc_server<T: R2D2Connection>(
     config: &IndexerConfig,
     custom_runtime: Option<Handle>,
 ) -> Result<ServerHandle, IndexerError> {
-    let mut builder = JsonRpcServerBuilder::new(
-        env!("CARGO_PKG_VERSION"),
-        prometheus_registry,
-        None,
-        None,
-        None,
-    );
+    let mut builder =
+        JsonRpcServerBuilder::new(env!("CARGO_PKG_VERSION"), prometheus_registry, None, None);
     let http_client = crate::get_http_client(config.rpc_client_url.as_str())?;
 
     let name_service_config =

crates/sui-json-rpc-api/src/write.rs

@@ -1,8 +1,6 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::net::SocketAddr;
-
 use fastcrypto::encoding::Base64;
 use jsonrpsee::core::RpcResult;
 use jsonrpsee::proc_macros::rpc;
@@ -42,21 +40,6 @@ pub trait WriteApi {
         request_type: Option<ExecuteTransactionRequestType>,
     ) -> RpcResult<SuiTransactionBlockResponse>;
 
-    #[method(name = "monitoredExecuteTransactionBlock")]
-    async fn monitored_execute_transaction_block(
-        &self,
-        /// BCS serialized transaction data bytes without its type tag, as base-64 encoded string.
-        tx_bytes: Base64,
-        /// A list of signatures (`flag || signature || pubkey` bytes, as base-64 encoded string). Signature is committed to the intent message of the transaction data, as base-64 encoded string.
-        signatures: Vec<Base64>,
-        /// options for specifying the content to be returned
-        options: Option<SuiTransactionBlockResponseOptions>,
-        /// The request type, derived from `SuiTransactionBlockResponseOptions` if None
-        request_type: Option<ExecuteTransactionRequestType>,
-        /// The client IP address, used for monitoring
-        client_addr: Option<SocketAddr>,
-    ) -> RpcResult<SuiTransactionBlockResponse>;
-
     /// Runs the transaction in dev-inspect mode. Which allows for nearly any
     /// transaction (or Move call) with any arguments. Detailed results are
     /// provided, including both the transaction effects and any return values.

crates/sui-json-rpc-tests/tests/routing_tests.rs

@@ -20,7 +20,7 @@ use sui_open_rpc_macros::open_rpc;
 
 #[tokio::test]
 async fn test_rpc_backward_compatibility() {
-    let mut builder = JsonRpcServerBuilder::new("1.5", &Registry::new(), None, None, None);
+    let mut builder = JsonRpcServerBuilder::new("1.5", &Registry::new(), None, None);
     builder.register_module(TestApiModule).unwrap();
 
     let address = local_ip_utils::new_local_tcp_socket_for_testing();
@@ -99,7 +99,7 @@ async fn test_rpc_backward_compatibility() {
 async fn test_disable_routing() {
     env::set_var("DISABLE_BACKWARD_COMPATIBILITY", "true");
 
-    let mut builder = JsonRpcServerBuilder::new("1.5", &Registry::new(), None, None, None);
+    let mut builder = JsonRpcServerBuilder::new("1.5", &Registry::new(), None, None);
     builder.register_module(TestApiModule).unwrap();
 
     let address = local_ip_utils::new_local_tcp_socket_for_testing();
@@ -135,10 +135,10 @@ async fn test_disable_routing() {
 // #[tokio::test]
 // async fn test_rpc_backward_compatibility_batched_request() {
 //     let mut builder = JsonRpcServerBuilder::new(
-//          "1.5", &Registry::new(), None, None, None,
+//          "1.5", &Registry::new(), None, None,
 //     );
 //     let mut builder = JsonRpcServerBuilder::new(
-//          "1.5", &Registry::new(), None, None, None,
+//          "1.5", &Registry::new(), None, None,
 //     );
 //     builder.register_module(TestApiModule).unwrap();