Merge branch 'tiago+gian/required-proposal-id' (#2365)

* tiago+gian/required-proposal-id: Changelog #2365 `prepare_proposal_data` test helper requires proposal id Fix docstr errs Make `proposal_id` a required field in gov proposals
anoma · Jan 10, 2024 · aefd723 · aefd723
2 parents 9bc5d94 + a4fa318
commit aefd723
Show file tree

Hide file tree

Showing 12 changed files with 32 additions and 27 deletions.
diff --git a/.changelog/unreleased/improvements/2365-required-proposal-id.md b/.changelog/unreleased/improvements/2365-required-proposal-id.md
@@ -0,0 +1,2 @@
+- When constructing a governance proposal the id is now a required field.
+  ([\#2365](https://github.com/anoma/namada/pull/2365))
diff --git a/apps/src/lib/bench_utils.rs b/apps/src/lib/bench_utils.rs
@@ -244,7 +244,7 @@ impl Default for BenchShell {
         let signed_tx = bench_shell.generate_tx(
             TX_INIT_PROPOSAL_WASM,
             InitProposalData {
-                id: None,
+                id: 0,
                 content: content_section.get_hash(),
                 author: defaults::albert_address(),
                 r#type: ProposalType::Default(None),

diff --git a/apps/src/lib/node/ledger/shell/finalize_block.rs b/apps/src/lib/node/ledger/shell/finalize_block.rs
@@ -1612,7 +1612,7 @@ mod test_finalize_block {
             shell.proposal_data.insert(proposal_id);
 
             let proposal = InitProposalData {
-                id: Some(proposal_id),
+                id: proposal_id,
                 content: Hash::default(),
                 author: validator.clone(),
                 voting_start_epoch: Epoch::default(),

diff --git a/benches/native_vps.rs b/benches/native_vps.rs
@@ -122,7 +122,7 @@ fn governance(c: &mut Criterion) {
                 shell.generate_tx(
                     TX_INIT_PROPOSAL_WASM,
                     InitProposalData {
-                        id: None,
+                        id: 0,
                         content: content_section.get_hash(),
                         author: defaults::albert_address(),
                         r#type: ProposalType::Default(None),
@@ -174,7 +174,7 @@ fn governance(c: &mut Criterion) {
                 shell.generate_tx(
                     TX_INIT_PROPOSAL_WASM,
                     InitProposalData {
-                        id: Some(1),
+                        id: 1,
                         content: content_section.get_hash(),
                         author: defaults::albert_address(),
                         r#type: ProposalType::Default(Some(
@@ -246,7 +246,7 @@ fn governance(c: &mut Criterion) {
 //      let governance_proposal = shell.generate_tx(
 //          TX_INIT_PROPOSAL_WASM,
 //          InitProposalData {
-//              id: None,
+//              id: 0,
 //              content: content_section.get_hash(),
 //              author: defaults::albert_address(),
 //              r#type: ProposalType::Default(None),

diff --git a/benches/txs.rs b/benches/txs.rs
@@ -462,7 +462,7 @@ fn init_proposal(c: &mut Criterion) {
                             shell.generate_tx(
                                 TX_INIT_PROPOSAL_WASM,
                                 InitProposalData {
-                                    id: None,
+                                    id: 0,
                                     content: content_section.get_hash(),
                                     author: defaults::albert_address(),
                                     r#type: ProposalType::Default(None),
@@ -512,7 +512,7 @@ fn init_proposal(c: &mut Criterion) {
                             shell.generate_tx(
                                 TX_INIT_PROPOSAL_WASM,
                                 InitProposalData {
-                                    id: Some(1),
+                                    id: 1,
                                     content: content_section.get_hash(),
                                     author: defaults::albert_address(),
                                     r#type: ProposalType::Default(Some(

diff --git a/core/src/ledger/governance/cli/onchain.rs b/core/src/ledger/governance/cli/onchain.rs
@@ -21,7 +21,7 @@ use crate::types::storage::Epoch;
 /// The proposal structure
 pub struct OnChainProposal {
     /// The proposal id
-    pub id: Option<u64>,
+    pub id: u64,
     /// The proposal content
     pub content: BTreeMap<String, String>,
     /// The proposal author address

diff --git a/core/src/ledger/storage_api/governance.rs b/core/src/ledger/storage_api/governance.rs
@@ -31,11 +31,10 @@ where
     S: StorageRead + StorageWrite,
 {
     let counter_key = governance_keys::get_counter_key();
-    let proposal_id = if let Some(id) = data.id {
-        id
-    } else {
-        storage.read(&counter_key)?.unwrap()
-    };
+    let proposal_id = storage.read(&counter_key)?.expect(
+        "Storage should have been initialized with an initial governance \
+         proposal id",
+    );
 
     let content_key = governance_keys::get_content_key(proposal_id);
     storage.write_bytes(&content_key, content)?;

diff --git a/core/src/types/dec.rs b/core/src/types/dec.rs
@@ -67,19 +67,21 @@ impl Dec {
     /// the division is impossible (e.g., division by zero or overflow), `None`
     /// is returned.
     ///
-    /// Example:
-    /// ```
-    /// 
+    /// For instance:
+    ///
+    /// ```ignore
     /// let x = Dec::new(3, 1).unwrap(); // Represents 0.3
     /// let y = Dec::new(2, 1).unwrap(); // Represents 0.2
     /// let result = x.trunc_div(&y).unwrap();
     /// assert_eq!(result, Dec::new(15, 1).unwrap()); // Result is 1.5 truncated to 1 decimal place
     /// ```
     ///
     /// # Arguments
+    ///
     /// * `rhs`: The right-hand side `Dec` value for the division.
     ///
     /// # Returns
+    ///
     /// An `Option<Dec>` which is `Some` with the result if the division is
     /// successful, or `None` if the division cannot be performed.
     pub fn trunc_div(&self, rhs: &Self) -> Option<Self> {

diff --git a/core/src/types/transaction/governance.rs b/core/src/types/transaction/governance.rs
@@ -34,7 +34,7 @@ pub enum ProposalError {
 )]
 pub struct InitProposalData {
     /// The proposal id
-    pub id: Option<u64>,
+    pub id: u64,
     /// The proposal content
     pub content: Hash,
     /// The proposal author address
@@ -170,7 +170,7 @@ pub mod tests {
     prop_compose! {
         /// Generate a proposal initialization
         pub fn arb_init_proposal()(
-            id: Option<u64>,
+            id: u64,
             content in arb_hash(),
             author in arb_non_internal_address(),
             r#type in arb_proposal_type(),

diff --git a/light_sdk/src/transaction/governance.rs b/light_sdk/src/transaction/governance.rs
@@ -19,7 +19,7 @@ impl InitProposal {
     /// Build a raw InitProposal transaction from the given parameters
     #[allow(clippy::too_many_arguments)]
     pub fn new(
-        id: Option<u64>,
+        id: u64,
         content: Hash,
         author: Address,
         r#type: ProposalType,

diff --git a/sdk/src/signing.rs b/sdk/src/signing.rs
@@ -1086,9 +1086,7 @@ pub async fn to_ledger_vector(
             .hash();
 
         tv.output.push("Type : Init proposal".to_string());
-        if let Some(id) = init_proposal_data.id.as_ref() {
-            tv.output.push(format!("ID : {}", id));
-        }
+        tv.output.push(format!("ID : {}", init_proposal_data.id));
         tv.output.extend(vec![
             format!(
                 "Proposal type : {}",
@@ -1107,9 +1105,8 @@ pub async fn to_ledger_vector(
             format!("Content : {}", HEXLOWER.encode(&extra.0)),
         ]);
 
-        if let Some(id) = init_proposal_data.id.as_ref() {
-            tv.output_expert.push(format!("ID : {}", id));
-        }
+        tv.output_expert
+            .push(format!("ID : {}", init_proposal_data.id));
         tv.output_expert.extend(vec![
             format!(
                 "Proposal type : {}",

diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs
@@ -1814,6 +1814,7 @@ fn proposal_submission() -> Result<()> {
     let albert = find_address(&test, ALBERT)?;
     let valid_proposal_json_path = prepare_proposal_data(
         &test,
+        0,
         albert,
         TestWasms::TxProposalCode.read_bytes(),
         12,
@@ -1886,6 +1887,7 @@ fn proposal_submission() -> Result<()> {
     let albert = find_address(&test, ALBERT)?;
     let invalid_proposal_json = prepare_proposal_data(
         &test,
+        1,
         albert,
         TestWasms::TxProposalCode.read_bytes(),
         1,
@@ -2144,7 +2146,7 @@ fn pgf_governance_proposal() -> Result<()> {
     };
 
     let valid_proposal_json_path =
-        prepare_proposal_data(&test, albert, pgf_stewards, 12);
+        prepare_proposal_data(&test, 0, albert, pgf_stewards, 12);
     let validator_one_rpc = get_actor_rpc(&test, Who::Validator(0));
 
     let submit_proposal_args = vec![
@@ -2333,7 +2335,7 @@ fn pgf_governance_proposal() -> Result<()> {
     };
 
     let valid_proposal_json_path =
-        prepare_proposal_data(&test, albert, pgf_funding, 36);
+        prepare_proposal_data(&test, 1, albert, pgf_funding, 36);
     let validator_one_rpc = get_actor_rpc(&test, Who::Validator(0));
 
     let submit_proposal_args = vec![
@@ -2929,6 +2931,7 @@ fn implicit_account_reveal_pk() -> Result<()> {
             let author = find_address(&test, source).unwrap();
             let valid_proposal_json_path = prepare_proposal_data(
                 &test,
+                0,
                 author,
                 TestWasms::TxProposalCode.read_bytes(),
                 12,
@@ -3045,12 +3048,14 @@ fn test_epoch_sleep() -> Result<()> {
 /// This can be submitted with "init-proposal" command.
 pub fn prepare_proposal_data(
     test: &setup::Test,
+    id: u64,
     source: Address,
     data: impl serde::Serialize,
     start_epoch: u64,
 ) -> PathBuf {
     let valid_proposal_json = json!({
         "proposal": {
+            "id": id,
             "content": {
                 "title": "TheTitle",
                 "authors": "[email protected]",