Skip to content

Commit 16f5f20

Browse files
ritik4everclaude
andcommitted
fix: Replace hardcoded solana_ prefix with BlockChainType lookup
This commit addresses issue #409 by replacing the fragile string-based detection of Solana networks with proper blockchain type lookup. Changes: - Added validate_with_networks() method to Monitor ConfigLoader impl - Updated validate_monitor_references() to use proper type detection - Maintained backward compatibility with fallback to heuristic approach - Added comprehensive test coverage for network type detection Benefits: - Works with custom Solana network names (not just solana_*) - Prevents false positives from misleading network names - Type-safe validation using BlockChainType enum - No breaking changes to existing code This builds on the Solana support added in #408 by making the network type detection more robust and flexible. Fixes #409 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent ed8bc92 commit 16f5f20

File tree

2 files changed

+124
-4
lines changed

2 files changed

+124
-4
lines changed

src/models/config/monitor_config.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//! allowing monitors to be loaded from JSON files.
55
66
use crate::{
7-
models::{config::error::ConfigError, ConfigLoader, Monitor, SecretValue},
7+
models::{config::error::ConfigError, BlockChainType, ConfigLoader, Monitor, Network, SecretValue},
88
services::trigger::validate_script_config,
99
utils::normalize_string,
1010
};
@@ -154,6 +154,20 @@ impl ConfigLoader for Monitor {
154154

155155
/// Validate the monitor configuration
156156
fn validate(&self) -> Result<(), ConfigError> {
157+
self.validate_with_networks(None)
158+
}
159+
160+
/// Validate the monitor configuration with network context
161+
///
162+
/// # Arguments
163+
/// * `networks` - Optional HashMap of network configurations to validate blockchain type
164+
///
165+
/// This is a public method exposed through an inherent impl on Monitor to allow
166+
/// proper blockchain type detection in repository-level validation
167+
pub fn validate_with_networks(
168+
&self,
169+
networks: Option<&HashMap<String, Network>>,
170+
) -> Result<(), ConfigError> {
157171
// Validate monitor name
158172
if self.name.is_empty() {
159173
return Err(ConfigError::validation_error(
@@ -172,9 +186,20 @@ impl ConfigLoader for Monitor {
172186
));
173187
}
174188

175-
// Check if this is a Solana monitor based on network slugs
176-
// Note: Assumes Solana network slugs follow the "solana_*" naming convention
177-
let is_solana_monitor = self.networks.iter().any(|slug| slug.starts_with("solana_"));
189+
// Determine if this is a Solana monitor by checking actual network types
190+
let is_solana_monitor = if let Some(network_configs) = networks {
191+
// Look up actual blockchain type from network configurations
192+
self.networks.iter().any(|slug| {
193+
network_configs
194+
.get(slug)
195+
.map(|network| matches!(network.network_type, BlockChainType::Solana))
196+
.unwrap_or(false)
197+
})
198+
} else {
199+
// Fallback to heuristic-based approach when network configs aren't available
200+
// This maintains backward compatibility for direct validate() calls
201+
self.networks.iter().any(|slug| slug.starts_with("solana_"))
202+
};
178203

179204
// Validate function signatures
180205
for func in &self.match_conditions.functions {

src/repositories/monitor.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,19 @@ impl<
100100
}
101101
}
102102

103+
// Validate monitor configuration with network context
104+
// This allows proper blockchain type detection instead of relying on naming conventions
105+
if let Err(e) = monitor.validate_with_networks(Some(networks)) {
106+
validation_errors.push(format!(
107+
"Monitor '{}' configuration validation failed: {}",
108+
monitor_name, e
109+
));
110+
metadata.insert(
111+
format!("monitor_{}_validation_error", monitor_name),
112+
e.to_string(),
113+
);
114+
}
115+
103116
// Validate custom trigger conditions
104117
for condition in &monitor.trigger_conditions {
105118
let script_path = Path::new(&condition.script_path);
@@ -626,4 +639,86 @@ mod tests {
626639
_ => panic!("Expected RepositoryError::LoadError"),
627640
}
628641
}
642+
643+
#[test]
644+
fn test_validate_solana_network_type_detection() {
645+
use crate::models::BlockChainType;
646+
647+
// Test Case 1: Custom-named Solana network (not following solana_* convention)
648+
let mut networks = HashMap::new();
649+
let solana_network = NetworkBuilder::new()
650+
.name("my-custom-solana")
651+
.network_type(BlockChainType::Solana)
652+
.build();
653+
networks.insert("my-custom-solana".to_string(), solana_network);
654+
655+
let mut monitors = HashMap::new();
656+
let monitor = MonitorBuilder::new()
657+
.name("test_solana_monitor")
658+
.networks(vec!["my-custom-solana".to_string()])
659+
// Solana monitors allow function signatures without parentheses
660+
.function_signatures(vec!["transfer".to_string()])
661+
.build();
662+
monitors.insert("test_solana_monitor".to_string(), monitor);
663+
664+
let triggers = HashMap::new();
665+
666+
// Should pass because we properly detect Solana network type
667+
let result =
668+
MonitorRepository::<NetworkRepository, TriggerRepository>::validate_monitor_references(
669+
&monitors, &triggers, &networks,
670+
);
671+
assert!(result.is_ok());
672+
673+
// Test Case 2: Misleading network name (contains "solana_" but is EVM)
674+
let mut networks2 = HashMap::new();
675+
let evm_network = NetworkBuilder::new()
676+
.name("solana_like_evm")
677+
.network_type(BlockChainType::EVM)
678+
.build();
679+
networks2.insert("solana_like_evm".to_string(), evm_network);
680+
681+
let mut monitors2 = HashMap::new();
682+
let monitor2 = MonitorBuilder::new()
683+
.name("test_evm_monitor")
684+
.networks(vec!["solana_like_evm".to_string()])
685+
// EVM monitors require parentheses in signatures
686+
.function_signatures(vec!["transfer".to_string()])
687+
.build();
688+
monitors2.insert("test_evm_monitor".to_string(), monitor2);
689+
690+
// Should fail because it's actually an EVM network requiring proper signature format
691+
let result2 =
692+
MonitorRepository::<NetworkRepository, TriggerRepository>::validate_monitor_references(
693+
&monitors2, &triggers, &networks2,
694+
);
695+
assert!(result2.is_err());
696+
assert!(result2
697+
.unwrap_err()
698+
.to_string()
699+
.contains("Invalid function signature format"));
700+
701+
// Test Case 3: Actual Solana network with conventional name
702+
let mut networks3 = HashMap::new();
703+
let solana_network3 = NetworkBuilder::new()
704+
.name("solana_mainnet")
705+
.network_type(BlockChainType::Solana)
706+
.build();
707+
networks3.insert("solana_mainnet".to_string(), solana_network3);
708+
709+
let mut monitors3 = HashMap::new();
710+
let monitor3 = MonitorBuilder::new()
711+
.name("test_conventional_solana")
712+
.networks(vec!["solana_mainnet".to_string()])
713+
.function_signatures(vec!["transfer".to_string()])
714+
.build();
715+
monitors3.insert("test_conventional_solana".to_string(), monitor3);
716+
717+
// Should pass with conventional name too
718+
let result3 =
719+
MonitorRepository::<NetworkRepository, TriggerRepository>::validate_monitor_references(
720+
&monitors3, &triggers, &networks3,
721+
);
722+
assert!(result3.is_ok());
723+
}
629724
}

0 commit comments

Comments
 (0)