Skip to content

Commit 46595ae

Browse files
stiegercstiegerc
committed
Chore: Make fields in RequestMetadata non-optional.
1 parent eee4085 commit 46595ae

File tree

8 files changed

+37
-78
lines changed

8 files changed

+37
-78
lines changed

‎rs/canonical_state/src/encoding/tests/compatibility.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,8 @@ fn canonical_encoding_request_v14_plus() {
323323
.method_name("test".to_string())
324324
.method_payload(vec![6])
325325
.metadata(Some(RequestMetadata {
326-
call_tree_depth: Some(13),
327-
call_tree_start_time: Some(Time::from_nanos_since_unix_epoch(101)),
326+
call_tree_depth: 13,
327+
call_tree_start_time: Time::from_nanos_since_unix_epoch(101),
328328
}))
329329
.build()
330330
.into();

‎rs/canonical_state/src/encoding/tests/encoding.rs

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
use super::test_fixtures::*;
22
use crate::{all_supported_versions, encoding::*};
3-
use ic_test_utilities::types::messages::RequestBuilder;
4-
use ic_types::messages::RequestMetadata;
53

64
#[test]
75
fn roundtrip_encoding_stream_header() {
@@ -26,24 +24,6 @@ fn roundtrip_encoding_request() {
2624
}
2725
}
2826

29-
#[test]
30-
fn request_missing_metadata_and_missing_metadata_fields_encode_the_same() {
31-
for certification_version in all_supported_versions() {
32-
let request1: RequestOrResponse = RequestBuilder::new().metadata(None).build().into();
33-
let request2: RequestOrResponse = RequestBuilder::new()
34-
.metadata(Some(RequestMetadata {
35-
call_tree_depth: None,
36-
call_tree_start_time: None,
37-
}))
38-
.build()
39-
.into();
40-
assert_eq!(
41-
encode_message(&request1, certification_version),
42-
encode_message(&request2, certification_version),
43-
);
44-
}
45-
}
46-
4727
#[test]
4828
fn roundtrip_encoding_response() {
4929
let response = response();

‎rs/canonical_state/src/encoding/tests/test_fixtures.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ pub fn request(certification_version: CertificationVersion) -> RequestOrResponse
3434
.method_payload(vec![6])
3535
.metadata(
3636
(certification_version >= CertificationVersion::V14).then_some(RequestMetadata {
37-
call_tree_depth: Some(1),
38-
call_tree_start_time: Some(Time::from_nanos_since_unix_epoch(100_000)),
37+
call_tree_depth: 1,
38+
call_tree_start_time: Time::from_nanos_since_unix_epoch(100_000),
3939
}),
4040
)
4141
.build()

‎rs/canonical_state/src/encoding/types.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ impl TryFrom<RequestOrResponse> for ic_types::messages::RequestOrResponse {
260260
impl From<&ic_types::messages::RequestMetadata> for RequestMetadata {
261261
fn from(metadata: &ic_types::messages::RequestMetadata) -> Self {
262262
RequestMetadata {
263-
call_tree_depth: metadata.call_tree_depth,
264-
call_tree_start_time: metadata.call_tree_start_time,
263+
call_tree_depth: Some(metadata.call_tree_depth),
264+
call_tree_start_time: Some(metadata.call_tree_start_time),
265265
call_subtree_deadline: None,
266266
}
267267
}
@@ -275,16 +275,6 @@ impl From<(&ic_types::messages::Request, CertificationVersion)> for Request {
275275
cycles: (&request.payment, certification_version).into(),
276276
icp: 0,
277277
};
278-
let metadata = match request.metadata.as_ref() {
279-
Some(ic_types::messages::RequestMetadata {
280-
call_tree_depth: None,
281-
call_tree_start_time: None,
282-
})
283-
| None => None,
284-
Some(metadata) => {
285-
(certification_version >= CertificationVersion::V14).then_some(metadata.into())
286-
}
287-
};
288278

289279
Self {
290280
receiver: request.receiver.get().to_vec(),
@@ -294,16 +284,20 @@ impl From<(&ic_types::messages::Request, CertificationVersion)> for Request {
294284
method_name: request.method_name.clone(),
295285
method_payload: request.method_payload.clone(),
296286
cycles_payment: None,
297-
metadata,
287+
metadata: request.metadata.as_ref().and_then(|metadata| {
288+
(certification_version >= CertificationVersion::V14).then_some(metadata.into())
289+
}),
298290
}
299291
}
300292
}
301293

302294
impl From<RequestMetadata> for ic_types::messages::RequestMetadata {
303295
fn from(metadata: RequestMetadata) -> Self {
304296
ic_types::messages::RequestMetadata {
305-
call_tree_depth: metadata.call_tree_depth,
306-
call_tree_start_time: metadata.call_tree_start_time,
297+
call_tree_depth: metadata.call_tree_depth.unwrap_or(0),
298+
call_tree_start_time: metadata
299+
.call_tree_start_time
300+
.unwrap_or(Time::from_nanos_since_unix_epoch(0)),
307301
}
308302
}
309303
}

‎rs/state_manager/src/tree_hash.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,14 +209,11 @@ mod tests {
209209
stream.push(
210210
RequestBuilder::new()
211211
.metadata(
212-
(certification_version >= CertificationVersion::V14).then_some(
213-
RequestMetadata {
214-
call_tree_depth: (i % 2 > 0).then_some(i % 3),
215-
call_tree_start_time: (i % 3 > 0)
216-
.then_some(i % 2)
217-
.map(Time::from_nanos_since_unix_epoch),
218-
},
219-
),
212+
(certification_version >= CertificationVersion::V14 && i % 5 != 0)
213+
.then_some(RequestMetadata {
214+
call_tree_depth: i % 3,
215+
call_tree_start_time: Time::from_nanos_since_unix_epoch(i % 2),
216+
}),
220217
)
221218
.build()
222219
.into(),
@@ -336,8 +333,8 @@ mod tests {
336333
"1ED37E00D177681A4111B6D45F518DF3E414B0B614333BB6552EBC0D8492B687",
337334
"62B2E77DFCD17C7E0CE3E762FD37281776C4B0A38CE1B83A1316614C3F849E39",
338335
"80D4B528CC9E09C775273994261DD544D45EFFF90B655D90FC3A6E3F633ED718",
339-
"C8E5C0C21DA665B94DD78C3CCA682A44780058E9D92BC120E7CF0DD2952CE824",
340-
"E2DA77E9435CB7C1F0E598DC3B3C41D305B216AF3E806E350BED1CC16E56817F",
336+
"970BC5155AEB4B4F81E470CBF6748EFA7D8805B936998A54AE70B7DD21F5DDCC",
337+
"EA3B53B72150E3982CB0E6773F86634685EE7B153DCFE10D86D9927778409D97",
341338
];
342339

343340
for certification_version in CertificationVersion::iter() {

‎rs/types/types/src/messages.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -680,23 +680,14 @@ mod tests {
680680
assert_matches!(signed_ingress1, Ok(signed_ingress1) if signed_ingress == signed_ingress1);
681681
}
682682

683-
fn make_request_metadata(
684-
call_tree_depth: Option<u64>,
685-
call_tree_start_time: Option<u64>,
686-
) -> Option<RequestMetadata> {
687-
Some(RequestMetadata {
688-
call_tree_depth,
689-
call_tree_start_time: call_tree_start_time.map(Time::from_nanos_since_unix_epoch),
690-
})
691-
}
692-
693683
#[test]
694684
fn serialize_request_via_bincode() {
695685
for metadata in [
696686
None,
697-
make_request_metadata(None, None),
698-
make_request_metadata(Some(13), None),
699-
make_request_metadata(None, Some(17)),
687+
Some(RequestMetadata {
688+
call_tree_depth: 13,
689+
call_tree_start_time: Time::from_nanos_since_unix_epoch(17),
690+
}),
700691
] {
701692
let request = Request {
702693
receiver: CanisterId::from(13),

‎rs/types/types/src/messages/inter_canister.rs

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,11 @@ pub type CallContextId = Id<CallContextIdTag, u64>;
4040
#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
4141
pub struct RequestMetadata {
4242
/// Indicates how many steps down the call tree a request is, starting at 0.
43-
pub call_tree_depth: Option<u64>,
43+
pub call_tree_depth: u64,
4444
/// The block time (on the respective subnet) at the start of the call at the
4545
/// root of the call tree that this request is part of. This is used for metrics
4646
/// only.
47-
pub call_tree_start_time: Option<Time>,
47+
pub call_tree_start_time: Time,
4848
}
4949

5050
/// Canister-to-canister request message.
@@ -439,10 +439,10 @@ impl From<Response> for RequestOrResponse {
439439
impl From<&RequestMetadata> for pb_queues::RequestMetadata {
440440
fn from(metadata: &RequestMetadata) -> Self {
441441
Self {
442-
call_tree_depth: metadata.call_tree_depth,
443-
call_tree_start_time_nanos: metadata
444-
.call_tree_start_time
445-
.map(|call_tree_start_time| call_tree_start_time.as_nanos_since_unix_epoch()),
442+
call_tree_depth: Some(metadata.call_tree_depth),
443+
call_tree_start_time_nanos: Some(
444+
metadata.call_tree_start_time.as_nanos_since_unix_epoch(),
445+
),
446446
call_subtree_deadline_nanos: None,
447447
}
448448
}
@@ -466,12 +466,10 @@ impl From<&Request> for pb_queues::Request {
466466
impl From<pb_queues::RequestMetadata> for RequestMetadata {
467467
fn from(metadata: pb_queues::RequestMetadata) -> Self {
468468
Self {
469-
call_tree_depth: metadata.call_tree_depth,
470-
call_tree_start_time: metadata
471-
.call_tree_start_time_nanos
472-
.map(|call_tree_start_time| {
473-
Time::from_nanos_since_unix_epoch(call_tree_start_time)
474-
}),
469+
call_tree_depth: metadata.call_tree_depth.unwrap_or(0),
470+
call_tree_start_time: Time::from_nanos_since_unix_epoch(
471+
metadata.call_tree_start_time_nanos.unwrap_or(0),
472+
),
475473
}
476474
}
477475
}

‎rs/types/types_test_utils/src/arbitrary.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,12 @@ prop_compose! {
102102
prop_compose! {
103103
/// Returns an arbitrary ['RequestMetadata'].
104104
pub fn request_metadata()(
105-
call_tree_depth in proptest::option::of(any::<u64>()),
106-
call_tree_start_time_nanos in proptest::option::of(any::<u64>()),
105+
call_tree_depth in any::<u64>(),
106+
call_tree_start_time_nanos in any::<u64>(),
107107
) -> RequestMetadata {
108108
RequestMetadata {
109109
call_tree_depth,
110-
call_tree_start_time: call_tree_start_time_nanos
111-
.map(Time::from_nanos_since_unix_epoch),
110+
call_tree_start_time: Time::from_nanos_since_unix_epoch(call_tree_start_time_nanos),
112111
}
113112
}
114113
}

0 commit comments

Comments
 (0)