-
Notifications
You must be signed in to change notification settings - Fork 810
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
Optimistic Project Funding #6994
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can open the PR as a draft until it is ready.
Or ping when it is ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some pretty superficial notes to start my review. Can continue with a more detailed review later tomorrow.
As a general remark, if you have a voting schema that somehow resembles what's already out there on OpenGov, I suggest you consider exploring the pallets that implement Tally
and integrate with pallet-referenda
.
substrate/frame/opf/src/lib.rs
Outdated
/// OPF Projects registration | ||
/// | ||
/// ## Dispatch Origin | ||
/// | ||
/// Must be AdminOrigin | ||
/// | ||
/// ## Details | ||
/// | ||
/// From this extrinsic only AdminOrigin can register project. | ||
/// | ||
/// ### Parameters | ||
/// - `projects_id`: The accounts that might be funded. | ||
/// | ||
/// ### Errors | ||
/// - [`Error::<T>::MaximumProjectsNumber`]: Maximum number of project subscriptions reached | ||
/// | ||
/// ## Events | ||
/// Emits [`Event::<T>::Projectslisted`]. | ||
#[pallet::call_index(1)] | ||
#[pallet::weight(0)] | ||
pub fn register_projects_batch( | ||
origin: OriginFor<T>, | ||
projects_id: BoundedVec<ProjectId<T>, T::MaxProjects>, | ||
) -> DispatchResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to just leave a register_project
call, then allow people to propose a batch of projects via Utilities::batch
/Utilities::batchAll
.
This implies the additional gain of not needing to handle BoundedVec
s, which sometimes can be annoying. Also, benchmarking is simpler.
Finally, consider this:
We already had registered three projects, max is 5. An actor not knowing this would attempt registering two projects (since 2 < 5): The extrinsic would fail at this point.
Now, consider the case with a batch
: the first project would be registered, but the second one would fail. However, the actor would be able to know which one was registered, which one failed, and why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my understanding of the pallet specifications, projects are first whitelisted in OpenGov, and then transferred to the OPF pallet, so I assumed that this transfer is not the task of any user, but requires a particular Origin, and in this case, it made sense to register in batch.
@pandres95 and @kianenigma , I have a question: Is there a reason for Not using pallet-democracy? |
Description
This PR is related to this issue .
Through the introduction of the OPF pallet and the DISTRIBUTION pallet, we are handling the Optimistic Project Funding.
It allows users to nominate projects (whitelisted in OpenGov) with their DOT. This mechanism will be funded with a constant stream of DOT taken directly from inflation and distributed to projects based on the proportion of DOT that has nominated them.
Integration
Review Notes
Terminology
The constants available in the runtime for the OPF Pallet:
Spends
.Functions
register_project
: Register by OpenGov, should take AccountId and project Purpose.unregister_project
: Unregister by OpenGovclaim
: To claim a spendChecklist
pallet-opf