Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/streaming funding #393

Merged
merged 43 commits into from
Sep 21, 2021
Merged

Feature/streaming funding #393

merged 43 commits into from
Sep 21, 2021

Conversation

IanMendozaJaimes
Copy link
Contributor

@IanMendozaJaimes IanMendozaJaimes commented Sep 4, 2021

Includes DAO #390

Added streaming funding.

Added three new tables:

  • dho_table : for storing the organizations that are dhos
  • dho_vote_table : for storing user's votes, it is scoped by contract's scope
  • dho_shares_table : for storing the dhos that will receive part of the harvest

Added tests for dao, harvest and scheduler.

Brief description

Dhos can be created and deleted with createdho and removedho respectively.
When a dho is removed, all its votes are removed as well and the dho is removed immediately from the sharing table.

To vote, users must call the action votedhos with all the dhos they want to vote for. I think this way is easier to implement and validate. Users can call this action at any time and the votes will be recalculated for that user except if the user's voice is delegated. The delegation is implemented as well, to delegate it the dao actions delegate/undelegate are used.

To calculate how much percentage of the harvest each dho receives, the function dhocalcdists has to be triggered. I set this function to be executed every hour in scheduler.

To clean the old votes, the function dhocleanvts has to be called. I set this function to be executed every day.

Some considerations:

  • I added the dho_voice scope so the voice delegation can use the same methods for delegate/undelegate
  • For the cs points multiplier I am currently using the rank, i.e. from 1-99, I decided that because with the normal 0-2 multiplier votes percentages can become 0 very easily.

Deployment

Contracts to be deployed:

  • Dao
  • Harvest
  • scheduler

Other considerations:

  • Settings need to be updated
  • Permissions need to be updated

IanMendozaJaimes and others added 30 commits June 10, 2021 10:38
@IanMendozaJaimes IanMendozaJaimes marked this pull request as ready for review September 10, 2021 01:37
#pragma once

#include "proposals_base.hpp"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW these campaigns will likely no longer exist going forward - except invite funding campaigns.

@n13
Copy link
Member

n13 commented Sep 10, 2021 via email

virtual name get_scope() = 0;
virtual void check_can_vote(const name & status, const name & stage);
virtual bool check_prop_majority(std::map<std::string, VariantValue> & args);
virtual uint64_t min_stake(const asset & quantity, const name & fund);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to keep everything similar in the API

we usually use this func(name x, name y)

Why is this func(const name & x, const name & y) ?

I can see how const makes sense, as it leads to cleaner code, but the &... is this needed? Does it change anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if all the functions in the contract use &, it can lead to a better performance, because now we're only using references and a reference only uses 8 bytes (usually), so in terms of memory passed among all the functions in the contract it's really small. In that way we can pass types that are heavier than 8 bytes like names or maps but we're only consuming 8 bytes.

The reference is probably useless in the action declaration like ACTION dao::create(), but it's definitely needed in the subsequent functions called inside create.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well unlike desktop systems, in smart contracts this kind of thing can actually make a difference, thanks for the explanation, let's leave it.

milestone_fund
};

const name prop_active_size = "prop.act.sz"_n;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we actually keeping track of all these sizes, or are they carryover from the other contracts?

I know that we had them in the old contracts, but not sure they are still all in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the dao contract uses all those sizes. I am not sure all of them are in use, I think the votepow.szis not used but I included anyways just to try to mimic proposals behavior. There are four sizes but only the votepow.sz is the one that I think is not needed


typedef struct dhovote {
name dho;
uint64_t points;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

points here is always 0-100 right, like percentage points...

Copy link
Contributor Author

@IanMendozaJaimes IanMendozaJaimes Sep 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amm well no, I am storing there the result of multiplying:
assigned percentage * user's contribution score
so it can be greater than 100.

When a user votes, the user needs to call votedhos, in that function the contract validates that the percentage the user is assigning sums up to 100. The user only has to vote for the orgs he is going to assign a percentage bigger than 0, currently the minimum is 1% because it checks the sum is 100, I would have to modify it to sum up to 10000 if you would like to include 2 decimals.

But having decimals will make the implementation harder because right now everything can be fit in one call to votedhos because at most the user can vote for 100 dhos 1% each one. With decimals that number becomes larger.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need decimals, and thanks for the explanation

@@ -1174,10 +1174,12 @@ void harvest::send_distribute_harvest (name key, asset amount) {
std::make_tuple(uint64_t(0), uint64_t(20), amount)
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be deferred?

@n13 n13 merged commit 866f558 into master Sep 21, 2021
@IanMendozaJaimes IanMendozaJaimes mentioned this pull request Sep 21, 2021
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants