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/plmc 398 add treasury pallet to base runtime should like the one in #154

Conversation

vstam1
Copy link
Collaborator

@vstam1 vstam1 commented Jan 24, 2024

TODO:

  • check try-runtime working

Adds the following pallets to base runtime:

  • Treasury
  • Democracy
  • Collective:
    • Council
    • Technical Committee
  • Election Phragmen
  • Scheduler
  • Preimage

Adds pallets to test runtime:

  • Election Phragmen

Important changes to custom pallets:

Democracy

  • From Reserve/Lock -> Hold/Freeze
  • Vote lock maximum from free_balance -> total_balance (allows voting with staked balance)

Election phragmen

  • From Reserve/Lock -> Hold/Freeze
  • Remove Deposit (reserve) requirement for voting
  • Vote lock maximum from free_balance -> total_balance (allows voting with staked balance)
  • Introduce vote locking period to lock users once they voted.
    • Config item VotingLockPeriod: Time users are locked for from the moment they vote.
    • Change in vote -> renews the locking period
    • Can only remove themselves as voters after VotingLockPeriod

Other important changes

  • Added ToGrowthTreasury struct to both base/test runtime. Implements new OnUnbalanced that works together with the fungibles traits, to allow sending slashed funds in both Elections and Democracy pallet to the treasury.
  • Changed fast-gov feature to fast-mode feature.
  • Introduced instant-mode for Integration testing.
  • Removed try-runtime + benchmark features from integration tests

Treasury Config:

  • ApproveOrigin: Either root or unanimous council vote. So either council can directly approve or can be approved via governance.
  • RejectOrigin: Either root or >1/2 (5/9) vote. Council majority can reject treasury proposal.

@vstam1 vstam1 changed the base branch from main to gov-release-branch January 29, 2024 16:03
@vstam1 vstam1 changed the base branch from gov-release-branch to main January 29, 2024 16:06
@vstam1 vstam1 changed the base branch from main to gov-release-branch January 29, 2024 16:06
@vstam1 vstam1 changed the base branch from gov-release-branch to main January 30, 2024 08:47
@vstam1 vstam1 marked this pull request as ready for review January 30, 2024 08:47
@JuaniRios JuaniRios changed the base branch from main to gov-release-branch January 30, 2024 14:19
@JuaniRios JuaniRios changed the base branch from gov-release-branch to main January 30, 2024 14:20
Signed-off-by: Juan Ignacio Rios <juani.rios.99@gmail.com>
pallets/democracy/src/weights.rs Outdated Show resolved Hide resolved
pallets/democracy/README.md Outdated Show resolved Hide resolved
@JuaniRios
Copy link
Contributor

We only fixed the tests to use the new fungible API, but didn't actually write a test where we reserve an amount, and can still vote on that right?

I saw the governance tests on integration-tests but didn't see us using a reserve and then voting, or a reserve and then electing

@vstam1
Copy link
Collaborator Author

vstam1 commented Jan 31, 2024

We only fixed the tests to use the new fungible API, but didn't actually write a test where we reserve an amount, and can still vote on that right?

I saw the governance tests on integration-tests but didn't see us using a reserve and then voting, or a reserve and then electing

Important thing to note it that we only use reserves in specific places (deposit for treasury proposal for example) and maybe use Holds. We know that Holds and locks/freezes can overlap. Also locks and freezes can be set after reserves, only reserve cannot be used on same tokens that are already locked. However, I will try to write some more comprehensive tests for the specific vote cases in combination with holds etc.

Copy link
Contributor

@lrazovic lrazovic left a comment

Choose a reason for hiding this comment

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

Few minor changes needed, but overall looks fine. As soon we are aligned on the runtime config, we can merge this.

runtimes/base/src/lib.rs Outdated Show resolved Hide resolved
integration-tests/src/tests/governance.rs Show resolved Hide resolved
pallets/democracy/README.md Show resolved Hide resolved
pallets/democracy/README.md Outdated Show resolved Hide resolved
pallets/elections-phragmen/README.md Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
integration-tests/src/tests/build_spec.rs Show resolved Hide resolved
runtimes/base/src/custom_migrations.rs Outdated Show resolved Hide resolved
runtimes/base/src/custom_migrations.rs Outdated Show resolved Hide resolved
@lrazovic lrazovic self-requested a review February 1, 2024 08:51
@lrazovic
Copy link
Contributor

lrazovic commented Feb 1, 2024

Following our offline discussion, we have agreed to proceed with merging this.

@lrazovic lrazovic merged commit d8262d4 into main Feb 1, 2024
@lrazovic lrazovic mentioned this pull request Feb 6, 2024
@JuaniRios JuaniRios deleted the feature/plmc-398-add-treasury-pallet-to-base-runtime-should-like-the-one-in branch March 18, 2024 09:18
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

3 participants