Runtimes workspace dependencies & cleaning #1609

lemunozm · 2023-11-16T17:43:56Z

Description

Refactorizations and cleanings in the runtime part.

The first step to get this: #1380

Fixes ~~#1582~~

Changes and Descriptions

lemunozm · 2023-11-22T15:32:29Z

Currently cargo complains (with a warning) about workspace inheritance with renaming packages (see the issue). We can:

Life with the warning until it be (hopefully) solved.
Change codec to parity_scale_codec everywhere in our database

mustermeiszer · 2023-11-23T09:00:00Z

Cargo.toml

@@ -158,7 +352,7 @@ runtime-common = { path = "runtime/common" }
 frame-benchmarking = { git = "https://github.com/paritytech/substrate", optional = true, default-features = false, branch = "polkadot-v0.9.43" }
 frame-system-benchmarking = { git = "https://github.com/paritytech/substrate", optional = true, default-features = false, branch = "polkadot-v0.9.43" }
 # integration testing
-runtime-integration-tests = { path = "runtime/integration-tests", optional = true, default-features = false }
+#runtime-integration-tests = { path = "runtime/integration-tests", optional = true, default-features = false }


Why were we linking integration tests as a dependency of our node before?

I thought we needed it for some features. Fine then

mustermeiszer · 2023-11-23T09:01:47Z

runtime/integration-tests/src/utils/extrinsics.rs

-		#[cfg(not(feature = "runtime-development"))]
-		polkadot_runtime_common::claims::PrevalidateAttests::<RelayRuntime>::new(),


Why the removal?

Nongeneric tests are always run in CI under runtime-development, so this implies that that line is never compiled. In the case of other runtimes, we have now the generic part which should take care of that. Actually, we no longer have runtime-x features.

Please correct me if I'm missing something here, but CI will always tests against the development.

wischli

Thanks for cleaning up and making our future lives much easier! I have a few questions, overall LGTM.

wischli · 2023-11-23T08:59:24Z

Cargo.toml

+license = "LGPL-3.0"
+homepage = "https://centrifuge.io/"
+repository = "https://github.com/centrifuge/centrifuge-chain"
+documentation = "https://github.com/centrifuge/centrifuge-chain"


Maybe we want to point to our rustdocs?

Right! I thought I copied the correct link 🤦🏻‍♂️

wischli · 2023-11-23T09:00:09Z

Cargo.toml

@@ -158,7 +352,7 @@ runtime-common = { path = "runtime/common" }
 frame-benchmarking = { git = "https://github.com/paritytech/substrate", optional = true, default-features = false, branch = "polkadot-v0.9.43" }
 frame-system-benchmarking = { git = "https://github.com/paritytech/substrate", optional = true, default-features = false, branch = "polkadot-v0.9.43" }
 # integration testing
-runtime-integration-tests = { path = "runtime/integration-tests", optional = true, default-features = false }
+#runtime-integration-tests = { path = "runtime/integration-tests", optional = true, default-features = false }


Answer above to Frederik, we do not want integration tests as a dependency of our centrifuge node, right?

@lemunozm yeah good point but then we should remove these lines all together.

Right!

I'm in a new PR trying to move the node to its own folder using workspace dependencies. I'll do there.

wischli · 2023-11-23T09:00:21Z

Cargo.toml

@@ -222,7 +416,7 @@ std = [
  "pallet-pool-system/std",
  "polkadot-primitives/std",
  "runtime-common/std",
-  "runtime-integration-tests/std",
+  #"runtime-integration-tests/std",


Artifact? Plus the other commented out features of runtime-integration-tests below

related to the previous answer

wischli · 2023-11-23T09:01:57Z

runtime/altair/Cargo.toml

+  "serde/std",
+  "log/std",
+
+  # Substrate related


wischli · 2023-11-23T09:02:31Z

runtime/altair/Cargo.toml

 ]

 # A feature that should be enabled when the runtime should be build for on-chain
 # deployment. This will disable stuff that shouldn't be part of the on-chain wasm
 # to make it smaller like logging for example.
 on-chain-release-build = [
  "sp-api/disable-logging",
+  "runtime-common/on-chain-release-build",


Can you give some details about this addition?

Yes, I understand that if we compile altair with on-chain-release-build we would want the runtime common used to have that related feature also. Am I wrong?

wischli · 2023-11-23T09:08:39Z

scripts/runtime-weight-template.hbs

-//! Autogenerated weights for {{pallet}}
-//!
-//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION {{version}}
-//! DATE: {{date}}, STEPS: `{{cmd.steps}}`, REPEAT: {{cmd.repeat}}, LOW RANGE: `{{cmd.lowest_range_values}}`, HIGH RANGE: `{{cmd.highest_range_values}}`
-//! EXECUTION: {{cmd.execution}}, WASM-EXECUTION: {{cmd.wasm_execution}}, CHAIN: {{cmd.chain}}, DB CACHE: {{cmd.db_cache}}


Why can we remove the bench template?

We're not using it, if we do not put any template in the CLI command it uses the default. That it's how it's working right now.

@lemunozm

* 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 <[email protected]> --------- Co-authored-by: Guillermo Perez <[email protected]> Co-authored-by: Frederik Gartenmeister <[email protected]>

@lemunozm

* 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 <[email protected]> --------- Co-authored-by: Guillermo Perez <[email protected]> Co-authored-by: Frederik Gartenmeister <[email protected]>

lemunozm added Q5-hard Can be done by an experienced coder with a good knowledge of the codebase. I6-refactoring Code needs refactoring. dependencies Pull requests that update a dependency file. I11-cleaning No mandatory issue that leave the repo more readable/organized labels Nov 16, 2023

lemunozm requested a review from cdamian November 16, 2023 17:43

lemunozm self-assigned this Nov 16, 2023

lemunozm mentioned this pull request Nov 20, 2023

Polkadot v1.1.0 #1605

Closed

8 tasks

lemunozm added 12 commits November 23, 2023 07:27

move fees logic to its own module

15a6b39

runtime-common with exporting list and using workspace features

99c816d

polish runtime-common

11d016b

updating development-runtime

e307302

fix development compilation issue

4976210

development using workspace dependencies

942c706

altair using workspace dependencies

f00039a

build dependencies

ee24e9e

centrifuge using workspace dependencies

af3465e

integration tests with workspace dependencies

ea3cd17

fix integration testing

ec70592

taplo fmt

9fcd29e

lemunozm force-pushed the runtime-refactor branch from 46ef4d0 to 9fcd29e Compare November 23, 2023 06:37

lemunozm changed the title ~~Runtimes refactor & cleaning~~ Runtimes workspace dependencies & cleaning Nov 23, 2023

remove old template

969283b

lemunozm marked this pull request as ready for review November 23, 2023 07:37

lemunozm requested review from mustermeiszer, NunoAlexandre, wischli and hieronx as code owners November 23, 2023 07:37

remove pallet exporting list

2503670

mustermeiszer reviewed Nov 23, 2023

View reviewed changes

wischli reviewed Nov 23, 2023

View reviewed changes

lemunozm added 2 commits November 23, 2023 11:45

fix doc link

bd5f34a

Merge remote-tracking branch 'origin/main' into runtime-refactor

3107748

lemunozm enabled auto-merge (squash) November 23, 2023 13:50

mustermeiszer approved these changes Nov 23, 2023

View reviewed changes

lemunozm merged commit 5730d02 into main Nov 23, 2023
9 checks passed

lemunozm deleted the runtime-refactor branch November 23, 2023 15:08

wischli added a commit that referenced this pull request Nov 29, 2023

chore: apply workspace #1609 to pool-fees dummy

f7af9c2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Runtimes workspace dependencies & cleaning #1609

Runtimes workspace dependencies & cleaning #1609

lemunozm commented Nov 16, 2023 •

edited

Loading

lemunozm commented Nov 22, 2023

mustermeiszer Nov 23, 2023

lemunozm Nov 23, 2023

mustermeiszer Nov 23, 2023

mustermeiszer Nov 23, 2023

lemunozm Nov 23, 2023

wischli left a comment

wischli Nov 23, 2023

lemunozm Nov 23, 2023

lemunozm Nov 23, 2023

wischli Nov 23, 2023

lemunozm Nov 23, 2023

NunoAlexandre Nov 23, 2023

lemunozm Nov 23, 2023

wischli Nov 23, 2023

lemunozm Nov 23, 2023

wischli Nov 23, 2023

wischli Nov 23, 2023

lemunozm Nov 23, 2023 •

edited

Loading

wischli Nov 23, 2023

lemunozm Nov 23, 2023 •

edited

Loading

		#[cfg(not(feature = "runtime-development"))]
		polkadot_runtime_common::claims::PrevalidateAttests::<RelayRuntime>::new(),

Runtimes workspace dependencies & cleaning #1609

Runtimes workspace dependencies & cleaning #1609

Conversation

lemunozm commented Nov 16, 2023 • edited Loading

Description

Changes and Descriptions

lemunozm commented Nov 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wischli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lemunozm Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lemunozm Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

lemunozm commented Nov 16, 2023 •

edited

Loading

lemunozm Nov 23, 2023 •

edited

Loading

lemunozm Nov 23, 2023 •

edited

Loading