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

Modify RuntimeChange to support easily new additions #1637

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Dec 1, 2023

Description

Currently, adding a new change to RuntimeChange implies a lot of boilerplate. With From/TryInto methods and duplicate the work for fast::RuntimeChange version. This PR tries to ease the process.

  • Added a macro to add new From/TryInto methods
  • fast::RuntimeChange type has benn removed, and now RuntimeChange allows itself to be parameterized with an Options type that gives it a specific behavior, i.e. a fast version of RuntimeChange.
  • Moved to its own changes module.

Required by:

@lemunozm lemunozm added I3-annoyance The code behaves as expected, but "expected" is an issue. P7-asap Issue should be addressed in the next days. I6-refactoring Code needs refactoring. labels Dec 1, 2023
@lemunozm lemunozm self-assigned this Dec 1, 2023

/// A change done in the runtime, shared between pallets
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub enum RuntimeChange<T: Changeable, Options: Clone = ()> {
Copy link
Contributor Author

@lemunozm lemunozm Dec 1, 2023

Choose a reason for hiding this comment

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

This is the key. If none option is passed (RuntimeChange<T>), the behaviour is the expected one. If FastDelay option is passed (RuntimeChange<T, FastDelay>) the behaviour is hacked to be faster. But both are the same generic type and implementations for them are shared

Copy link
Collaborator

@mustermeiszer mustermeiszer left a comment

Choose a reason for hiding this comment

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

I love it! Great idea, really, really nice!

Comment on lines +63 to +78
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub struct FastDelay;

impl<T: Changeable> From<RuntimeChange<T, FastDelay>> for PoolChangeProposal {
fn from(runtime_change: RuntimeChange<T, FastDelay>) -> Self {
let new_requirements = runtime_change
.requirement_list()
.into_iter()
.map(|req| match req {
Requirement::DelayTime(_) => Requirement::DelayTime(60), // 1 min
req => req,
});

PoolChangeProposal::new(new_requirements)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats great!

@lemunozm lemunozm enabled auto-merge (squash) December 1, 2023 08:18
@lemunozm lemunozm merged commit b8cefb4 into main Dec 1, 2023
8 checks passed
@lemunozm
Copy link
Contributor Author

lemunozm commented Dec 1, 2023

@wischli for reference for pool fees, this is how it looks now the addition of a new runtime change variant: https://github.com/centrifuge/centrifuge-chain/pull/1629/files#diff-617c568adf46d885f9ea8e1a77de91c3e9987002c2bdc0e03a084ddb69a7d65a

Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Thanks a ton for this incredible change! The boilerplate setup difference is ridiculous now: A few lines and that's it. Awesome work @lemunozm 🎉

@lemunozm lemunozm deleted the runtime-change-easy-support branch December 1, 2023 10:19
wischli added a commit that referenced this pull request Dec 1, 2023
wischli added a commit that referenced this pull request Jan 23, 2024
* chore: init blank pool fees pallet

* chore: apply workspace #1609 to pool-fees dummy

* fix: docker-compose

* feat: add changeguard pool fees support

* wip: prepare payment logic

* refactor: use reserve instead of nav

* chore: add extrinsics

* feat: prep + pay disbursements

* refactor: Apply fees ChangeGuard to #1637

* feat: add pool fees to all runtimes

* feat: add fees pool registration

* fix: existing pool unit tests

* fix: existing integration tests

* docs: add pool fee types

* wip: init fees unit tests

* tests: extrinsics wip

* chore: add events

* tests: add pool fees unit tests

* fix: support retroactive disbursements

* refactor: add epoch transition hook

* refactor: add pool fee prefix to types

* refactor: remove redundand trait bounds

* wip: pool system integration tests

* refactor: move portfolio valuation from loans to cfg-types

* chore: add pool fee account id

* wip: pool fee nav

* wip: fix uts

* wip: fix apply review by @lemunozm

* fix: issues after rebase

* tests: add saturated_proration

* refactor: simplify pool fee amounts

* chore: aum + fix fees UTs

* chore: apply AUM to pool-system

* fix: remove AUM coupling in PoolFees

* fix: transfer on close, unit tests

* fix: use total nav

* fix: taplo

* fix: fee calc on nav closing

* feat: impl TimeAsSecs for timestamp mock

* fix: test on_closing instead of update_active_fees

* fix: clippy

* tests: fix + add missing pool fees

* refactor: make update fees result instead of void

* tests: add insufficient resource in p-system

* bench: add pool fees, apply to system + registry

* fix: tests

* refactor: explicitly use Seconds in FeeAmountProration impl

* docs: add PoolFeeAmount and NAV update

* refactor: update NAV, total assets after review from @mustermeiszer

* fix: clippy

* refactor: Add PoolFeePayable

* fix: clippy

* fix: correct epoch execution with fees (#1695)

* fix: correct epoch execution with fees

* refactor: use new nav syntax

* tests: fix auto epoch closing

* feat: epoch execution migration

* chore: add epoch migration to altair

---------

Co-authored-by: William Freudenberger <w.freude@icloud.com>

---------

Co-authored-by: Guillermo Perez <gpmayorga@users.noreply.github.com>
Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>
wischli added a commit that referenced this pull request Jan 29, 2024
* chore: init blank pool fees pallet

* chore: apply workspace #1609 to pool-fees dummy

* fix: docker-compose

* feat: add changeguard pool fees support

* wip: prepare payment logic

* refactor: use reserve instead of nav

* chore: add extrinsics

* feat: prep + pay disbursements

* refactor: Apply fees ChangeGuard to #1637

* feat: add pool fees to all runtimes

* feat: add fees pool registration

* fix: existing pool unit tests

* fix: existing integration tests

* docs: add pool fee types

* wip: init fees unit tests

* tests: extrinsics wip

* chore: add events

* tests: add pool fees unit tests

* fix: support retroactive disbursements

* refactor: add epoch transition hook

* refactor: add pool fee prefix to types

* refactor: remove redundand trait bounds

* wip: pool system integration tests

* refactor: move portfolio valuation from loans to cfg-types

* chore: add pool fee account id

* wip: pool fee nav

* wip: fix uts

* wip: fix apply review by @lemunozm

* fix: issues after rebase

* tests: add saturated_proration

* refactor: simplify pool fee amounts

* chore: aum + fix fees UTs

* chore: apply AUM to pool-system

* fix: remove AUM coupling in PoolFees

* fix: transfer on close, unit tests

* fix: use total nav

* fix: taplo

* fix: fee calc on nav closing

* feat: impl TimeAsSecs for timestamp mock

* fix: test on_closing instead of update_active_fees

* fix: clippy

* tests: fix + add missing pool fees

* refactor: make update fees result instead of void

* tests: add insufficient resource in p-system

* bench: add pool fees, apply to system + registry

* fix: tests

* refactor: explicitly use Seconds in FeeAmountProration impl

* docs: add PoolFeeAmount and NAV update

* refactor: update NAV, total assets after review from @mustermeiszer

* fix: clippy

* refactor: Add PoolFeePayable

* fix: clippy

* fix: correct epoch execution with fees (#1695)

* fix: correct epoch execution with fees

* refactor: use new nav syntax

* tests: fix auto epoch closing

* feat: epoch execution migration

* chore: add epoch migration to altair

---------

Co-authored-by: William Freudenberger <w.freude@icloud.com>

---------

Co-authored-by: Guillermo Perez <gpmayorga@users.noreply.github.com>
Co-authored-by: Frederik Gartenmeister <mustermeiszer@posteo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I3-annoyance The code behaves as expected, but "expected" is an issue. I6-refactoring Code needs refactoring. P7-asap Issue should be addressed in the next days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants