From 5f735e03a72cec6e65d2002811fc59a23bde969f Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Fri, 5 Jan 2024 14:42:20 +0000 Subject: [PATCH 1/4] Make `proposal_id` a required field in gov proposals --- apps/src/lib/bench_utils.rs | 2 +- apps/src/lib/node/ledger/shell/finalize_block.rs | 2 +- benches/native_vps.rs | 6 +++--- benches/txs.rs | 4 ++-- core/src/ledger/governance/cli/onchain.rs | 2 +- core/src/ledger/storage_api/governance.rs | 9 ++++----- core/src/types/transaction/governance.rs | 4 ++-- light_sdk/src/transaction/governance.rs | 2 +- sdk/src/signing.rs | 9 +++------ 9 files changed, 18 insertions(+), 22 deletions(-) diff --git a/apps/src/lib/bench_utils.rs b/apps/src/lib/bench_utils.rs index ddfae1a4cc..da55dfc8a8 100644 --- 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 index 3620d2f3b0..7ec6e3dd42 100644 --- 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 index a4829c4601..6959dfafe4 100644 --- a/benches/native_vps.rs +++ b/benches/native_vps.rs @@ -126,7 +126,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), @@ -178,7 +178,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( @@ -250,7 +250,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 index d7290852bb..2e32fed2ef 100644 --- 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 index 976557c913..14c93a068d 100644 --- a/core/src/ledger/governance/cli/onchain.rs +++ b/core/src/ledger/governance/cli/onchain.rs @@ -20,7 +20,7 @@ use crate::types::storage::Epoch; /// The proposal structure pub struct OnChainProposal { /// The proposal id - pub id: Option, + pub id: u64, /// The proposal content pub content: BTreeMap, /// The proposal author address diff --git a/core/src/ledger/storage_api/governance.rs b/core/src/ledger/storage_api/governance.rs index 4daf731fb4..592734eb89 100644 --- 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/transaction/governance.rs b/core/src/types/transaction/governance.rs index 0e7f41179b..ce41734192 100644 --- 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, + pub id: u64, /// The proposal content pub content: Hash, /// The proposal author address @@ -180,7 +180,7 @@ pub mod tests { prop_compose! { /// Generate a proposal initialization pub fn arb_init_proposal()( - id: Option, + 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 index c09f0fc8c1..a3a37a6fd2 100644 --- 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, + id: u64, content: Hash, author: Address, r#type: ProposalType, diff --git a/sdk/src/signing.rs b/sdk/src/signing.rs index e52eb668b4..87488cf4a2 100644 --- 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 : {}", From 06e61724cc9dd19e25e36f5ef1d1e2e5131f74c6 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Fri, 5 Jan 2024 15:07:54 +0000 Subject: [PATCH 2/4] Fix docstr errs --- core/src/types/dec.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/types/dec.rs b/core/src/types/dec.rs index 4ec4e223ef..878ac57914 100644 --- a/core/src/types/dec.rs +++ b/core/src/types/dec.rs @@ -67,9 +67,9 @@ 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(); @@ -77,9 +77,11 @@ impl Dec { /// ``` /// /// # Arguments + /// /// * `rhs`: The right-hand side `Dec` value for the division. /// /// # Returns + /// /// An `Option` 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 { From 72da2ed955c9305db278e4a4ec45ee47edd68699 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 9 Jan 2024 12:55:50 +0100 Subject: [PATCH 3/4] `prepare_proposal_data` test helper requires proposal id --- tests/src/e2e/ledger_tests.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 60da7a9b3e..4184c6de7e 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1811,6 +1811,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, @@ -1883,6 +1884,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, @@ -2141,7 +2143,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![ @@ -2330,7 +2332,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![ @@ -2926,6 +2928,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, @@ -3042,12 +3045,14 @@ fn test_epoch_sleep() -> Result<()> { /// This can be submitted with "init-proposal" command. 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": "test@test.com", From a4fa3181e595fcbd0d032a528df42cf4c161b18f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 9 Jan 2024 12:59:28 +0100 Subject: [PATCH 4/4] Changelog #2365 --- .changelog/unreleased/improvements/2365-required-proposal-id.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/2365-required-proposal-id.md diff --git a/.changelog/unreleased/improvements/2365-required-proposal-id.md b/.changelog/unreleased/improvements/2365-required-proposal-id.md new file mode 100644 index 0000000000..4debed4491 --- /dev/null +++ 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)) \ No newline at end of file