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: [UTP-242] Make subnet replica version available to canisters via management API #2202

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion rs/execution_environment/src/canister_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,8 @@ impl CanisterManager {
| Ok(Ic00Method::BitcoinSendTransaction)
| Ok(Ic00Method::BitcoinSendTransactionInternal)
| Ok(Ic00Method::BitcoinGetCurrentFeePercentiles)
| Ok(Ic00Method::NodeMetricsHistory) => Err(UserError::new(
| Ok(Ic00Method::NodeMetricsHistory)
| Ok(Ic00Method::SubnetMetrics) => Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!("Only canisters can call ic00 method {}", method_name),
)),
Expand Down
36 changes: 33 additions & 3 deletions rs/execution_environment/src/execution_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ use ic_management_canister_types::{
LoadCanisterSnapshotArgs, MasterPublicKeyId, Method as Ic00Method, NodeMetricsHistoryArgs,
Payload as Ic00Payload, ProvisionalCreateCanisterWithCyclesArgs, ProvisionalTopUpCanisterArgs,
SchnorrPublicKeyArgs, SchnorrPublicKeyResponse, SetupInitialDKGArgs, SignWithECDSAArgs,
SignWithSchnorrArgs, StoredChunksArgs, TakeCanisterSnapshotArgs, UninstallCodeArgs,
UpdateSettingsArgs, UploadChunkArgs, IC_00,
SignWithSchnorrArgs, StoredChunksArgs, SubnetMetricsArgs, SubnetMetricsResponse,
TakeCanisterSnapshotArgs, UninstallCodeArgs, UpdateSettingsArgs, UploadChunkArgs, IC_00,
};
use ic_metrics::MetricsRegistry;
use ic_registry_provisional_whitelist::ProvisionalWhitelist;
Expand Down Expand Up @@ -472,7 +472,7 @@ impl ExecutionEnvironment {
instruction_limits: InstructionLimits,
rng: &mut dyn RngCore,
idkg_subnet_public_keys: &BTreeMap<MasterPublicKeyId, MasterPublicKey>,
_replica_version: &ReplicaVersion,
replica_version: &ReplicaVersion,
registry_settings: &RegistryExecutionSettings,
round_limits: &mut RoundLimits,
) -> (ReplicatedState, Option<NumInstructions>) {
Expand Down Expand Up @@ -1379,6 +1379,15 @@ impl ExecutionEnvironment {
}
}

Ok(Ic00Method::SubnetMetrics) => {
let res = SubnetMetricsArgs::decode(payload)
.and_then(|args| self.subnet_metrics(replica_version, args));
ExecuteSubnetMessageResult::Finished {
response: res,
refund: msg.take_cycles(),
}
}

Ok(Ic00Method::FetchCanisterLogs) => ExecuteSubnetMessageResult::Finished {
response: Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
Expand Down Expand Up @@ -2226,6 +2235,27 @@ impl ExecutionEnvironment {
Ok(Encode!(&result).unwrap())
}

fn subnet_metrics(
&self,
replica_version: &ReplicaVersion,
args: SubnetMetricsArgs,
) -> Result<Vec<u8>, UserError> {
// TODO: Check taken from node_metric_history; but is this actually needed? Can such a call be routed wrong?
michael-weigelt marked this conversation as resolved.
Show resolved Hide resolved
if args.subnet_id != self.own_subnet_id.get() {
return Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"Provided target subnet ID {} does not match current subnet ID {}.",
args.subnet_id, self.own_subnet_id
),
));
}
let res = SubnetMetricsResponse {
replica_version: replica_version.to_string(),
};
Ok(Encode!(&res).unwrap())
}

// Executes an inter-canister response.
//
// Returns a tuple with the result, along with a flag indicating whether or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ impl ExecutionEnvironmentMetrics {
| ic00::Method::BitcoinSendTransaction
| ic00::Method::BitcoinGetCurrentFeePercentiles
| ic00::Method::NodeMetricsHistory
| ic00::Method::SubnetMetrics
| ic00::Method::FetchCanisterLogs
| ic00::Method::ProvisionalCreateCanisterWithCycles
| ic00::Method::ProvisionalTopUpCanister
Expand Down
5 changes: 5 additions & 0 deletions rs/execution_environment/src/ic00_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ impl Ic00MethodPermissions {
allow_remote_subnet_sender: true,
allow_only_nns_subnet_sender: false,
},
Ic00Method::SubnetMetrics => Self {
method,
allow_remote_subnet_sender: true,
allow_only_nns_subnet_sender: false,
},
Ic00Method::FetchCanisterLogs => Self {
method,
// `FetchCanisterLogs` method is only allowed for messages sent by users,
Expand Down
2 changes: 2 additions & 0 deletions rs/execution_environment/src/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2413,6 +2413,7 @@ fn can_execute_subnet_msg(
| Ic00Method::BitcoinSendTransactionInternal
| Ic00Method::BitcoinGetSuccessors
| Ic00Method::NodeMetricsHistory
| Ic00Method::SubnetMetrics
| Ic00Method::FetchCanisterLogs
| Ic00Method::ProvisionalCreateCanisterWithCycles
| Ic00Method::ProvisionalTopUpCanister
Expand Down Expand Up @@ -2476,6 +2477,7 @@ fn get_instructions_limits_for_subnet_message(
| BitcoinGetCurrentFeePercentiles
| BitcoinGetSuccessors
| NodeMetricsHistory
| SubnetMetrics
| FetchCanisterLogs
| ProvisionalCreateCanisterWithCycles
| ProvisionalTopUpCanister
Expand Down
1 change: 1 addition & 0 deletions rs/execution_environment/tests/dts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,6 +1165,7 @@ fn dts_aborted_execution_does_not_block_subnet_messages() {
| Method::BitcoinSendTransactionInternal
| Method::BitcoinGetSuccessors
| Method::NodeMetricsHistory
| Method::SubnetMetrics
| Method::ProvisionalCreateCanisterWithCycles
| Method::ProvisionalTopUpCanister => {}
// Unsupported methods accepting just one argument.
Expand Down
5 changes: 3 additions & 2 deletions rs/system_api/src/routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use ic_management_canister_types::{
ECDSAPublicKeyArgs, InstallChunkedCodeArgs, InstallCodeArgsV2, ListCanisterSnapshotArgs,
LoadCanisterSnapshotArgs, MasterPublicKeyId, Method as Ic00Method, NodeMetricsHistoryArgs,
Payload, ProvisionalTopUpCanisterArgs, SchnorrPublicKeyArgs, SignWithECDSAArgs,
SignWithSchnorrArgs, StoredChunksArgs, TakeCanisterSnapshotArgs, UninstallCodeArgs,
UpdateSettingsArgs, UploadChunkArgs,
SignWithSchnorrArgs, StoredChunksArgs, SubnetMetricsArgs, TakeCanisterSnapshotArgs,
UninstallCodeArgs, UpdateSettingsArgs, UploadChunkArgs,
};
use ic_replicated_state::NetworkTopology;
use itertools::Itertools;
Expand Down Expand Up @@ -168,6 +168,7 @@ pub(super) fn resolve_destination(
Ok(Ic00Method::NodeMetricsHistory) => {
Ok(NodeMetricsHistoryArgs::decode(payload)?.subnet_id)
}
Ok(Ic00Method::SubnetMetrics) => Ok(SubnetMetricsArgs::decode(payload)?.subnet_id),
Ok(Ic00Method::FetchCanisterLogs) => {
Err(ResolveDestinationError::UserError(UserError::new(
ic_error_types::ErrorCode::CanisterRejectedMessage,
Expand Down
1 change: 1 addition & 0 deletions rs/system_api/src/sandbox_safe_system_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ impl SystemStateChanges {
| Ok(Ic00Method::BitcoinSendTransaction)
| Ok(Ic00Method::BitcoinGetCurrentFeePercentiles)
| Ok(Ic00Method::NodeMetricsHistory)
| Ok(Ic00Method::SubnetMetrics)
| Ok(Ic00Method::FetchCanisterLogs)
| Ok(Ic00Method::UploadChunk)
| Ok(Ic00Method::StoredChunks)
Expand Down
12 changes: 12 additions & 0 deletions rs/tests/execution/general_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ use general_execution_tests::api_tests::node_metrics_history_ingress_update_fail
use general_execution_tests::api_tests::node_metrics_history_non_existing_subnet_fails;
use general_execution_tests::api_tests::node_metrics_history_query_fails;
use general_execution_tests::api_tests::node_metrics_history_update_succeeds;
use general_execution_tests::api_tests::subnet_metrics_another_subnet_succeeds;
use general_execution_tests::api_tests::subnet_metrics_ingress_query_fails;
use general_execution_tests::api_tests::subnet_metrics_ingress_update_fails;
use general_execution_tests::api_tests::subnet_metrics_non_existing_subnet_fails;
use general_execution_tests::api_tests::subnet_metrics_query_fails;
use general_execution_tests::api_tests::subnet_metrics_update_succeeds;
use general_execution_tests::api_tests::test_controller;
use general_execution_tests::api_tests::test_cycles_burn;
use general_execution_tests::api_tests::test_in_replicated_execution;
Expand Down Expand Up @@ -45,6 +51,12 @@ fn main() -> Result<()> {
.add_test(systest!(node_metrics_history_non_existing_subnet_fails))
.add_test(systest!(node_metrics_history_ingress_update_fails))
.add_test(systest!(node_metrics_history_ingress_query_fails))
.add_test(systest!(subnet_metrics_update_succeeds))
.add_test(systest!(subnet_metrics_query_fails))
.add_test(systest!(subnet_metrics_another_subnet_succeeds))
.add_test(systest!(subnet_metrics_non_existing_subnet_fails))
.add_test(systest!(subnet_metrics_ingress_update_fails))
.add_test(systest!(subnet_metrics_ingress_query_fails))
.add_test(systest!(can_access_big_heap_and_big_stable_memory))
.add_test(systest!(can_access_big_stable_memory))
.add_test(systest!(can_handle_overflows_when_indexing_stable_memory))
Expand Down
157 changes: 157 additions & 0 deletions rs/tests/execution/general_execution_tests/api_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,3 +408,160 @@ pub fn node_metrics_history_ingress_query_fails(env: TestEnv) {
}
})
}

pub fn subnet_metrics_update_succeeds(env: TestEnv) {
// Arrange.
let (app_node, agent) = setup_app_node_and_agent(&env);
let logger = env.logger();
let subnet_id = app_node.subnet_id().unwrap().get();
block_on({
async move {
let canister = UniversalCanister::new_with_retries(
&agent,
app_node.effective_canister_id(),
&logger,
)
.await;
// Act.
let result = canister
.update(wasm().call_simple(
ic00::IC_00,
Method::SubnetMetrics,
call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()),
))
.await;
// Assert.
assert!(result.is_ok());
assert!(!result.ok().unwrap().is_empty()); // Assert it has some non zero data.
}
})
}

pub fn subnet_metrics_query_fails(env: TestEnv) {
// Arrange.
let (app_node, agent) = setup_app_node_and_agent(&env);
let logger = env.logger();
let subnet_id = app_node.subnet_id().unwrap().get();
block_on({
async move {
let canister = UniversalCanister::new_with_retries(
&agent,
app_node.effective_canister_id(),
&logger,
)
.await;
// Act.
let result = canister
.query(wasm().call_simple(
ic00::IC_00,
Method::SubnetMetrics,
call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()),
))
.await;
// Assert.
assert_reject_msg(
result,
RejectCode::CanisterError,
"cannot be executed in non replicated query mode",
);
}
})
}

pub fn subnet_metrics_another_subnet_succeeds(env: TestEnv) {
// Arrange.
let (app_node_1, agent_1) = setup_app_node_and_agent(&env);
// Create another subnet and use its id in the request.
let (app_node_2, _agent_2) = setup_app_node_and_agent(&env);
let logger = env.logger();
let subnet_id = app_node_2.subnet_id().unwrap().get();
block_on({
async move {
let canister = UniversalCanister::new_with_retries(
&agent_1,
app_node_1.effective_canister_id(),
&logger,
)
.await;
// Act.
let result = canister
.update(wasm().call_simple(
ic00::IC_00,
Method::SubnetMetrics,
call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()),
))
.await;
// Assert.
assert!(result.is_ok());
assert!(!result.ok().unwrap().is_empty()); // Assert it has some non zero data.
}
})
}

pub fn subnet_metrics_non_existing_subnet_fails(env: TestEnv) {
// Arrange.
let (app_node, agent) = setup_app_node_and_agent(&env);
let logger = env.logger();
// Create non existing subnet id.
let subnet_id = PrincipalId::new_subnet_test_id(1);
block_on({
async move {
let canister = UniversalCanister::new_with_retries(
&agent,
app_node.effective_canister_id(),
&logger,
)
.await;
// Act.
let result = canister
.update(wasm().call_simple(
ic00::IC_00,
Method::SubnetMetrics,
call_args().other_side(ic00::SubnetMetricsArgs { subnet_id }.encode()),
))
.await;
// Assert.
assert_reject(result, RejectCode::CanisterReject);
}
})
}

pub fn subnet_metrics_ingress_update_fails(env: TestEnv) {
// Arrange.
let (app_node, agent) = setup_app_node_and_agent(&env);
let subnet_id = app_node.subnet_id().unwrap().get();
block_on({
async move {
// Act.
let result = agent
.update(&Principal::management_canister(), "subnet_metrics")
.with_arg(ic00::SubnetMetricsArgs { subnet_id }.encode())
.call_and_wait()
.await;
// Assert.
assert_reject_msg(
result,
RejectCode::CanisterReject,
"ic00 method subnet_metrics can not be called via ingress messages",
);
}
})
}

pub fn subnet_metrics_ingress_query_fails(env: TestEnv) {
// Arrange.
let (app_node, agent) = setup_app_node_and_agent(&env);
let subnet_id = app_node.subnet_id().unwrap().get();
block_on({
async move {
// Act.
let result = agent
.query(&Principal::management_canister(), "subnet_metrics")
.with_arg(ic00::SubnetMetricsArgs { subnet_id }.encode())
.call()
.await;
// Assert.
assert_eq!(result, Err(AgentError::CertificateNotAuthorized()));
}
})
}
28 changes: 28 additions & 0 deletions rs/types/management_canister_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ pub enum Method {
BitcoinSendTransactionInternal, // API for sending transactions to the network.
BitcoinGetSuccessors, // API for fetching blocks from the network.

// Subnet information
NodeMetricsHistory,
SubnetMetrics,

FetchCanisterLogs,

Expand Down Expand Up @@ -2586,6 +2588,32 @@ pub enum QueryMethod {
FetchCanisterLogs,
}

/// `CandidType` for `SubnetMetricsArgs`
/// ```text
/// record {
/// subnet_id: principal;
/// }
/// ```
#[derive(Clone, Debug, Default, CandidType, Deserialize)]
pub struct SubnetMetricsArgs {
pub subnet_id: PrincipalId,
}

impl Payload<'_> for SubnetMetricsArgs {}

/// `CandidType` for `SubnetMetricsResponse`
/// ```text
/// record {
/// replica_version: text;
/// }
/// ```
#[derive(Clone, Debug, Default, CandidType, Deserialize)]
pub struct SubnetMetricsResponse {
pub replica_version: String,
}

impl Payload<'_> for SubnetMetricsResponse {}

/// `CandidType` for `NodeMetricsHistoryArgs`
/// ```text
/// record {
Expand Down
1 change: 1 addition & 0 deletions rs/types/types/src/messages/ingress_messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ pub fn extract_effective_canister_id(
| Ok(Method::BitcoinGetSuccessors)
| Ok(Method::BitcoinGetCurrentFeePercentiles)
| Ok(Method::NodeMetricsHistory)
| Ok(Method::SubnetMetrics)
| Ok(Method::FetchCanisterLogs) => {
// Subnet method not allowed for ingress.
Err(ParseIngressError::SubnetMethodNotAllowed)
Expand Down
3 changes: 2 additions & 1 deletion rs/types/types/src/messages/inter_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ impl Request {
| Ok(Method::BitcoinSendTransactionInternal)
| Ok(Method::BitcoinGetSuccessors)
| Ok(Method::BitcoinGetCurrentFeePercentiles)
| Ok(Method::NodeMetricsHistory) => {
| Ok(Method::NodeMetricsHistory)
| Ok(Method::SubnetMetrics) => {
// No effective canister id.
None
}
Expand Down
Loading