fix(BOUN-1255): fix canister queries, add cli interval (#3078)

ic-os/components/boundary-guestos/opt/ic/bin/setup-ic-boundary.sh

@@ -59,6 +59,7 @@ HTTP_CLIENT_TIMEOUT_CONNECT="3s"
 NFTABLES_SYSTEM_REPLICAS_PATH="/run/ic-node/etc/nftables/system_replicas.ruleset"
 RETRY_UPDATE_CALL="true"
 RATE_LIMIT_PER_SECOND_PER_SUBNET="1000"
+RATE_LIMIT_GENERIC_FILE="/run/ic-node/etc/ic-boundary/canister-ratelimit.yml"
 REGISTRY_NNS_URLS="${NNS_URL}"
 REGISTRY_NNS_PUB_KEY_PEM="/run/ic-node/etc/default/nns_public_key.pem"
 REGISTRY_LOCAL_STORE_PATH="/var/opt/registry/store"

rs/boundary_node/ic_boundary/src/cli.rs

@@ -252,6 +252,7 @@ pub struct RateLimiting {
     /// Allowed number of update calls per second per ip per boundary node. Panics if 0 is passed!
     #[clap(env, long)]
     pub rate_limit_per_second_per_ip: Option<u32>,
+
     /// Path to a generic rate-limiter rules, if the file does not exist - no rules are applied.
     /// File is checked every 10sec and is reloaded if the changes are detected.
     /// Expecting YAML list with objects that have at least one of
@@ -274,6 +275,10 @@ pub struct RateLimiting {
     /// If specified together with the file above - file takes precedence.
     #[clap(env, long)]
     pub rate_limit_generic_canister_id: Option<CanisterId>,
+
+    /// How frequently to poll for rules (from file or canister)
+    #[clap(env, long, default_value = "30s", value_parser = parse_duration)]
+    pub rate_limit_generic_poll_interval: Duration,
 }
 
 #[derive(Args)]
@@ -383,7 +388,8 @@ pub struct Misc {
     #[clap(env, long)]
     pub skip_replica_tls_verification: bool,
 
-    /// Configuration of the node's crypto-vault
+    /// Configuration of the node's crypto-vault to use with the IC agent.
+    /// If not specified - then the agent will use anonymous sender.
     #[clap(env, long, value_parser=parse_crypto_config)]
     pub crypto_config: Option<CryptoConfig>,
 }

rs/boundary_node/ic_boundary/src/core.rs

@@ -238,13 +238,19 @@ pub async fn main(cli: Cli) -> Result<(), Error> {
     let agent = if cli.rate_limiting.rate_limit_generic_canister_id.is_some()
         || cli.obs.obs_log_anonymization_canister_id.is_some()
     {
-        if cli.misc.crypto_config.is_none() || registry_client.is_none() {
-            return Err(anyhow!("Registry and crypto config are both required to use rate-limiting canister or anonymization salt canister"));
+        if cli.misc.crypto_config.is_some() && registry_client.is_none() {
+            return Err(anyhow!(
+                "IC-Agent: registry client is required when crypto-config is in use"
+            ));
+        }
+
+        if cli.misc.crypto_config.is_none() {
+            warn!("IC-Agent: crypto-config is missing, using anonymous principal");
         }
 
         let agent = create_agent(
-            cli.misc.crypto_config.clone().unwrap(),
-            registry_client.unwrap(),
+            cli.misc.crypto_config.clone(),
+            registry_client,
             cli.listen.listen_http_port_loopback,
         )
         .await?;
@@ -415,7 +421,10 @@ pub async fn main(cli: Cli) -> Result<(), Error> {
     runners.push(Box::new(metrics_runner));
 
     if let Some(v) = generic_limiter {
-        let runner = Box::new(WithThrottle(v, ThrottleParams::new(10 * SECOND)));
+        let runner = Box::new(WithThrottle(
+            v,
+            ThrottleParams::new(cli.rate_limiting.rate_limit_generic_poll_interval),
+        ));
         runners.push(runner);
     }
 
@@ -504,11 +513,10 @@ pub async fn main(cli: Cli) -> Result<(), Error> {
     Ok(())
 }
 
-async fn create_agent(
+async fn create_sender(
     crypto_config: CryptoConfig,
     registry_client: Arc<RegistryClientImpl>,
-    port: u16,
-) -> Result<Agent, Error> {
+) -> Result<Sender, Error> {
     let crypto_component = tokio::task::spawn_blocking({
         let registry_client = Arc::clone(&registry_client);
 
@@ -540,7 +548,7 @@ async fn create_agent(
     let node_id = derive_node_id(&public_key).expect("failed to derive node id");
 
     // Custom Signer
-    let sender = Sender::Node {
+    Ok(Sender::Node {
         pub_key: public_key.key_value,
         sign: Arc::new(move |msg: &MessageId| {
             #[allow(clippy::disallowed_methods)]
@@ -553,6 +561,18 @@ async fn create_agent(
 
             Ok(sig)
         }),
+    })
+}
+
+async fn create_agent(
+    crypto_config: Option<CryptoConfig>,
+    registry_client: Option<Arc<RegistryClientImpl>>,
+    port: u16,
+) -> Result<Agent, Error> {
+    let sender = if let (Some(v), Some(r)) = (crypto_config, registry_client) {
+        create_sender(v, r).await?
+    } else {
+        Sender::Anonymous
     };
 
     let agent = Agent::new(format!("http://127.0.0.1:{port}").parse()?, sender);

rs/boundary_node/ic_boundary/src/rate_limiting/fetcher.rs

@@ -1,24 +1,15 @@
-use std::{path::PathBuf, sync::Arc, time::SystemTime};
+use std::{path::PathBuf, sync::Arc};
 
 use anyhow::{anyhow, Context as _, Error};
 use async_trait::async_trait;
-use candid::Decode;
+use candid::{Decode, Encode};
 use ic_canister_client::Agent;
 use ic_types::CanisterId;
-use rate_limits_api::{v1::RateLimitRule, GetConfigResponse};
+use rate_limits_api::{v1::RateLimitRule, GetConfigResponse, Version};
 use tokio::fs;
 
 const SCHEMA_VERSION: u64 = 1;
 
-fn nonce() -> Vec<u8> {
-    SystemTime::now()
-        .duration_since(SystemTime::UNIX_EPOCH)
-        .unwrap()
-        .as_nanos()
-        .to_le_bytes()
-        .to_vec()
-}
-
 #[async_trait]
 pub trait FetchesRules: Send + Sync {
     async fn fetch_rules(&self) -> Result<Vec<RateLimitRule>, Error>;
@@ -56,12 +47,10 @@ pub struct CanisterConfigFetcher(pub Agent, pub CanisterId);
 impl FetchesConfig for CanisterConfigFetcher {
     async fn fetch_config(&self) -> Result<Vec<u8>, Error> {
         self.0
-            .execute_update(
-                &self.1,      // effective_canister_id
-                &self.1,      // canister_id
-                "get_config", // method
-                vec![],       // arguments
-                nonce(),      // nonce
+            .execute_query(
+                &self.1,                            // effective_canister_id
+                "get_config",                       // method
+                Encode!(&None::<Version>).unwrap(), // arguments
             )
             .await
             .map_err(|e| anyhow!("failed to fetch config from the canister: {e:#}"))?

rs/boundary_node/ic_boundary/src/rate_limiting/generic.rs

@@ -126,6 +126,7 @@ impl Limiter {
             .fetch_rules()
             .await
             .context("unable to fetch rules")?;
+
         self.apply_rules(rules);
         Ok(())
     }
@@ -184,7 +185,10 @@ impl Limiter {
 #[async_trait]
 impl Run for Arc<Limiter> {
     async fn run(&mut self) -> Result<(), Error> {
-        self.refresh().await
+        if let Err(e) = self.refresh().await {
+            warn!("Ratelimiter: unable to refresh: {e:#}");
+        }
+        Ok(())
     }
 }