-
Notifications
You must be signed in to change notification settings - Fork 26
feat: add rpc filter for early rejection of configured method-param t… #64
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,8 @@ const ROUTE_HEADER_LABEL_MISSING: &str = "missing"; | |
| const JSON_RPC_INTERNAL_ERROR_CODE: i64 = -32603; | ||
| const JSON_RPC_REQUEST_TIMEOUT_CODE: i64 = -32000; | ||
| const JSON_RPC_REQUEST_TIMEOUT_MESSAGE: &str = "Request timeout"; | ||
| const JSON_RPC_METHOD_NOT_ALLOWED_CODE: i32 = -32601; | ||
| const JSON_RPC_METHOD_NOT_ALLOWED_MESSAGE: &str = "Method not allowed"; | ||
| const HEADER_X_ENDPOINT: &str = "X-Endpoint"; | ||
| const HEADER_X_RPC_NODE: &str = "X-RPC-Node"; | ||
| const HEADER_X_SUBSCRIPTION_ID: &str = "X-Subscription-ID"; | ||
|
|
@@ -639,6 +641,32 @@ fn json_rpc_error_value( | |
| Value::Object(response) | ||
| } | ||
|
|
||
| fn observe_parameter_filter_match(method: &str) { | ||
| if let Some(tracker) = metrics::track_request(metrics_method_label(method)) { | ||
| tracker.observe(StatusCode::METHOD_NOT_ALLOWED); | ||
| } | ||
| } | ||
|
|
||
| fn parameter_filter_response(id: Value) -> Response { | ||
| let mut response = json_rpc_error_response( | ||
| id, | ||
| JSON_RPC_METHOD_NOT_ALLOWED_CODE, | ||
| JSON_RPC_METHOD_NOT_ALLOWED_MESSAGE, | ||
| None, | ||
| ); | ||
| *response.status_mut() = StatusCode::METHOD_NOT_ALLOWED; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sets 405 unconditionally. Everywhere else the server treats HTTP error statuses as opt-in via Should the 405 be gated behind emit_http_errors (200 by default, like every other client error) so the "HTTP errors are opt-in" contract holds? If the 405 is intentional for ops visibility, maybe a one-line exception in the HTTP-status section so the two don't read as contradictory. |
||
| response | ||
| } | ||
|
|
||
| fn parameter_filter_response_value(id: Value) -> Value { | ||
| json_rpc_error_value( | ||
| id, | ||
| JSON_RPC_METHOD_NOT_ALLOWED_CODE, | ||
| JSON_RPC_METHOD_NOT_ALLOWED_MESSAGE, | ||
| None, | ||
| ) | ||
| } | ||
|
|
||
| fn is_http_503_eligible_json_rpc_error(code: i64, message: Option<&str>) -> bool { | ||
| if code == JSON_RPC_INTERNAL_ERROR_CODE { | ||
| return true; | ||
|
|
@@ -1059,6 +1087,19 @@ async fn handle_single_request( | |
| } | ||
| }; | ||
|
|
||
| if state | ||
| .rpc_parameter_filters | ||
| .matches(&request.method, request.params.as_deref()) | ||
| { | ||
| let method = request.method; | ||
| let id = request.id.unwrap_or(Value::Null); | ||
| return metrics::with_request_metric_labels(request_metric_labels, async move { | ||
| observe_parameter_filter_match(&method); | ||
| Ok(parameter_filter_response(id)) | ||
| }) | ||
| .await; | ||
| } | ||
|
|
||
| let timeout = state.rpc_request_timeout; | ||
| let dispatch_request = request.into_dispatch_request(); | ||
| let response = metrics::with_request_metric_labels( | ||
|
|
@@ -1084,10 +1125,23 @@ async fn execute_batch_requests( | |
|
|
||
| let mut responses: Vec<Option<BatchItemResponse>> = (0..batch_len).map(|_| None).collect(); | ||
| let mut join_set: JoinSet<BatchTaskOutput> = JoinSet::new(); | ||
| let mut parameter_filter_matched = false; | ||
|
|
||
| for (idx, request_value) in requests.into_iter().enumerate() { | ||
| match parse_json_rpc_request(request_value) { | ||
| Ok(request) => { | ||
| if state | ||
| .rpc_parameter_filters | ||
| .matches(&request.method, request.params.as_deref()) | ||
| { | ||
| observe_parameter_filter_match(&request.method); | ||
| responses[idx] = Some(parameter_filter_response_value( | ||
| request.id.unwrap_or(Value::Null), | ||
| )); | ||
| parameter_filter_matched = true; | ||
| continue; | ||
| } | ||
|
|
||
| let response_id = request.id.clone().unwrap_or(Value::Null); | ||
| let dispatch_request = request.into_dispatch_request(); | ||
| let state = state.clone(); | ||
|
|
@@ -1159,7 +1213,11 @@ async fn execute_batch_requests( | |
| response_values.push(value); | ||
| } | ||
|
|
||
| Ok(Json(Value::Array(response_values)).into_response()) | ||
| let mut response = Json(Value::Array(response_values)).into_response(); | ||
| if parameter_filter_matched { | ||
| *response.status_mut() = StatusCode::METHOD_NOT_ALLOWED; | ||
|
Comment on lines
+1216
to
+1218
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One filtered item sets the whole batch's HTTP status to 405, while Related: if emit_http_errors is on and a batch has both a filtered item and a 503-eligible error, promote_http_status_for_json_rpc_errors overwrites this 405 with 503 unconditionally, so the documented "returns 405" doesn't hold in that combo. |
||
| } | ||
| Ok(response) | ||
| } | ||
|
|
||
| async fn handle_batch_request( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| // SPDX-License-Identifier: AGPL-3.0-only | ||
| /* | ||
| * Copyright 2025-2026 Triton One Limited. All rights reserved. | ||
| */ | ||
|
|
||
| use std::{ | ||
| collections::HashMap, | ||
| fs, | ||
| path::{Path, PathBuf}, | ||
| }; | ||
|
|
||
| use serde::Deserialize; | ||
| use serde_json::Value; | ||
|
|
||
| #[derive(Debug, Default, Deserialize)] | ||
| #[serde(default, rename_all = "kebab-case")] | ||
| struct RpcFileConfig { | ||
| rpc_parameter_filters: Vec<Vec<Value>>, | ||
| } | ||
|
|
||
| /// Startup-compiled RPC request filters. | ||
| /// | ||
| /// The first lookup excludes unfiltered methods. The arity lookup then excludes | ||
| /// differently-shaped calls before any structural JSON comparisons are needed. | ||
| #[derive(Debug, Default)] | ||
| pub(crate) struct RpcParameterFilterSet { | ||
| by_method_and_arity: HashMap<String, HashMap<usize, Vec<Vec<Value>>>>, | ||
| len: usize, | ||
| } | ||
|
|
||
| impl RpcParameterFilterSet { | ||
| pub(crate) fn load(path: Option<&Path>) -> Result<Self, String> { | ||
| let Some(path) = path else { | ||
| return Ok(Self::default()); | ||
| }; | ||
|
|
||
| let contents = fs::read_to_string(path) | ||
| .map_err(|err| format!("failed to read config file '{}': {err}", path.display()))?; | ||
| let config: RpcFileConfig = serde_yaml::from_str(&contents) | ||
| .map_err(|err| format!("failed to parse config file '{}': {err}", path.display()))?; | ||
| Self::from_entries(config.rpc_parameter_filters, Some(path)) | ||
| } | ||
|
|
||
| pub(crate) fn matches(&self, method: &str, params: Option<&[Value]>) -> bool { | ||
| if self.len == 0 { | ||
| return false; | ||
| } | ||
| let Some(params) = params else { | ||
| return false; | ||
| }; | ||
| self.by_method_and_arity | ||
| .get(method) | ||
| .and_then(|by_arity| by_arity.get(¶ms.len())) | ||
| .is_some_and(|candidates| candidates.iter().any(|candidate| candidate == params)) | ||
| } | ||
|
|
||
| pub(crate) fn len(&self) -> usize { | ||
| self.len | ||
| } | ||
|
|
||
| pub(crate) fn from_entries( | ||
| entries: Vec<Vec<Value>>, | ||
| path: Option<&Path>, | ||
| ) -> Result<Self, String> { | ||
| let mut filters = Self::default(); | ||
| for (index, mut entry) in entries.into_iter().enumerate() { | ||
| if entry.is_empty() { | ||
| return Err(Self::entry_error(path, index, "entry must not be empty")); | ||
| } | ||
| let method = entry.remove(0); | ||
| let Value::String(method) = method else { | ||
| return Err(Self::entry_error( | ||
| path, | ||
| index, | ||
| "first value must be a method string", | ||
| )); | ||
| }; | ||
| if method.trim().is_empty() { | ||
| return Err(Self::entry_error(path, index, "method must not be empty")); | ||
| } | ||
|
|
||
| let candidates = filters | ||
| .by_method_and_arity | ||
| .entry(method) | ||
|
Comment on lines
+78
to
+84
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small one: the loader validates |
||
| .or_default() | ||
| .entry(entry.len()) | ||
| .or_default(); | ||
| if !candidates.contains(&entry) { | ||
| candidates.push(entry); | ||
| filters.len += 1; | ||
| } | ||
| } | ||
| Ok(filters) | ||
| } | ||
|
|
||
| fn entry_error(path: Option<&Path>, index: usize, message: &str) -> String { | ||
| let source = path | ||
| .map(Path::to_path_buf) | ||
| .unwrap_or_else(|| PathBuf::from("<inline>")); | ||
| format!( | ||
| "invalid rpc-parameter-filters entry {} in '{}': {message}", | ||
| index + 1, | ||
| source.display() | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use serde_json::json; | ||
|
|
||
| use super::RpcParameterFilterSet; | ||
|
|
||
| fn filters(entries: Vec<Vec<Value>>) -> RpcParameterFilterSet { | ||
| RpcParameterFilterSet::from_entries(entries, None).expect("valid filters") | ||
| } | ||
|
|
||
| use serde_json::Value; | ||
|
|
||
| #[test] | ||
| fn exact_match_is_indexed_by_method_and_arity() { | ||
| let filters = filters(vec![ | ||
| vec![json!("getThing"), json!("address")], | ||
| vec![json!("getThing"), json!("address"), json!({"mode": "full"})], | ||
| ]); | ||
|
|
||
| assert!(filters.matches("getThing", Some(&[json!("address")]))); | ||
| assert!(filters.matches( | ||
| "getThing", | ||
| Some(&[json!("address"), json!({"mode": "full"})]) | ||
| )); | ||
| assert!(!filters.matches("getthing", Some(&[json!("address")]))); | ||
| assert!(!filters.matches( | ||
| "getThing", | ||
| Some(&[json!("address"), json!({"mode": "other"})]) | ||
| )); | ||
| assert!(!filters.matches( | ||
| "getThing", | ||
| Some(&[json!("address"), json!({"mode": "full"}), json!(true)]) | ||
| )); | ||
| assert!(!filters.matches("getThing", None)); | ||
| } | ||
|
|
||
| #[test] | ||
| fn object_key_order_does_not_affect_equality() { | ||
| let filters = filters(vec![vec![ | ||
| json!("getThing"), | ||
| json!({"first": 1, "second": 2}), | ||
| ]]); | ||
|
|
||
| assert!(filters.matches("getThing", Some(&[json!({"second": 2, "first": 1})]))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn duplicate_entries_are_removed() { | ||
| let filters = filters(vec![ | ||
| vec![json!("getThing"), json!("address")], | ||
| vec![json!("getThing"), json!("address")], | ||
| ]); | ||
|
|
||
| assert_eq!(filters.len(), 1); | ||
| } | ||
|
|
||
| #[test] | ||
| fn invalid_entries_are_rejected_with_index() { | ||
| let err = RpcParameterFilterSet::from_entries( | ||
| vec![vec![json!("getThing"), json!(1)], vec![json!(42)]], | ||
| None, | ||
| ) | ||
| .expect_err("non-string method must fail"); | ||
|
|
||
| assert!(err.contains("entry 2")); | ||
| assert!(err.contains("first value must be a method string")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn shared_yaml_ignores_ingestor_keys_and_loads_filters() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let path = dir.path().join("superbank.yaml"); | ||
| std::fs::write( | ||
| &path, | ||
| r#" | ||
| source: grpc | ||
| endpoint: https://example.invalid | ||
| rpc-parameter-filters: | ||
| - [getThing, address] | ||
| "#, | ||
| ) | ||
| .expect("write config"); | ||
|
|
||
| let filters = RpcParameterFilterSet::load(Some(&path)).expect("load config"); | ||
| assert!(filters.matches("getThing", Some(&[json!("address")]))); | ||
| } | ||
| } | ||
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.
This reuses
-32601, which is JSON-RPC's "Method not found" and the same code the server already returns for genuinely-unknown methods (the fallback arm in dispatch, and tests/mod.rs assertscode == -32601). So a client keying on the code can't tell a blocked (method, params) from a method that doesn't exist. Only the message text and the HTTP status differ.The repo already handles this exact shape (method is valid, a specific param value is rejected by server policy): the commitment=processed rejection in signatures.rs and blocks.rs returns
-32602"Only confirmed or finalized commitments are supported" with data: {requestedCommitment}. Could the filter match that, -32602 + data describing the matched method/params, or take a dedicated code in the-32000..-32099server range if it should read as "server-imposed"?