This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Separate out staking module into balances and payment #629

Merged

gavofyork merged 19 commits into master from gav-sep-staking

Aug 30, 2018

Member

gavofyork commented Aug 29, 2018 •

edited

Loading

Does what it says on the tin. Closes #211.

Despite the apparent extra 300 lines, this actually removes quite a lot of code. With a cunning move of the PublicAux type from consensus into system, we have massively reduced the dependency tree of modules.

Now, we have system being the root dependency module: timestamp, consensus and balances are dependent on it. The entire on-chain governance and smart-contract system is dependent only on balances, while session-management is dependent on consensus and timestamp, and staking is dependent on session and balances. It's now possible to use balances within a PoA system without needing to bring in PoS concepts.

One upshot of this is the hugely simplified mockups for tests in democracy, contract and council modules. Since they are no longer dependent on consensus, session, timestamp and staking, all of those modules can be removed from the mock and everything becomes massively simpler.

99% of this PR is just moved code and renaming to correct references. The only significant code change is balances now including a hook IsAccountLiquid which allows staking to impose its Bonding semantics on the transfer function.

Needless to say, many storage items will have changed from Staking XXX to Balances XXX. UI maintainers take note. (cc @jacogr )


          Initial commit.

b8f6230

gavofyork added the A3-in_progress label

gavofyork added 9 commits

August 29, 2018 19:27


          Split out balances module

7a76f9b


          Minimise Balances trait requirements

8d5e964


          Fix up balances, remove balances stuff from staking

318fda2


          Split off and fix up staking module

5c79378


          Fix executive tests

f8f11a8


          Fix up democracy module

d7202c0


          make council work again

e82ec0d


          Remove unneeded cruft from democracy

fafaf19


          Fix up contract module

8fcee6e

gavofyork requested a review from pepyakin

August 29, 2018 20:28


          Fix up rest of tests

3c5f4a2

gavofyork added A0-please_review and removed A3-in_progress labels

gavofyork added 6 commits

August 29, 2018 23:31


          Fix minor TODOs

ed90aa1


          Fix tests

bfdf79d


          Remove superfluous code

7c8f8df


          Move offline inherents to consensus module.

Fixes #630


          Version needs Decode.


          Move Decode back

e9410f5

gavofyork requested a review from arkpar

August 30, 2018 12:20

pepyakin reviewed

View reviewed changes

Contributor

pepyakin left a comment

Cool! Few nits

substrate/runtime/balances/src/lib.rs Outdated

		@@ -0,0 +1,661 @@
		// Copyright 2017 Parity Technologies (UK) Ltd.
		// This file is part of Substrate Demo.

Contributor

pepyakin Aug 30, 2018

Could you change this to Substrate?

substrate/runtime/balances/src/lib.rs

+              #![cfg_attr(not(feature = "std"), no_std)]
+              #[cfg(feature = "std")]
+              extern crate serde;

Contributor

pepyakin Aug 30, 2018

This extern crate can be removed.

Member Author

gavofyork Aug 30, 2018

Sadly not - deriving Deserialize requires it to be around.

Contributor

pepyakin Aug 30, 2018

Hm, weird, before posting this, I --checked everything.

substrate/runtime/balances/src/lib.rs Outdated

+              extern crate substrate_runtime_io as runtime_io;
+              extern crate substrate_runtime_primitives as primitives;
+              extern crate substrate_runtime_consensus as consensus;
+              extern crate substrate_runtime_sandbox as sandbox;

Contributor

pepyakin Aug 30, 2018

This extern crate can be removed.

substrate/runtime/balances/src/lib.rs Outdated

+              extern crate substrate_primitives;
+              extern crate substrate_runtime_io as runtime_io;
+              extern crate substrate_runtime_primitives as primitives;
+              extern crate substrate_runtime_consensus as consensus;

Contributor

pepyakin Aug 30, 2018

This extern crate can be removed.

substrate/runtime/balances/src/lib.rs Outdated

+              extern crate substrate_runtime_primitives as primitives;
+              extern crate substrate_runtime_consensus as consensus;
+              extern crate substrate_runtime_sandbox as sandbox;
+              extern crate substrate_runtime_session as session;

Contributor

pepyakin Aug 30, 2018

This extern crate can be removed.

substrate/runtime/balances/src/lib.rs Outdated

+              		//
+              		// This balance is a "reserve" balance that other subsystems use in order to set aside tokens
+              		// that are still "owned" by the account holder, but which are unspendable. This is different
+              		// and wholly unrelated to the `Bondage` system used for staking.

Contributor

pepyakin Aug 30, 2018

Do we need this remark about staking and Bondage?

substrate/runtime/balances/src/lib.rs Outdated


		// PUBLIC DISPATCH

		/// Transfer some unlocked staking balance to another staker.

Contributor

pepyakin Aug 30, 2018

I think it's better to avoid terms stak{ing,er}

substrate/runtime/balances/src/lib.rs Outdated

+              		// This is the only balance that matters in terms of most operations on tokens. It is
+              		// alone used to determine the balance when in the contract execution environment. When this
+              		// balance falls below the value of `ExistentialDeposit`, then the "current account" is
+              		// deleted: specifically, `Bondage` and `FreeBalance`. Furthermore, `OnFreeBalanceZero` callback

Contributor

pepyakin Aug 30, 2018

Should we reword this comment? There is no such thing as Bondage now

substrate/runtime/council/Cargo.toml Show resolved Hide resolved

substrate/runtime/executive/Cargo.toml Show resolved Hide resolved


          Fix nits

750848e

Contributor

pepyakin commented Aug 30, 2018

Looks like #629 (comment) still not addressed


          Refactor to allow custom message

0fe3e87

Member Author

gavofyork commented Aug 30, 2018

Member Author

gavofyork commented Aug 30, 2018

pepyakin added the A8-looksgood label

pepyakin removed the A0-please_review label

pepyakin approved these changes

View reviewed changes

gavofyork merged commit 7c10a6c into master

gavofyork deleted the gav-sep-staking branch

August 30, 2018 16:43

gguoss pushed a commit to chainx-org/substrate that referenced this pull request


          Separate out staking module into balances and payment (paritytech#629)

846cf40

* Initial commit.

* Split out balances module

* Minimise Balances trait requirements

* Fix up balances, remove balances stuff from staking

* Split off and fix up staking module

* Fix executive tests

* Fix up democracy module

* make council work again

* Remove unneeded cruft from democracy

* Fix up contract module

* Fix up rest of tests

* Fix minor TODOs

* Fix tests

* Remove superfluous code

* Move offline inherents to consensus module.

Fixes paritytech#630

* Version needs Decode.

* Move Decode back

* Fix nits

* Refactor to allow custom message

dvdplm added a commit that referenced this pull request


          Merge remote-tracking branch 'origin/master' into dp/make-CallExecuto…

ef22f1c

…rs-generic-over-hasher-and-rlpcodec

* origin/master: (26 commits)
  Contract runtime polishing (#601)
  WIP on chain heap (#639)
  Events to track extrinsic success (#640)
  Install llvm-tools-preview component (#643)
  fix wasm executor compile error (#631)
  random fixes (#638)
  Empty becomes (), reflecting convention (#637)
  Allow to build_upon skipped entries, but don't walk back (#635)
  Separate out staking module into balances and payment (#629)
  Update .gitlab-ci.yml (#633)
  Do not attempt to rustup if in CI. This is taken care of by the base (#621)
  Avoid need for ident strings in storage (#624)
  rename to panic_handler as panic_implementation is deprecated in nightly (#626)
  5 random fixes (#2) (#623)
  fix one typo in README (#627)
  Misspelled words (#625)
  Contracts: Per block gas limit (#506)
  Make sure to ban invalid transactions. (#615) (#620)
  Forward-port BFT fixes from v0.2 and restructure agreement cancelling (#619)
  Allow specifying listening multiaddresses (#577)
  ...

dvdplm added a commit that referenced this pull request


          Merge branch 'master' into dp/chore/fetch-dependencies-from-crates

fd7dafb

* master: (22 commits)
  Introduce treasury and document (#646)
  Off-the-table staking preference (#656)
  Implement function `json_metadata` in `decl_module!` (#654)
  Fix warnings in networking (#652)
  Add a reputation system (#645)
  Check for pruned block state (#648)
  Contract runtime polishing (#601)
  WIP on chain heap (#639)
  Events to track extrinsic success (#640)
  Install llvm-tools-preview component (#643)
  fix wasm executor compile error (#631)
  random fixes (#638)
  Empty becomes (), reflecting convention (#637)
  Allow to build_upon skipped entries, but don't walk back (#635)
  Separate out staking module into balances and payment (#629)
  Update .gitlab-ci.yml (#633)
  Do not attempt to rustup if in CI. This is taken care of by the base (#621)
  Avoid need for ident strings in storage (#624)
  rename to panic_handler as panic_implementation is deprecated in nightly (#626)
  5 random fixes (#2) (#623)
  ...

lamafab pushed a commit to lamafab/substrate that referenced this pull request


          Update to substrate master (paritytech#629)

b3cfedf

* Update to substrate master

* Update service/src/lib.rs

Co-Authored-By: Bastian Köcher <[email protected]>

* bump subsrate

liuchengxu pushed a commit to chainx-org/substrate that referenced this pull request


          Fix duplicate key (paritytech#629)

75c8255

* Fix duplicate validator key

* Update wasm

* Update according from review

* Refactoring

* Build wasm

* Nit

* Update substrate

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet