Skip to content

Commit

Permalink
refactor: Remove dependencies on nns governance crate from sns cli an…
Browse files Browse the repository at this point in the history
…d ic-admin (#1252)

Continuing to remove dependencies on nns governance crate to the end of
making it have no bazel visibility as a dependency.

---------

Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>
  • Loading branch information
max-dfinity and IDX GitHub Automation authored Sep 17, 2024
1 parent 4cede51 commit 41f6ce3
Show file tree
Hide file tree
Showing 34 changed files with 707 additions and 668 deletions.
10 changes: 8 additions & 2 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ members = [
"rs/nervous_system/common/test_canister",
"rs/nervous_system/common/test_keys",
"rs/nervous_system/common/test_utils",
"rs/nervous_system/common/validation",
"rs/nervous_system/humanize",
"rs/nervous_system/integration_tests",
"rs/nervous_system/lock",
Expand Down
75 changes: 4 additions & 71 deletions rs/nervous_system/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use dfn_core::api::time_nanos;
use ic_base_types::CanisterId;
use ic_canister_log::{export, GlobalBuffer, LogBuffer, LogEntry};
use ic_canisters_http_types::{HttpRequest, HttpResponse, HttpResponseBuilder};
use ic_ledger_core::tokens::{CheckedAdd, CheckedSub};
use ic_ledger_core::Tokens;
use ic_ledger_core::{
tokens::{CheckedAdd, CheckedSub},
Tokens,
};
use lazy_static::lazy_static;
use maplit::hashmap;
use num_traits::ops::inv::Inv;
Expand Down Expand Up @@ -720,75 +722,6 @@ pub fn serve_metrics(
}
}

/// Verifies that the url is within the allowed length, and begins with
/// `http://` or `https://`. In addition, it will return an error in case of a
/// possibly "dangerous" condition, such as the url containing a username or
/// password, or having a port, or not having a domain name.
pub fn validate_proposal_url(
url: &str,
min_length: usize,
max_length: usize,
field_name: &str,
allowed_domains: Option<Vec<&str>>,
) -> Result<(), String> {
// // Check that the URL is a sensible length
if url.len() > max_length {
return Err(format!(
"{field_name} must be less than {max_length} characters long, but it is {} characters long. (Field was set to `{url}`.)",
url.len(),
));
}
if url.len() < min_length {
return Err(format!(
"{field_name} must be greater or equal to than {min_length} characters long, but it is {} characters long. (Field was set to `{url}`.)",
url.len(),
));
}

//

if !url.starts_with("https://") {
return Err(format!(
"{field_name} must begin with https://. (Field was set to `{url}`.)",
));
}

let parts_url: Vec<&str> = url.split("://").collect();
if parts_url.len() > 2 {
return Err(format!(
"{field_name} contains an invalid sequence of characters"
));
}

if parts_url.len() < 2 {
return Err(format!("{field_name} is missing content after protocol."));
}

if url.contains('@') {
return Err(format!(
"{field_name} cannot contain authentication information"
));
}

let parts_past_protocol = parts_url[1].split_once('/');

let (domain, _path) = match parts_past_protocol {
Some((domain, path)) => (domain, Some(path)),
None => (parts_url[1], None),
};

match allowed_domains {
Some(allowed) => match allowed.iter().any(|allowed| domain == *allowed) {
true => Ok(()),
false => Err(format!(
"{field_name} was not in the list of allowed domains: {:?}",
allowed
)),
},
None => Ok(()),
}
}

/// Returns the total amount of memory (heap, stable memory, etc) that the calling canister has allocated.
#[cfg(target_arch = "wasm32")]
pub fn total_memory_size_bytes() -> usize {
Expand Down
13 changes: 13 additions & 0 deletions rs/nervous_system/common/validation/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
load("@rules_rust//rust:defs.bzl", "rust_library")

package(default_visibility = ["//visibility:public"])

rust_library(
name = "validation",
srcs = glob(["src/**"]),
crate_name = "ic_nervous_system_common_validation",
version = "0.9.0",
deps = [
# Keep sorted.
],
)
12 changes: 12 additions & 0 deletions rs/nervous_system/common/validation/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "ic-nervous-system-common-validation"
version.workspace = true
authors.workspace = true
edition.workspace = true
description.workspace = true
documentation.workspace = true

[lib]
path = "src/lib.rs"

[dependencies]
68 changes: 68 additions & 0 deletions rs/nervous_system/common/validation/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/// Verifies that the url is within the allowed length, and begins with
/// `http://` or `https://`. In addition, it will return an error in case of a
/// possibly "dangerous" condition, such as the url containing a username or
/// password, or having a port, or not having a domain name.
pub fn validate_proposal_url(
url: &str,
min_length: usize,
max_length: usize,
field_name: &str,
allowed_domains: Option<Vec<&str>>,
) -> Result<(), String> {
// // Check that the URL is a sensible length
if url.len() > max_length {
return Err(format!(
"{field_name} must be less than {max_length} characters long, but it is {} characters long. (Field was set to `{url}`.)",
url.len(),
));
}
if url.len() < min_length {
return Err(format!(
"{field_name} must be greater or equal to than {min_length} characters long, but it is {} characters long. (Field was set to `{url}`.)",
url.len(),
));
}

//

if !url.starts_with("https://") {
return Err(format!(
"{field_name} must begin with https://. (Field was set to `{url}`.)",
));
}

let parts_url: Vec<&str> = url.split("://").collect();
if parts_url.len() > 2 {
return Err(format!(
"{field_name} contains an invalid sequence of characters"
));
}

if parts_url.len() < 2 {
return Err(format!("{field_name} is missing content after protocol."));
}

if url.contains('@') {
return Err(format!(
"{field_name} cannot contain authentication information"
));
}

let parts_past_protocol = parts_url[1].split_once('/');

let (domain, _path) = match parts_past_protocol {
Some((domain, path)) => (domain, Some(path)),
None => (parts_url[1], None),
};

match allowed_domains {
Some(allowed) => match allowed.iter().any(|allowed| domain == *allowed) {
true => Ok(()),
false => Err(format!(
"{field_name} was not in the list of allowed domains: {:?}",
allowed
)),
},
None => Ok(()),
}
}
1 change: 0 additions & 1 deletion rs/nervous_system/integration_tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ BASE_DEPENDENCIES = [
# Currently unused.
# DEPENDENCIES = BASE_DEPENDENCIES + [
# "//rs/sns/init",
# "//rs/nns/governance",
# "//rs/nns/sns-wasm",
# "//rs/nns/handlers/root/impl:root",
# "//rs/sns/swap",
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ BASE_DEPENDENCIES = [
# Keep sorted.
"//rs/crypto/sha2",
"//rs/nervous_system/clients",
"//rs/nervous_system/common/validation",
"//rs/nervous_system/proto",
"//rs/nns/common",
"//rs/protobuf",
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ comparable = { version = "0.5", features = ["derive"] }
ic-base-types = { path = "../../../types/base_types" }
ic-crypto-sha2 = { path = "../../../crypto/sha2" }
ic-nervous-system-clients = { path = "../../../nervous_system/clients" }
ic-nervous-system-common-validation = { path = "../../../nervous_system/common/validation" }
ic-nervous-system-proto = { path = "../../../nervous_system/proto" }
ic-nns-common = { path = "../../common" }
ic-protobuf = { path = "../../../protobuf" }
Expand Down
3 changes: 2 additions & 1 deletion rs/nns/governance/api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod bitcoin;
pub mod pb;
pub mod proposal_helpers;
pub mod proposal_submission_helpers;
pub mod proposal_validation;
pub mod subnet_rental;
pub mod test_api;
80 changes: 80 additions & 0 deletions rs/nns/governance/api/src/proposal_validation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use crate::pb::v1::Proposal;

// The limits on NNS proposal title len (in bytes).
const PROPOSAL_TITLE_BYTES_MIN: usize = 5;
const PROPOSAL_TITLE_BYTES_MAX: usize = 256;
// Proposal validation
// 30000 B
const PROPOSAL_SUMMARY_BYTES_MAX: usize = 30000;
// 2048 characters
const PROPOSAL_URL_CHAR_MAX: usize = 2048;
// 10 characters
const PROPOSAL_URL_CHAR_MIN: usize = 10;

/// Validates the user submitted proposal fields.
pub fn validate_user_submitted_proposal_fields(proposal: &Proposal) -> Result<(), String> {
validate_proposal_title(&proposal.title)?;
validate_proposal_summary(&proposal.summary)?;
validate_proposal_url(&proposal.url)?;

Ok(())
}

/// Returns whether the following requirements are met:
/// 1. proposal must have a title.
/// 2. title len (bytes, not characters) is between min and max.
pub fn validate_proposal_title(title: &Option<String>) -> Result<(), String> {
// Require that proposal has a title.
let len = title.as_ref().ok_or("Proposal lacks a title")?.len();

// Require that title is not too short.
if len < PROPOSAL_TITLE_BYTES_MIN {
return Err(format!(
"Proposal title is too short (must be at least {} bytes)",
PROPOSAL_TITLE_BYTES_MIN,
));
}

// Require that title is not too long.
if len > PROPOSAL_TITLE_BYTES_MAX {
return Err(format!(
"Proposal title is too long (can be at most {} bytes)",
PROPOSAL_TITLE_BYTES_MAX,
));
}

Ok(())
}

/// Returns whether the following requirements are met:
/// 1. summary len (bytes, not characters) is below the max.
pub fn validate_proposal_summary(summary: &str) -> Result<(), String> {
if summary.len() > PROPOSAL_SUMMARY_BYTES_MAX {
return Err(format!(
"The maximum proposal summary size is {} bytes, this proposal is: {} bytes",
PROPOSAL_SUMMARY_BYTES_MAX,
summary.len(),
));
}

Ok(())
}

/// Returns whether the following requirements are met:
/// 1. If a url is provided, it is between the max and min
/// 2. If a url is specified, it must be from the list of allowed domains.
pub fn validate_proposal_url(url: &str) -> Result<(), String> {
// An empty string will fail validation as it is not a valid url,
// but it's fine for us.
if !url.is_empty() {
ic_nervous_system_common_validation::validate_proposal_url(
url,
PROPOSAL_URL_CHAR_MIN,
PROPOSAL_URL_CHAR_MAX,
"Proposal url",
Some(vec!["forum.dfinity.org"]),
)?
}

Ok(())
}
Loading

0 comments on commit 41f6ce3

Please sign in to comment.