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

fork pallet-democracy and refactor for optimistic governance #365

Merged
merged 10 commits into from
Nov 23, 2021

Conversation

gregdhill
Copy link
Member

@gregdhill gregdhill commented Nov 10, 2021

Forks the Substrate democracy pallet in order to implement optimistic governance by default.

@gregdhill gregdhill marked this pull request as draft November 10, 2021 13:39
@gregdhill gregdhill force-pushed the feat/democracy-pallet branch 2 times, most recently from 0758266 to f3cd3fc Compare November 16, 2021 16:23
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
@gregdhill gregdhill marked this pull request as ready for review November 23, 2021 09:46
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Copy link
Member

@sander2 sander2 left a comment

Choose a reason for hiding this comment

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

Looks mostly ok to me. Some of the parity code looks a bit strange to me, but I'm assuming it's been audited

/// weighted according to this value with no refund.
///
/// Weight: `O(S)` where S is the number of seconds a proposal already has.
#[pallet::weight(T::WeightInfo::second(*seconds_upper_bound))]
Copy link
Member

Choose a reason for hiding this comment

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

This is quite an unfortunate effect of storing the deposits in a vec. Since this is paritys code I'll assume this is for a good reason, but I haven't come across it yet

index: ReferendumIndex,
status: ReferendumStatus<T::BlockNumber, T::Hash, BalanceOf<T>>,
) -> Result<bool, DispatchError> {
let total_issuance = T::Currency::total_issuance();
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how this exactly will work if voting power decreases over time

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, when I integrate the escrow pallet it will probably have to dynamically calculate the tally from the current block height.

mut n1: T,
mut d1: T,
mut n2: T,
mut d2: T,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should check if d1 and d2 are non-zero here, and return an option/result

@gregdhill gregdhill merged commit 57613cd into interlay:master Nov 23, 2021
@gregdhill gregdhill deleted the feat/democracy-pallet branch November 23, 2021 14:37
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.

None yet

2 participants