Add simple proposal voting example #12

eloylp · 2023-07-07T13:33:33Z

This PR adds a basic voting smart contract with the following features:

Allows voting on multiple proposals
Each proposal is configurable by:
- Timelock
- Target approval rate
- Maximum number of participants
Only designated admin is able to create proposals

This is a simplified version of https://github.com/eigerco/smart-proposals

eloylp · 2023-07-07T13:37:43Z

contracts/voting/src/lib.rs

+    pub fn create_proposal(env: Env, id: u64) -> Result<(), Error> {
+        let voting_period_secs = 3600; // one hour
+        let target_approval_rate_bps = 50_00; // At least 50% of the votes to be approved.
+        let total_voters = 1000; // Up to 1000 participants.


I think this parameters could be easily configured in the Wizard.

Why doesn this happen in init?

Let's make them init parameters.

We can move them to the init. Lets just take into account if we move them there, we need to store them in the ledger for later retrieval in the create_proposal function. Like we are doing here.

I have doubts regarding this, because adding/retrieving elements from the storage, has consequences in fees. See this docs. So maybe they are just good as hardcoded values ? I really dont know.

I will proceed on moving them to init in the meanwhile 👍

You are right. Maybe add a comment in the code for future consideration of making them hardcoded if the fee is too high :)

Fixed at e3cf2f7

eloylp · 2023-07-07T13:38:20Z

contracts/voting/src/lib.rs

+            .set(&DataKey::Proposals, &Map::<u64, Proposal>::new(&env));
+    }
+
+    pub fn create_proposal(env: Env, id: u64) -> Result<(), Error> {


Currently its an ID .. but maybe adding a hash field would be better. Let me know your thoughts.

Why doesn this happen in init?

You said this is not doable in init?

SOrry missed this reply, i replied here : #12 (comment)

eloylp · 2023-07-10T10:39:48Z

contracts/voting/src/test.rs

+    );
+}
+
+fn setup_test<'a>() -> (Env, ProposalVotingContractClient<'a>, Address) {


Maybe we could make this kind of helpers more generic in the future, whenever we see this patterns repeating.

eloylp · 2023-07-10T10:40:50Z

contracts/voting/src/test.rs

+    assert_eq!(Err(Error::VotingClosed), result)
+}
+
+fn advance_ledger_time_in(time: u64, env: &mut Env) {


This is another test helper i found useful.

This will need further study due to fees: https://soroban.stellar.org/docs/fundamentals-and-concepts/fees-and-metering#resource-fee

geofmureithi

LGTM

geofmureithi · 2023-07-11T06:59:34Z

contracts/voting/src/lib.rs

+            .set(&DataKey::Proposals, &Map::<u64, Proposal>::new(&env));
+    }
+
+    pub fn create_proposal(env: Env, id: u64) -> Result<(), Error> {


Why doesn this happen in init?

geofmureithi · 2023-07-11T07:01:35Z

contracts/voting/src/lib.rs

+#[contractimpl]
+impl ProposalVotingContract {
+    pub fn init(env: Env, admin: Address) {
+        env.storage().set(&DataKey::Admin, &admin);


We need to ensure you are logged in as admin

Suggested change

env.storage().set(&DataKey::Admin, &admin);

admin.require_auth();

env.storage().set(&DataKey::Admin, &admin);

geofmureithi · 2023-07-11T07:02:10Z

contracts/voting/src/lib.rs

+    pub fn create_proposal(env: Env, id: u64) -> Result<(), Error> {
+        let voting_period_secs = 3600; // one hour
+        let target_approval_rate_bps = 50_00; // At least 50% of the votes to be approved.
+        let total_voters = 1000; // Up to 1000 participants.


Why doesn this happen in init?

contracts/Cargo.toml

eloylp · 2023-07-27T16:51:14Z

@geofmureithi just added rust docs and checked add. See last 2 commits.

Requesting re-review !

geofmureithi

If there is a specific reason not to run creation in init, please highlight it. Otherwise, LGTM

geofmureithi

If there is a specific reason not to run creation in init, please highlight it. Otherwise, LGTM

eloylp · 2023-07-28T09:04:47Z

If there is a specific reason not to run creation in init, please highlight it. Otherwise, LGTM

Thanks for review @geofmureithi . This contract has the capability of tracking multiple proposals during its lifecycle. The parameters we store in the init function are only intended as "defaults", for later use in create_proposal function, that way its not needed to specify all proposal parameters on each call, just the id of the new proposal, which i think is convenient.

If an user, wants different parameters for an specific proposal during the lifecycle of this contract, it can invoke directly the create_custom_proposal as that function will require all proposal parameters, bypassing defaults.

geofmureithi · 2023-07-28T13:16:38Z

But once you deploy you get a contact id, shouldn't we be using that?

eloylp · 2023-07-28T13:54:17Z

But once you deploy you get a contact id, shouldn't we be using that?

@geofmureithi

With the current example, we can have multiple proposals in the same smart contract. So for voting we would need the contract id + the proposal id.

An example of how to invoke the contract can be checked here: #5 (comment)

geofmureithi · 2023-07-28T14:03:39Z

I have not seen any other examples approach it that way. Especially because everything else is already hardcoded hence it wont create a new proposal but rather a clone of the same proposal just with different id.

eloylp · 2023-07-28T14:26:10Z

I have not seen any other examples approach it that way. Especially because everything else is already hardcoded hence it wont create a new proposal but rather a clone of the same proposal just with different id.

I was probably influenced by this . Although is true eth is a different environment.

There are other examples, like the multi swap or auth . They use multiple data structures to represent different entities.

Anyway, if you think there is something wrong with the current design, let me know and we can change it.

geofmureithi · 2023-07-28T14:34:51Z

I think the problem you ate trying to solve is already solved by wasm. I see how you can see it that way but instead of your approach we would rather have a single proposal per contract. I think there is no objectively better one because it matters in use case.
Yet we are building for developers. You created a two part process where one is needed. If someone wants to reuse then they can write a bash script.
IMO lets not reinvent the wheel.

eloylp commented Jul 7, 2023

View reviewed changes

eloylp mentioned this pull request Jul 7, 2023

Choose/elaborate a configurable smart contract to add to Nebula's UI #5

Closed

eloylp force-pushed the add-simple-voting-example branch from fa0daa0 to 675b8b3 Compare July 7, 2023 13:56

eloylp commented Jul 10, 2023

View reviewed changes

eloylp mentioned this pull request Jul 10, 2023

UI design #9

Closed

eloylp marked this pull request as ready for review July 10, 2023 21:29

geofmureithi requested review from geofmureithi and mariopil July 11, 2023 08:03

eloylp had a problem deploying to github-pages July 20, 2023 16:26 — with GitHub Actions Failure

eloylp force-pushed the add-simple-voting-example branch from 7e3eb6f to 5b38e9d Compare July 20, 2023 16:28

eloylp had a problem deploying to github-pages July 20, 2023 16:28 — with GitHub Actions Failure

eloylp self-assigned this Jul 26, 2023

eloylp added 9 commits July 26, 2023 18:08

Add simple proposal voting example

a9dd557

Fix problem with symbol length

e12993e

Add initial workspace for hosting all contracts

689bba1

Move parameters to constructor

b04f4c7

This will need further study due to fees: https://soroban.stellar.org/docs/fundamentals-and-concepts/fees-and-metering#resource-fee

Update voting contract to last SDK

7cb3430

Simplify cargo toml of voting example

0644489

Move voting contract out of cargo workspace

cbd3a03

Move contract to contract folder

267cf1e

Ensure voting contract can only be initialised once

db17fc5

eloylp force-pushed the add-simple-voting-example branch from 5b38e9d to db17fc5 Compare July 26, 2023 16:09

eloylp linked an issue Jul 26, 2023 that may be closed by this pull request

Define the use of the new smart contract import tool for the proof of concept #7

Closed

2 tasks

eloylp removed a link to an issue Jul 26, 2023

Define the use of the new smart contract import tool for the proof of concept #7

Closed

2 tasks

eloylp linked an issue Jul 26, 2023 that may be closed by this pull request

Choose/elaborate a configurable smart contract to add to Nebula's UI #5

Closed

eloylp mentioned this pull request Jul 26, 2023

Add initial CI for Rust contracts #22

Merged

geofmureithi approved these changes Jul 27, 2023

View reviewed changes

Add rust docs to simple voting example

cfcf87f

Ensure checked add

a170e98

eloylp force-pushed the add-simple-voting-example branch from 7bbab66 to a170e98 Compare July 27, 2023 16:49

eloylp requested a review from geofmureithi July 27, 2023 16:51

geofmureithi approved these changes Jul 28, 2023

View reviewed changes

eloylp merged commit dbba7bd into main Jul 28, 2023

eloylp deleted the add-simple-voting-example branch July 28, 2023 10:56

eloylp mentioned this pull request Jul 30, 2023

Think about the design of the voting contract example #26

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple proposal voting example #12

Add simple proposal voting example #12

eloylp commented Jul 7, 2023

eloylp Jul 7, 2023

geofmureithi Jul 11, 2023

mariopil Jul 11, 2023

eloylp Jul 11, 2023

mariopil Jul 11, 2023

eloylp Jul 11, 2023

eloylp Jul 7, 2023

geofmureithi Jul 11, 2023

geofmureithi Jul 28, 2023

eloylp Jul 28, 2023

eloylp Jul 10, 2023 •

edited

Loading

eloylp Jul 10, 2023 •

edited

Loading

geofmureithi left a comment

geofmureithi Jul 11, 2023

geofmureithi Jul 11, 2023

geofmureithi Jul 11, 2023

eloylp commented Jul 27, 2023

geofmureithi left a comment

geofmureithi left a comment

eloylp commented Jul 28, 2023 •

edited

Loading

geofmureithi commented Jul 28, 2023

eloylp commented Jul 28, 2023

geofmureithi commented Jul 28, 2023

eloylp commented Jul 28, 2023 •

edited

Loading

geofmureithi commented Jul 28, 2023

	env.storage().set(&DataKey::Admin, &admin);
	admin.require_auth();
	env.storage().set(&DataKey::Admin, &admin);

Add simple proposal voting example #12

Add simple proposal voting example #12

Conversation

eloylp commented Jul 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloylp Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

eloylp Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

geofmureithi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloylp commented Jul 27, 2023

geofmureithi left a comment

Choose a reason for hiding this comment

geofmureithi left a comment

Choose a reason for hiding this comment

eloylp commented Jul 28, 2023 • edited Loading

geofmureithi commented Jul 28, 2023

eloylp commented Jul 28, 2023

geofmureithi commented Jul 28, 2023

eloylp commented Jul 28, 2023 • edited Loading

geofmureithi commented Jul 28, 2023

eloylp Jul 10, 2023 •

edited

Loading

eloylp Jul 10, 2023 •

edited

Loading

eloylp commented Jul 28, 2023 •

edited

Loading

eloylp commented Jul 28, 2023 •

edited

Loading