Skip to content

Commit

Permalink
Chore: Make fields in RequestMetadata non-optional.
Browse files Browse the repository at this point in the history
  • Loading branch information
stiegerc committed Jan 10, 2024
1 parent eee4085 commit 46595ae
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 78 deletions.
4 changes: 2 additions & 2 deletions rs/canonical_state/src/encoding/tests/compatibility.rs
Expand Up @@ -323,8 +323,8 @@ fn canonical_encoding_request_v14_plus() {
.method_name("test".to_string())
.method_payload(vec![6])
.metadata(Some(RequestMetadata {
call_tree_depth: Some(13),
call_tree_start_time: Some(Time::from_nanos_since_unix_epoch(101)),
call_tree_depth: 13,
call_tree_start_time: Time::from_nanos_since_unix_epoch(101),
}))
.build()
.into();
Expand Down
20 changes: 0 additions & 20 deletions rs/canonical_state/src/encoding/tests/encoding.rs
@@ -1,7 +1,5 @@
use super::test_fixtures::*;
use crate::{all_supported_versions, encoding::*};
use ic_test_utilities::types::messages::RequestBuilder;
use ic_types::messages::RequestMetadata;

#[test]
fn roundtrip_encoding_stream_header() {
Expand All @@ -26,24 +24,6 @@ fn roundtrip_encoding_request() {
}
}

#[test]
fn request_missing_metadata_and_missing_metadata_fields_encode_the_same() {
for certification_version in all_supported_versions() {
let request1: RequestOrResponse = RequestBuilder::new().metadata(None).build().into();
let request2: RequestOrResponse = RequestBuilder::new()
.metadata(Some(RequestMetadata {
call_tree_depth: None,
call_tree_start_time: None,
}))
.build()
.into();
assert_eq!(
encode_message(&request1, certification_version),
encode_message(&request2, certification_version),
);
}
}

#[test]
fn roundtrip_encoding_response() {
let response = response();
Expand Down
4 changes: 2 additions & 2 deletions rs/canonical_state/src/encoding/tests/test_fixtures.rs
Expand Up @@ -34,8 +34,8 @@ pub fn request(certification_version: CertificationVersion) -> RequestOrResponse
.method_payload(vec![6])
.metadata(
(certification_version >= CertificationVersion::V14).then_some(RequestMetadata {
call_tree_depth: Some(1),
call_tree_start_time: Some(Time::from_nanos_since_unix_epoch(100_000)),
call_tree_depth: 1,
call_tree_start_time: Time::from_nanos_since_unix_epoch(100_000),
}),
)
.build()
Expand Down
24 changes: 9 additions & 15 deletions rs/canonical_state/src/encoding/types.rs
Expand Up @@ -260,8 +260,8 @@ impl TryFrom<RequestOrResponse> for ic_types::messages::RequestOrResponse {
impl From<&ic_types::messages::RequestMetadata> for RequestMetadata {
fn from(metadata: &ic_types::messages::RequestMetadata) -> Self {
RequestMetadata {
call_tree_depth: metadata.call_tree_depth,
call_tree_start_time: metadata.call_tree_start_time,
call_tree_depth: Some(metadata.call_tree_depth),
call_tree_start_time: Some(metadata.call_tree_start_time),
call_subtree_deadline: None,
}
}
Expand All @@ -275,16 +275,6 @@ impl From<(&ic_types::messages::Request, CertificationVersion)> for Request {
cycles: (&request.payment, certification_version).into(),
icp: 0,
};
let metadata = match request.metadata.as_ref() {
Some(ic_types::messages::RequestMetadata {
call_tree_depth: None,
call_tree_start_time: None,
})
| None => None,
Some(metadata) => {
(certification_version >= CertificationVersion::V14).then_some(metadata.into())
}
};

Self {
receiver: request.receiver.get().to_vec(),
Expand All @@ -294,16 +284,20 @@ impl From<(&ic_types::messages::Request, CertificationVersion)> for Request {
method_name: request.method_name.clone(),
method_payload: request.method_payload.clone(),
cycles_payment: None,
metadata,
metadata: request.metadata.as_ref().and_then(|metadata| {
(certification_version >= CertificationVersion::V14).then_some(metadata.into())
}),
}
}
}

impl From<RequestMetadata> for ic_types::messages::RequestMetadata {
fn from(metadata: RequestMetadata) -> Self {
ic_types::messages::RequestMetadata {
call_tree_depth: metadata.call_tree_depth,
call_tree_start_time: metadata.call_tree_start_time,
call_tree_depth: metadata.call_tree_depth.unwrap_or(0),
call_tree_start_time: metadata
.call_tree_start_time
.unwrap_or(Time::from_nanos_since_unix_epoch(0)),
}
}
}
Expand Down
17 changes: 7 additions & 10 deletions rs/state_manager/src/tree_hash.rs
Expand Up @@ -209,14 +209,11 @@ mod tests {
stream.push(
RequestBuilder::new()
.metadata(
(certification_version >= CertificationVersion::V14).then_some(
RequestMetadata {
call_tree_depth: (i % 2 > 0).then_some(i % 3),
call_tree_start_time: (i % 3 > 0)
.then_some(i % 2)
.map(Time::from_nanos_since_unix_epoch),
},
),
(certification_version >= CertificationVersion::V14 && i % 5 != 0)
.then_some(RequestMetadata {
call_tree_depth: i % 3,
call_tree_start_time: Time::from_nanos_since_unix_epoch(i % 2),
}),
)
.build()
.into(),
Expand Down Expand Up @@ -336,8 +333,8 @@ mod tests {
"1ED37E00D177681A4111B6D45F518DF3E414B0B614333BB6552EBC0D8492B687",
"62B2E77DFCD17C7E0CE3E762FD37281776C4B0A38CE1B83A1316614C3F849E39",
"80D4B528CC9E09C775273994261DD544D45EFFF90B655D90FC3A6E3F633ED718",
"C8E5C0C21DA665B94DD78C3CCA682A44780058E9D92BC120E7CF0DD2952CE824",
"E2DA77E9435CB7C1F0E598DC3B3C41D305B216AF3E806E350BED1CC16E56817F",
"970BC5155AEB4B4F81E470CBF6748EFA7D8805B936998A54AE70B7DD21F5DDCC",
"EA3B53B72150E3982CB0E6773F86634685EE7B153DCFE10D86D9927778409D97",
];

for certification_version in CertificationVersion::iter() {
Expand Down
17 changes: 4 additions & 13 deletions rs/types/types/src/messages.rs
Expand Up @@ -680,23 +680,14 @@ mod tests {
assert_matches!(signed_ingress1, Ok(signed_ingress1) if signed_ingress == signed_ingress1);
}

fn make_request_metadata(
call_tree_depth: Option<u64>,
call_tree_start_time: Option<u64>,
) -> Option<RequestMetadata> {
Some(RequestMetadata {
call_tree_depth,
call_tree_start_time: call_tree_start_time.map(Time::from_nanos_since_unix_epoch),
})
}

#[test]
fn serialize_request_via_bincode() {
for metadata in [
None,
make_request_metadata(None, None),
make_request_metadata(Some(13), None),
make_request_metadata(None, Some(17)),
Some(RequestMetadata {
call_tree_depth: 13,
call_tree_start_time: Time::from_nanos_since_unix_epoch(17),
}),
] {
let request = Request {
receiver: CanisterId::from(13),
Expand Down
22 changes: 10 additions & 12 deletions rs/types/types/src/messages/inter_canister.rs
Expand Up @@ -40,11 +40,11 @@ pub type CallContextId = Id<CallContextIdTag, u64>;
#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct RequestMetadata {
/// Indicates how many steps down the call tree a request is, starting at 0.
pub call_tree_depth: Option<u64>,
pub call_tree_depth: u64,
/// The block time (on the respective subnet) at the start of the call at the
/// root of the call tree that this request is part of. This is used for metrics
/// only.
pub call_tree_start_time: Option<Time>,
pub call_tree_start_time: Time,
}

/// Canister-to-canister request message.
Expand Down Expand Up @@ -439,10 +439,10 @@ impl From<Response> for RequestOrResponse {
impl From<&RequestMetadata> for pb_queues::RequestMetadata {
fn from(metadata: &RequestMetadata) -> Self {
Self {
call_tree_depth: metadata.call_tree_depth,
call_tree_start_time_nanos: metadata
.call_tree_start_time
.map(|call_tree_start_time| call_tree_start_time.as_nanos_since_unix_epoch()),
call_tree_depth: Some(metadata.call_tree_depth),
call_tree_start_time_nanos: Some(
metadata.call_tree_start_time.as_nanos_since_unix_epoch(),
),
call_subtree_deadline_nanos: None,
}
}
Expand All @@ -466,12 +466,10 @@ impl From<&Request> for pb_queues::Request {
impl From<pb_queues::RequestMetadata> for RequestMetadata {
fn from(metadata: pb_queues::RequestMetadata) -> Self {
Self {
call_tree_depth: metadata.call_tree_depth,
call_tree_start_time: metadata
.call_tree_start_time_nanos
.map(|call_tree_start_time| {
Time::from_nanos_since_unix_epoch(call_tree_start_time)
}),
call_tree_depth: metadata.call_tree_depth.unwrap_or(0),
call_tree_start_time: Time::from_nanos_since_unix_epoch(
metadata.call_tree_start_time_nanos.unwrap_or(0),
),
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions rs/types/types_test_utils/src/arbitrary.rs
Expand Up @@ -102,13 +102,12 @@ prop_compose! {
prop_compose! {
/// Returns an arbitrary ['RequestMetadata'].
pub fn request_metadata()(
call_tree_depth in proptest::option::of(any::<u64>()),
call_tree_start_time_nanos in proptest::option::of(any::<u64>()),
call_tree_depth in any::<u64>(),
call_tree_start_time_nanos in any::<u64>(),
) -> RequestMetadata {
RequestMetadata {
call_tree_depth,
call_tree_start_time: call_tree_start_time_nanos
.map(Time::from_nanos_since_unix_epoch),
call_tree_start_time: Time::from_nanos_since_unix_epoch(call_tree_start_time_nanos),
}
}
}
Expand Down

0 comments on commit 46595ae

Please sign in to comment.