Skip to content
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

feat(boundary): setup of rate-limit canister #1961

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ members = [
"rs/boundary_node/certificate_issuance/certificate_orchestrator",
"rs/boundary_node/discower_bowndary",
"rs/boundary_node/ic_boundary",
"rs/boundary_node/rate_limits",
"rs/boundary_node/rate_limits/api",
"rs/boundary_node/rate_limits/canister_client",
"rs/boundary_node/systemd_journal_gatewayd_shim",
"rs/canister_client",
"rs/canister_client/sender",
Expand Down
21 changes: 21 additions & 0 deletions rs/boundary_node/rate_limits/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "rate_limits"
version.workspace = true
authors.workspace = true
edition.workspace = true
description.workspace = true
documentation.workspace = true

[dependencies]
candid = { workspace = true }
ciborium = { workspace = true }
ic-cdk = { workspace = true }
ic-cdk-macros = { workspace = true }
ic-cdk-timers = { workspace = true }
ic-stable-structures = { workspace = true }
rate-limits-api = { path = "./api" }
serde = { workspace = true }

[lib]
crate-type = ["cdylib"]
path = "canister/canister.rs"
14 changes: 14 additions & 0 deletions rs/boundary_node/rate_limits/api/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "rate-limits-api"
version.workspace = true
authors.workspace = true
edition.workspace = true
description.workspace = true
documentation.workspace = true

[dependencies]
candid = {workspace = true}
serde = {workspace = true}

[lib]
path = "src/lib.rs"
28 changes: 28 additions & 0 deletions rs/boundary_node/rate_limits/api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
use candid::CandidType;
use serde::Deserialize;

pub type Version = u64;
pub type Timestamp = u64;
pub type RuleId = String;

pub type GetConfigResponse = Result<ConfigResponse, String>;

#[derive(CandidType, Deserialize, Debug)]
pub struct ConfigResponse {
pub version: Version,
pub active_since: Timestamp,
pub config: OutputConfig,
}

#[derive(CandidType, Deserialize, Debug)]
pub struct OutputConfig {
pub rules: Vec<OutputRule>,
}

#[derive(CandidType, Deserialize, Debug)]
pub struct OutputRule {
pub id: RuleId,
pub rule_raw: Option<Vec<u8>>,
pub description: Option<String>,
pub disclosed_at: Option<Timestamp>,
}
23 changes: 23 additions & 0 deletions rs/boundary_node/rate_limits/canister/canister.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use rate_limits_api::{GetConfigResponse, Version};
use storage::VERSION;
use types::{ConfigResponse, OutputConfig};
mod storage;
mod types;

#[ic_cdk_macros::update]
fn get_config(version: Option<Version>) -> GetConfigResponse {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Friendly suggestion - you may not agree with it. Notice that canisters don't really have a main function, so it's hard to gauge "what runs first". This can make it tricky to have a clear understanding of the order of operations happening in the canister upon startup (the various initialization steps, like declaring stable memory, canister methods, etc). For that reason, my suggestion would be to not rush to break up the canister code into separate files. Instead, treat canister/lib.rs similarly to how you would treat your main.rs file, meaning configure all your initialization logic there sequentially, like you would do in a main function. So in this case:

# lib.rs

... define stable memory

... define dependencies that will be used in your canister methods

... define canister methods

then on a per-need basis you'd create new files to host things that get imported into your lib.rs. All this is mostly so that it's easier to understand the flow of the canister. As it is right now, you would have to ask yourself, what's happening first? Stable memory in storage.rs? Canister methods in canister.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that canisters don't really have a main function, so it's hard to gauge "what runs first".

Yes, canister is an actor.

For that reason, my suggestion would be to not rush to break up the canister code into separate files.

I understand. Since I have already prototyped the canister locally, i wanted to have at least a minimum separation of files. For example canister/types.rs and canister/canister.rs (lib.rs) make sense already now IMO. Stable memory variables and init i can add in the next MR. Wdyt? However, I still wanted a working canister at this point.

let test_version_inc = VERSION.with(|v| {
let mut ver = v.borrow_mut();
let current_version = ver.get(&()).unwrap_or(0);
ver.insert((), current_version + 1);
current_version
});

let response = ConfigResponse {
version: version.unwrap_or(test_version_inc),
active_since: 1,
config: OutputConfig { rules: vec![] },
};

Ok(response.into())
}
111 changes: 111 additions & 0 deletions rs/boundary_node/rate_limits/canister/interface.did
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
type Version = nat64; // Represents the config version number
type Timestamp = nat64; // Represents timestamp in nanoseconds since the epoch (1970-01-01)
type RuleId = text; // Unique identifier for each rule
type SchemaVersion = nat64; // Version of the schema for encoding/decoding the rules
type IncidentId = text; // Unique identifier for each incident


// Input structure for defining a rule with mandatory fields within a config
type InputRule = record {
incident_id: IncidentId; // Unique identifier for the incident
rule_raw: blob; // Raw rule data (in binary format), expected to be a valid json object
description: text; // Textual description of the rule
};

// Output structure for rules
// Optional fields rule_raw and description may remain hidden while the rule is under confidentiality restrictions
type OutputRule = record {
id: RuleId; // Unique identifier for the rule
rule_raw: opt blob; // Raw rule data (in binary format), expected to be a valid json object, none if the rule is currently confidential
description: opt text; // Textual description of the rule, none if the rule is currently confidential
disclosed_at: opt Timestamp; // Timestamp when the rule was disclosed, none if the rule is still confidential
};

type OutputConfig = record {
rules: vec OutputRule;
};

// Response structure for returning the requested configuration and associated metadata
type OutputConfigResponse = record {
version: Version; // Version of the configuration
active_since: Timestamp; // Time when this configuration was added (became active)
config: OutputConfig; // Contains the list of rules
};

// Verbose details of an individual rule
// Optional rule_raw and description fields are for restricted publicly viewing access
type OutputRuleMetadata = record {
id: RuleId; // Unique identifier for the rule
rule_raw: opt blob; // Raw rule data (binary format), expected to be a valid json object, none if the rule is currently confidential
description: opt text; // Textual description of the rule, none if the rule is currently confidential
disclosed_at: opt Timestamp; // Timestamp when the rule was disclosed, none if the rule is still confidential
added_in_version: Version; // Version when the rule was added (became active)
removed_in_version: opt Version; // Version when the rule was deactivated (removed), none if the rule is still active
};

type GetRuleByIdResponse = variant {
Ok: OutputRuleMetadata;
Err: text;
};

type GetConfigResponse = variant {
Ok: OutputConfigResponse;
Err: text;
};

type DiscloseRulesResponse = variant {
Ok;
Err: text;
};

type OverwriteConfigResponse = variant {
Ok;
Err: text;
};

type DiscloseRulesByRuleIdsResponse = variant {
Ok;
Err: text;
};

type DiscloseRulesByIncidentIdResponse = variant {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you have a lot of

type XResponse = variant {
  Ok;
  Err: text;
};

maybe it'd make sense to consolidate those somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These types could be consolidate at some later point. But once consolidated the extensibility/flexibility is lost. Probably some Ok variant will contain messages, i haven't yet carefully thought through.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but couldn't that just be a new Response type? I didn't mean you can only respond with this smaller type. E.g

type EmptyResponse = variant {
  Ok;
  Err: text;
};

type ListResponse = variant {
  Ok: vec RuleId,
  Err: text;
};

service: {
  disclose: (DiscloseArg) -> EmptyResponse;
  list: (ListArg) -> ListResponse;
};

Ok;
Err: text;
};

type GetRulesByIncidentIdResponse = variant {
Ok: vec RuleId;
Err: text;
};

// Configuration containing a list of rules that replaces the current configuration
type InputConfig = record {
schema_version: SchemaVersion; // schema version used to serialized the rules
rules: vec InputRule;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be possible when specifying rules to also specify whether they are public or not? Or do you have to basically set the config, and then in another call disclose rules? I guess what I'm asking is "what does a normal user flow look like?"

Copy link
Contributor Author

@nikolay-komarevskiy nikolay-komarevskiy Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expected flow is:

  1. Each newly added rule is confidential by default
  2. To open the rule for public viewing a separate call to disclose_() function is needed
    We could indeed add an additional field policy with private/public, but it seems to be unnecessary. If needed it can be added.


// Initialization arguments for the service
type InitArg = record {
registry_polling_period_secs: nat64; // IDs of existing API boundary nodes are polled from the registry with this periodicity
};

service : (InitArg) -> {
// Replaces the current rate-limit configuration with a new set of rules
overwrite_config: (InputConfig) -> (OverwriteConfigResponse);

// Make the viewing of the specified rules publicly accessible
disclose_rules_by_rule_ids: (vec RuleId) -> (DiscloseRulesByRuleIdsResponse);

// Make the viewing of the specified rules related to an incident ID publicly accessible
disclose_rules_by_incident_id: (IncidentId) -> (DiscloseRulesByIncidentIdResponse);
Copy link
Contributor

@rikonor rikonor Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts on consolidating these two disclose methods? e.g

type DiscloseArg = variant {
  Rules: vec RuleId;
  Incidents: vec IncidentId;
};

disclose: (vec DiscloseArg) -> (DiscloseResponse);

and similarly for get_rules:

type ListArg = variant {
  Rules: vec RuleId;
  Incidents: vec IncidentId;
};

list: (vec ListArg) -> (ListResponse);

Copy link
Contributor Author

@nikolay-komarevskiy nikolay-komarevskiy Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the disclose one! but for get_rules it would be strange as for a RuleId it returns a single rule.


// Fetches the rate-limit rule configuration for a specified version
// If no version is provided, the latest configuration is returned
get_config: (opt Version) -> (GetConfigResponse) query;

// Fetch the rule with metadata by its ID
get_rule_by_id: (RuleId) -> (GetRuleByIdResponse) query;

// Fetch all rules IDs related to an ID of the incident
get_rules_by_incident_id: (IncidentId) -> (GetRulesByIncidentIdResponse) query;
}
31 changes: 31 additions & 0 deletions rs/boundary_node/rate_limits/canister/storage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use std::cell::RefCell;

use ic_stable_structures::{
memory_manager::{MemoryId, MemoryManager, VirtualMemory},
DefaultMemoryImpl, StableBTreeMap,
};

use crate::types::Version;

// Stable Memory
type Memory = VirtualMemory<DefaultMemoryImpl>;

type StableMap<K, V> = StableBTreeMap<K, V, Memory>;
type _StableSet<T> = StableMap<T, ()>;
type StableValue<T> = StableMap<(), T>;

const MEMORY_ID_VERSION: u8 = 0;

// Memory
thread_local! {
static MEMORY_MANAGER: RefCell<MemoryManager<DefaultMemoryImpl>> =
RefCell::new(MemoryManager::init(DefaultMemoryImpl::default()));
}

thread_local! {
pub static VERSION: RefCell<StableValue<Version>> = RefCell::new(
StableValue::init(
MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_VERSION))),
)
);
}
49 changes: 49 additions & 0 deletions rs/boundary_node/rate_limits/canister/types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
pub type Version = u64;
pub type Timestamp = u64;
pub type RuleId = String;

pub struct ConfigResponse {
pub version: Version,
pub active_since: Timestamp,
pub config: OutputConfig,
}

pub struct OutputConfig {
pub rules: Vec<OutputRule>,
}

pub struct OutputRule {
pub id: RuleId,
pub rule_raw: Option<Vec<u8>>,
pub description: Option<String>,
pub disclosed_at: Option<Timestamp>,
}

impl From<OutputRule> for rate_limits_api::OutputRule {
fn from(value: OutputRule) -> Self {
rate_limits_api::OutputRule {
description: value.description,
disclosed_at: value.disclosed_at,
id: value.id,
rule_raw: value.rule_raw,
}
}
}

impl From<OutputConfig> for rate_limits_api::OutputConfig {
fn from(value: OutputConfig) -> Self {
rate_limits_api::OutputConfig {
rules: value.rules.into_iter().map(|r| r.into()).collect(),
}
}
}

impl From<ConfigResponse> for rate_limits_api::ConfigResponse {
fn from(value: ConfigResponse) -> Self {
rate_limits_api::ConfigResponse {
version: value.version,
active_since: value.active_since,
config: value.config.into(),
}
}
}
13 changes: 13 additions & 0 deletions rs/boundary_node/rate_limits/canister_client/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "canister_client"
version.workspace = true
authors.workspace = true
edition.workspace = true
description.workspace = true
documentation.workspace = true

[dependencies]
rate-limits-api = { path = "../api" }
ic-agent = { workspace = true }
tokio = { workspace = true }
candid = { workspace = true }
33 changes: 33 additions & 0 deletions rs/boundary_node/rate_limits/canister_client/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use candid::{Decode, Encode, Principal};
use ic_agent::{identity::AnonymousIdentity, Agent};
use rate_limits_api::GetConfigResponse;

const RATE_LIMIT_CANISTER_ID: &str = "zwbmv-jyaaa-aaaab-qacaa-cai";
const IC_DOMAIN: &str = "https://ic0.app";

#[tokio::main]
async fn main() {
let agent = Agent::builder()
.with_url(IC_DOMAIN)
.with_identity(AnonymousIdentity {})
.build()
.expect("failed to build the agent");

agent.fetch_root_key().await.unwrap();

let canister_id = Principal::from_text(RATE_LIMIT_CANISTER_ID).unwrap();

// let args = Encode!(&Some(1u64)).unwrap();
let args = Encode!(&None::<u64>).unwrap();

let response = agent
.update(&canister_id, "get_config")
.with_arg(args)
.call_and_wait()
.await
.expect("update call failed");

let decoded = Decode!(&response, GetConfigResponse).expect("failed to decode candid response");

println!("get_config response: {decoded:#?}");
}
Loading
Loading