refactor: move serve_journal into upgrade_journal.rs (#3393)

## What

`serve_journal` is moved into `upgrade_journal.rs` within SNS
Governance. Previously it was located in `ic-nervous-system-common`

## Why

1. `serve_journal` was only used by SNS Governance, so it didn't really
need to be in ic-nervous-system-common. I think it was there because
that's also where `serve_logs` was. If there was another reason, please
let me know.
2. `serve_journal` a somewhat dangerous function because it is generic.
It takes anything that implements Serialize. I think this was done
because no function in `ic-nervous-system-common` can take
`UpgradeJournal` because that would require depending on
`ic-sns-governance` which would introduce a circular dependency.

But the function being generic is dangerous. We really only want to
serve the journal with this function, not any other type. This would
normally not be a problem, because what else are you going to pass to
`serve_journal` besides the journal? But now we have two types named
journal: the one in `ic-sns-governance` and `ic-sns-governance-api`. And
you really want to serialize the `ic-sns-governance-api` one, because
that's the one that has the manual `Serialize` implementation on it that
leads to the journal having a user-friendly output. By moving the
function into `ic-sns-governance`, we're able to make it not-generic,
and depend on only one type. Then there's no chance that it will be
called incorrect.

For convenience, I made the function take a `ic-sns-governance`
`UpgradeJournal` and do the conversion to the `ic-sns-governance-api`
`UpgradeJournal` itself. This simplifies its usage at the call site

In the next PR, I'm moving some things around, and this change is very
useful to make sure we're passing right thing to serve_journal.

[Next PR →](#3391)

rs/nervous_system/common/src/lib.rs

@@ -722,21 +722,6 @@ pub fn serve_metrics(
     }
 }
 
-pub fn serve_journal<Journal>(journal: &Journal) -> HttpResponse
-where
-    Journal: serde::Serialize,
-{
-    match serde_json::to_string(journal) {
-        Err(err) => {
-            HttpResponseBuilder::server_error(format!("Failed to encode journal: {}", err)).build()
-        }
-        Ok(body) => HttpResponseBuilder::ok()
-            .header("Content-Type", "application/json")
-            .with_body_and_content_length(body)
-            .build(),
-    }
-}
-
 /// Returns the total amount of memory (heap, stable memory, etc) that the calling canister has allocated.
 #[cfg(target_arch = "wasm32")]
 pub fn total_memory_size_bytes() -> usize {

rs/sns/governance/BUILD.bazel

@@ -62,6 +62,7 @@ DEPENDENCIES = [
     "@crate_index//:rust_decimal",
     "@crate_index//:serde",
     "@crate_index//:serde_bytes",
+    "@crate_index//:serde_json",
     "@crate_index//:strum",
 ]
 

rs/sns/governance/Cargo.toml

@@ -79,6 +79,7 @@ rust_decimal = "1.36.0"
 rust_decimal_macros = "1.36.0"
 serde = { workspace = true }
 serde_bytes = { workspace = true }
+serde_json = { workspace = true }
 strum = { workspace = true }
 strum_macros = { workspace = true }
 canbench-rs = { version = "0.1.7", optional = true }

rs/sns/governance/canister/canister.rs

@@ -11,7 +11,7 @@ use ic_nervous_system_clients::{
 };
 use ic_nervous_system_common::{
     dfn_core_stable_mem_utils::{BufferedStableMemReader, BufferedStableMemWriter},
-    serve_journal, serve_logs, serve_logs_v2, serve_metrics,
+    serve_logs, serve_logs_v2, serve_metrics,
 };
 use ic_nervous_system_proto::pb::v1::{
     GetTimersRequest, GetTimersResponse, ResetTimersRequest, ResetTimersResponse, Timers,
@@ -25,29 +25,27 @@ use ic_sns_governance::{
     logs::{ERROR, INFO},
     pb::v1 as sns_gov_pb,
     types::{Environment, HeapGrowthPotential},
+    upgrade_journal::serve_journal,
+};
+use ic_sns_governance_api::pb::v1::{
+    get_running_sns_version_response::UpgradeInProgress, governance::Version,
+    ClaimSwapNeuronsRequest, ClaimSwapNeuronsResponse, FailStuckUpgradeInProgressRequest,
+    FailStuckUpgradeInProgressResponse, GetMaturityModulationRequest,
+    GetMaturityModulationResponse, GetMetadataRequest, GetMetadataResponse, GetMode,
+    GetModeResponse, GetNeuron, GetNeuronResponse, GetProposal, GetProposalResponse,
+    GetRunningSnsVersionRequest, GetRunningSnsVersionResponse,
+    GetSnsInitializationParametersRequest, GetSnsInitializationParametersResponse,
+    GetUpgradeJournalRequest, GetUpgradeJournalResponse, Governance as GovernanceProto,
+    ListNervousSystemFunctionsResponse, ListNeurons, ListNeuronsResponse, ListProposals,
+    ListProposalsResponse, ManageNeuron, ManageNeuronResponse, NervousSystemParameters,
+    RewardEvent, SetMode, SetModeResponse,
 };
 #[cfg(feature = "test")]
 use ic_sns_governance_api::pb::v1::{
     AddMaturityRequest, AddMaturityResponse, AdvanceTargetVersionRequest,
     AdvanceTargetVersionResponse, GovernanceError, MintTokensRequest, MintTokensResponse, Neuron,
     RefreshCachedUpgradeStepsRequest, RefreshCachedUpgradeStepsResponse,
 };
-use ic_sns_governance_api::pb::{
-    v1 as api,
-    v1::{
-        get_running_sns_version_response::UpgradeInProgress, governance::Version,
-        ClaimSwapNeuronsRequest, ClaimSwapNeuronsResponse, FailStuckUpgradeInProgressRequest,
-        FailStuckUpgradeInProgressResponse, GetMaturityModulationRequest,
-        GetMaturityModulationResponse, GetMetadataRequest, GetMetadataResponse, GetMode,
-        GetModeResponse, GetNeuron, GetNeuronResponse, GetProposal, GetProposalResponse,
-        GetRunningSnsVersionRequest, GetRunningSnsVersionResponse,
-        GetSnsInitializationParametersRequest, GetSnsInitializationParametersResponse,
-        GetUpgradeJournalRequest, GetUpgradeJournalResponse, Governance as GovernanceProto,
-        ListNervousSystemFunctionsResponse, ListNeurons, ListNeuronsResponse, ListProposals,
-        ListProposalsResponse, ManageNeuron, ManageNeuronResponse, NervousSystemParameters,
-        RewardEvent, SetMode, SetModeResponse,
-    },
-};
 use prost::Message;
 use rand::{RngCore, SeedableRng};
 use rand_chacha::ChaCha20Rng;
@@ -646,9 +644,7 @@ pub fn http_request(request: HttpRequest) -> HttpResponse {
                 .clone()
                 .expect("The upgrade journal is not initialized for this SNS.");
 
-            let journal = api::UpgradeJournal::from(journal);
-
-            serve_journal(&journal.entries)
+            serve_journal(&journal)
         }
         "/metrics" => serve_metrics(encode_metrics),
         "/logs" => serve_logs_v2(request, &INFO, &ERROR),

rs/sns/governance/canister/tests.rs

@@ -1,11 +1,7 @@
 use super::*;
 use assert_matches::assert_matches;
 use candid_parser::utils::{service_equal, CandidSource};
-use ic_sns_governance_api::pb::v1::{
-    governance::{Version, Versions},
-    upgrade_journal_entry::{Event, UpgradeStepsRefreshed},
-    DisburseMaturityInProgress, Neuron, UpgradeJournal, UpgradeJournalEntry,
-};
+use ic_sns_governance_api::pb::v1::{DisburseMaturityInProgress, Neuron};
 use maplit::btreemap;
 use pretty_assertions::assert_eq;
 use std::collections::HashSet;
@@ -115,6 +111,12 @@ fn test_populate_finalize_disbursement_timestamp_seconds() {
 
 #[test]
 fn test_upgrade_journal() {
+    use ic_sns_governance::pb::v1::{
+        governance::{Version, Versions},
+        upgrade_journal_entry::{Event, UpgradeStepsRefreshed},
+        UpgradeJournal, UpgradeJournalEntry,
+    };
+
     let journal = UpgradeJournal {
         entries: vec![UpgradeJournalEntry {
             timestamp_seconds: Some(1000),
@@ -135,7 +137,7 @@ fn test_upgrade_journal() {
 
     // Currently, the `/journal` Http endpoint serves the entries directly, rather than the whole
     // journal object.
-    let http_response = serve_journal(&journal.entries);
+    let http_response = serve_journal(&journal);
     let expected_headers: HashSet<(_, _)> = HashSet::from_iter([
         ("Content-Type".to_string(), "application/json".to_string()),
         ("Content-Length".to_string(), "277".to_string()),

rs/sns/governance/src/upgrade_journal.rs

@@ -238,3 +238,17 @@ impl upgrade_journal_entry::Event {
         }
     }
 }
+
+pub fn serve_journal(journal: &UpgradeJournal) -> ic_canisters_http_types::HttpResponse {
+    use ic_canisters_http_types::HttpResponseBuilder;
+    let journal = ic_sns_governance_api::pb::v1::UpgradeJournal::from(journal.clone());
+    match serde_json::to_string(&journal.entries) {
+        Err(err) => {
+            HttpResponseBuilder::server_error(format!("Failed to encode journal: {}", err)).build()
+        }
+        Ok(body) => HttpResponseBuilder::ok()
+            .header("Content-Type", "application/json")
+            .with_body_and_content_length(body)
+            .build(),
+    }
+}