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
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