Skip to content

Commit 82d7b06

Browse files
authored
chore: [EXC-2089] Add logs in system API to track whether bitcoin methods are called on the management canister (#6088)
Calling the bitcoin API via the management canister is [deprecated](https://internetcomputer.org/docs/references/ic-interface-spec#ic-bitcoin-api) since a while ago and developers are expected to interact with the relevant bitcoin canister (mainnet or testnet) directly. This PR adds a log that would catch any cases where this is still happening in preparation of fully removing this code from the replica. Adding a log was preferred over adding some metric as we don't have access to a `MetricsRegistry` in the system API and propagating that would be a bunch more work. Instead, adding a log is fairly easier/less invasive (some info is still propagated to help diagnose but it's quite limited in scope).
1 parent 121e69a commit 82d7b06

File tree

2 files changed

+58
-0
lines changed

2 files changed

+58
-0
lines changed

rs/embedders/src/wasmtime_embedder/system_api/routing.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::str::FromStr;
44
use ic_base_types::{CanisterId, PrincipalId, SubnetId};
55
use ic_btc_interface::NetworkInRequest as BitcoinNetwork;
66
use ic_error_types::UserError;
7+
use ic_logger::{info, ReplicaLogger};
78
use ic_management_canister_types_private::{
89
BitcoinGetBalanceArgs, BitcoinGetBlockHeadersArgs, BitcoinGetCurrentFeePercentilesArgs,
910
BitcoinGetUtxosArgs, BitcoinSendTransactionArgs, CanisterIdRecord, CanisterInfoRequest,
@@ -57,6 +58,8 @@ pub(super) fn resolve_destination(
5758
method_name: &str,
5859
payload: &[u8],
5960
own_subnet: SubnetId,
61+
caller: CanisterId,
62+
logger: &ReplicaLogger,
6063
) -> Result<PrincipalId, ResolveDestinationError> {
6164
// Figure out the destination subnet based on the method and the payload.
6265
let method = Ic00Method::from_str(method_name);
@@ -131,6 +134,9 @@ pub(super) fn resolve_destination(
131134
args.network,
132135
network_topology,
133136
own_subnet,
137+
caller,
138+
method_name,
139+
logger,
134140
))
135141
}
136142
Ok(Ic00Method::BitcoinGetUtxos) => {
@@ -139,6 +145,9 @@ pub(super) fn resolve_destination(
139145
args.network,
140146
network_topology,
141147
own_subnet,
148+
caller,
149+
method_name,
150+
logger,
142151
))
143152
}
144153
Ok(Ic00Method::BitcoinGetBlockHeaders) => {
@@ -147,6 +156,9 @@ pub(super) fn resolve_destination(
147156
args.network,
148157
network_topology,
149158
own_subnet,
159+
caller,
160+
method_name,
161+
logger,
150162
))
151163
}
152164
Ok(Ic00Method::BitcoinSendTransaction) => {
@@ -156,6 +168,9 @@ pub(super) fn resolve_destination(
156168
args.network,
157169
network_topology,
158170
own_subnet,
171+
caller,
172+
method_name,
173+
logger,
159174
))
160175
}
161176
Ok(Ic00Method::BitcoinGetCurrentFeePercentiles) => {
@@ -164,6 +179,9 @@ pub(super) fn resolve_destination(
164179
args.network,
165180
network_topology,
166181
own_subnet,
182+
caller,
183+
method_name,
184+
logger,
167185
))
168186
}
169187
Ok(Ic00Method::NodeMetricsHistory) => {
@@ -431,7 +449,17 @@ fn route_bitcoin_message(
431449
network: BitcoinNetwork,
432450
network_topology: &NetworkTopology,
433451
own_subnet: SubnetId,
452+
caller: CanisterId,
453+
method_name: &str,
454+
logger: &ReplicaLogger,
434455
) -> PrincipalId {
456+
info!(
457+
logger,
458+
"Canister {} called Bitcoin method {} with network: {:?} directly on the management canister",
459+
caller,
460+
method_name,
461+
network
462+
);
435463
match network {
436464
// Route to the bitcoin canister if it exists, otherwise route to own subnet.
437465
// NOTE: Local deployments can run regtest mode for testing, and that routes to the
@@ -455,6 +483,7 @@ mod tests {
455483
use assert_matches::assert_matches;
456484
use candid::Encode;
457485
use ic_base_types::RegistryVersion;
486+
use ic_logger::no_op_logger;
458487
use ic_management_canister_types_private::{
459488
DerivationPath, EcdsaCurve, EcdsaKeyId, SchnorrAlgorithm, SchnorrKeyId, SignWithECDSAArgs,
460489
VetKdCurve, VetKdKeyId,
@@ -612,6 +641,7 @@ mod tests {
612641

613642
#[test]
614643
fn resolve_reshare_chain_key() {
644+
let logger = no_op_logger();
615645
for (network_topology, key_id) in [
616646
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
617647
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -623,6 +653,8 @@ mod tests {
623653
&Ic00Method::ReshareChainKey.to_string(),
624654
&reshare_chain_key_request(key_id.clone(), subnet_test_id(1)),
625655
subnet_test_id(2),
656+
canister_test_id(1),
657+
&logger,
626658
)
627659
.unwrap(),
628660
PrincipalId::new_subnet_test_id(1)
@@ -632,6 +664,7 @@ mod tests {
632664

633665
#[test]
634666
fn resolve_reshare_chain_key_key_not_held_error() {
667+
let logger = no_op_logger();
635668
for (network_topology, key_id) in [
636669
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
637670
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -643,6 +676,8 @@ mod tests {
643676
&Ic00Method::ReshareChainKey.to_string(),
644677
&reshare_chain_key_request(key_id.clone(), subnet_test_id(2)),
645678
subnet_test_id(2),
679+
canister_test_id(1),
680+
&logger,
646681
)
647682
.unwrap_err(),
648683
ResolveDestinationError::ChainKeyError(err) => assert_eq!(
@@ -659,6 +694,7 @@ mod tests {
659694

660695
#[test]
661696
fn resolve_reshare_chain_key_unknown_subnet_error() {
697+
let logger = no_op_logger();
662698
for (network_topology, key_id) in [
663699
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
664700
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -670,6 +706,8 @@ mod tests {
670706
&Ic00Method::ReshareChainKey.to_string(),
671707
&reshare_chain_key_request(key_id.clone(), subnet_test_id(3)),
672708
subnet_test_id(2),
709+
canister_test_id(1),
710+
&logger,
673711
)
674712
.unwrap_err(),
675713
ResolveDestinationError::ChainKeyError(err) => assert_eq!(
@@ -686,6 +724,7 @@ mod tests {
686724

687725
#[test]
688726
fn resolve_reshare_chain_key_wrong_subnet_error() {
727+
let logger = no_op_logger();
689728
for (network_topology, key_id) in [
690729
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
691730
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -698,6 +737,8 @@ mod tests {
698737
// Subnet 2 doesn't have the requested key.
699738
&reshare_chain_key_request(key_id.clone(), subnet_test_id(2)),
700739
subnet_test_id(2),
740+
canister_test_id(1),
741+
&logger,
701742
)
702743
.unwrap_err(),
703744
ResolveDestinationError::ChainKeyError(err) => assert_eq!(
@@ -714,6 +755,7 @@ mod tests {
714755

715756
#[test]
716757
fn resolve_reshare_chain_key_subnet_not_found_error() {
758+
let logger = no_op_logger();
717759
for (network_topology, key_id) in [
718760
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
719761
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -726,6 +768,8 @@ mod tests {
726768
// Subnet 3 doesn't exist
727769
&reshare_chain_key_request(key_id.clone(), subnet_test_id(3)),
728770
subnet_test_id(2),
771+
canister_test_id(1),
772+
&logger,
729773
)
730774
.unwrap_err(),
731775
ResolveDestinationError::ChainKeyError(err) => assert_eq!(
@@ -742,6 +786,7 @@ mod tests {
742786

743787
#[test]
744788
fn resolve_chain_key_request() {
789+
let logger = no_op_logger();
745790
for (network_topology, method, payload) in [
746791
(
747792
network_with_ecdsa_subnets(),
@@ -765,6 +810,8 @@ mod tests {
765810
&method.to_string(),
766811
&payload,
767812
subnet_test_id(1),
813+
canister_test_id(1),
814+
&logger,
768815
)
769816
.unwrap(),
770817
PrincipalId::new_subnet_test_id(0)
@@ -774,6 +821,7 @@ mod tests {
774821

775822
#[test]
776823
fn resolve_chain_key_request_error() {
824+
let logger = no_op_logger();
777825
for (method, payload, master_key_id) in [
778826
(
779827
Ic00Method::SignWithECDSA,
@@ -796,6 +844,8 @@ mod tests {
796844
&method.to_string(),
797845
&payload,
798846
subnet_test_id(1),
847+
canister_test_id(1),
848+
&logger,
799849
)
800850
.unwrap_err(),
801851
ResolveDestinationError::ChainKeyError(err) => assert_eq!(
@@ -811,6 +861,7 @@ mod tests {
811861

812862
#[test]
813863
fn resolve_chain_key_public_key_works_with_disabled_keys() {
864+
let logger = no_op_logger();
814865
for (network_topology, method, payload) in [
815866
(
816867
network_with_ecdsa_subnets(),
@@ -834,6 +885,8 @@ mod tests {
834885
&method.to_string(),
835886
&payload,
836887
subnet_test_id(1),
888+
canister_test_id(1),
889+
&logger,
837890
)
838891
.unwrap(),
839892
PrincipalId::new_subnet_test_id(0)
@@ -843,6 +896,7 @@ mod tests {
843896

844897
#[test]
845898
fn resolve_reshare_chain_key_works_with_disabled_keys() {
899+
let logger = no_op_logger();
846900
for (network_topology, key_id) in [
847901
(network_with_ecdsa_subnets(), ecdsa_master_key_id(1)),
848902
(network_with_schnorr_subnets(), schnorr_master_key_id(1)),
@@ -854,6 +908,8 @@ mod tests {
854908
&Ic00Method::ReshareChainKey.to_string(),
855909
&reshare_chain_key_request(key_id, subnet_test_id(0)),
856910
subnet_test_id(1),
911+
canister_test_id(1),
912+
&logger,
857913
)
858914
.unwrap(),
859915
PrincipalId::new_subnet_test_id(0)

rs/embedders/src/wasmtime_embedder/system_api/sandbox_safe_system_state.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,8 @@ impl SystemStateModifications {
400400
msg.method_name.as_str(),
401401
msg.method_payload.as_slice(),
402402
own_subnet_id,
403+
system_state.canister_id,
404+
logger,
403405
)
404406
.map(CanisterId::unchecked_from_principal)
405407
{

0 commit comments

Comments
 (0)