fix: SNS Gov canister should deserialize to proto Governance, not API…

… Governance (#3391)

I think this was a bug in the code previously. 


[here](https://github.com/dfinity/ic/blob/4b9b18fcada06a30f9d2286eddcacff6521ace47/rs/sns/governance/canister/canister.rs#L258-L270)
is where we serialize Governance:

```rust
#[pre_upgrade]
fn canister_pre_upgrade() {
    log!(INFO, "Executing pre upgrade");

    let mut writer = BufferedStableMemWriter::new(STABLE_MEM_BUFFER_SIZE);

    governance() 
        .proto // This gets the `Governance` that's from `ic-sns-governance`
        .encode(&mut writer)
        .expect("Error. Couldn't serialize canister pre-upgrade.");

    writer.flush(); // or `drop(writer)`
    log!(INFO, "Completed pre upgrade");
}
```

And this is where we were deserializing it:

```rust
#[post_upgrade]
fn canister_post_upgrade() {
    log!(INFO, "Executing post upgrade");

    let reader = BufferedStableMemReader::new(STABLE_MEM_BUFFER_SIZE);

    match GovernanceProto::decode(reader) { // this is deserializing to the Governance that's in `ic-sns-governance-api`
        Err(err) => {
            // [...]
        }
        Ok(mut governance_proto) => {
            // Post-process GovernanceProto

            // TODO: Delete this once it's been released.
            populate_finalize_disbursement_timestamp_seconds(&mut governance_proto);

            canister_init_(governance_proto); // then in here we pass to canister_init_
            Ok(())
        }
    }
    .expect("Couldn't upgrade canister.");
    // [...]
}
```

`canister_init_`:
```rust
#[init]
fn canister_init_(init_payload: GovernanceProto) {
    let init_payload = sns_gov_pb::Governance::from(init_payload); // we convert the `ic-sns-governance-api` Governance to the `ic-sns-governance` Governance
    // [...]
}
```

This seems not quite right to me because we serialize one Governance
(`ic-sns-governance`), but deserialize to the other Governance
(`ic-sns-governance-api`) and then convert. Why not just deserialize to
the `ic-sns-governance` Governance directly? This only works because the
`ic-sns-governance-api` types still set `prost` derives, but I don't
think we want to be protobuffing the `ic-sns-governance-api` types
anyway.

(In fact, since we don't want to be protobuffing them, I actually remove
their prost implementations in the next PR)

---

However, we should still initialize the canister using the API types.
Doing this ensures that no proto types are part of our public candid
interface. So the `canister_init` method has been restored and annotated
with `#[init]`, and takes the API type like `canister_init_` did
previously. Now `canister_init_` takes the proto type, so it can be used
by `canister_post_upgrade` and by `canister_init` after `canister_init`
does the conversion.

---

[← Previous PR](#3393) | [Next PR
→](#3392)

rs/sns/governance/canister/canister.rs

@@ -35,7 +35,7 @@ use ic_sns_governance_api::pb::v1::{
     GetModeResponse, GetNeuron, GetNeuronResponse, GetProposal, GetProposalResponse,
     GetRunningSnsVersionRequest, GetRunningSnsVersionResponse,
     GetSnsInitializationParametersRequest, GetSnsInitializationParametersResponse,
-    GetUpgradeJournalRequest, GetUpgradeJournalResponse, Governance as GovernanceProto,
+    GetUpgradeJournalRequest, GetUpgradeJournalResponse, Governance as GovernanceApi,
     ListNervousSystemFunctionsResponse, ListNeurons, ListNeuronsResponse, ListProposals,
     ListProposalsResponse, ManageNeuron, ManageNeuronResponse, NervousSystemParameters,
     RewardEvent, SetMode, SetModeResponse,
@@ -208,11 +208,13 @@ fn caller() -> PrincipalId {
     PrincipalId::from(cdk_caller())
 }
 
-/// In contrast to canister_init(), this method does not do deserialization.
-/// In addition to canister_init, this method is called by canister_post_upgrade.
 #[init]
-fn canister_init_(init_payload: GovernanceProto) {
+fn canister_init(init_payload: GovernanceApi) {
     let init_payload = sns_gov_pb::Governance::from(init_payload);
+    canister_init_(init_payload);
+}
+
+fn canister_init_(init_payload: sns_gov_pb::Governance) {
     let init_payload = ValidGovernanceProto::try_from(init_payload).expect(
         "Cannot start canister, because the deserialized \
          GovernanceProto is invalid in some way",
@@ -277,7 +279,7 @@ fn canister_post_upgrade() {
 
     let reader = BufferedStableMemReader::new(STABLE_MEM_BUFFER_SIZE);
 
-    match GovernanceProto::decode(reader) {
+    match sns_gov_pb::Governance::decode(reader) {
         Err(err) => {
             log!(
                 ERROR,
@@ -304,7 +306,7 @@ fn canister_post_upgrade() {
     log!(INFO, "Completed post upgrade");
 }
 
-fn populate_finalize_disbursement_timestamp_seconds(governance_proto: &mut GovernanceProto) {
+fn populate_finalize_disbursement_timestamp_seconds(governance_proto: &mut sns_gov_pb::Governance) {
     for neuron in governance_proto.neurons.values_mut() {
         for disbursement in neuron.disburse_maturity_in_progress.iter_mut() {
             disbursement.finalize_disbursement_timestamp_seconds = Some(

rs/sns/governance/canister/tests.rs

@@ -1,7 +1,11 @@
 use super::*;
 use assert_matches::assert_matches;
 use candid_parser::utils::{service_equal, CandidSource};
-use ic_sns_governance_api::pb::v1::{DisburseMaturityInProgress, Neuron};
+use ic_sns_governance::pb::v1::{
+    governance::{Version, Versions},
+    upgrade_journal_entry::{Event, UpgradeStepsRefreshed},
+    DisburseMaturityInProgress, Neuron, UpgradeJournal, UpgradeJournalEntry,
+};
 use maplit::btreemap;
 use pretty_assertions::assert_eq;
 use std::collections::HashSet;
@@ -61,7 +65,7 @@ fn test_set_time_warp() {
 fn test_populate_finalize_disbursement_timestamp_seconds() {
     // Step 1: prepare a neuron with 2 in progress disbursement, one with
     // finalize_disbursement_timestamp_seconds as None, and the other has incorrect timestamp.
-    let mut governance_proto = GovernanceProto {
+    let mut governance_proto = sns_gov_pb::Governance {
         neurons: btreemap! {
             "1".to_string() => Neuron {
                 disburse_maturity_in_progress: vec![
@@ -86,7 +90,7 @@ fn test_populate_finalize_disbursement_timestamp_seconds() {
     populate_finalize_disbursement_timestamp_seconds(&mut governance_proto);
 
     // Step 3: verifies that both disbursements have the correct finalization timestamps.
-    let expected_governance_proto = GovernanceProto {
+    let expected_governance_proto = sns_gov_pb::Governance {
         neurons: btreemap! {
             "1".to_string() => Neuron {
                 disburse_maturity_in_progress: vec![
@@ -111,12 +115,6 @@ 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),