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

Introduce Parathreads (runtime) #341

Merged
merged 76 commits into from
Oct 11, 2019
Merged

Introduce Parathreads (runtime) #341

merged 76 commits into from
Oct 11, 2019

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Jul 26, 2019

TODO

  • Upgrade to latest Substrate + Refactor key management substrate#3213 (using that branch and premerged with Update to latest Substrate #340 )
  • Make build
  • Fix tests
  • Add new tests for parathread and altered parachain functionality.
  • Introduce new modules and linkage to the runtime libs.rs
  • Refactor TypeId so it doesn't pull in sr_io and the memory management stuff.
  • Check that the accumulator logic (which uses the chain's state) isn't an issue for the tx queue otherwise we'll construct invalid blocks when there's more than the ThreadCount number of selection transactions waiting

ensure!(actual == head_data_hash, DispatchError::Stale);

// updated the selected threads.
selected_threads.insert(pos, (id, collator));
Copy link
Member Author

Choose a reason for hiding this comment

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

@tomusdrw don't know how well this will work with the current txqueue model... it assumes that we retain these changes in the candidate block's state as we populate our candidate block with txs from the tx queue.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand how it relates to the pool, but if you call validate not only during validate_transaction but also during apply_extrinsic the changes will indeed be retained.
However when we import extrinsics to the pool they will runt validate_transaction on a pristine state of latest block and the changes are obviously not persisted.

Is that what you had in mind or did I miss your point completely?

Copy link
Member Author

Choose a reason for hiding this comment

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

it should work ok like that, i think.

but generally, these transactions are a little more sophisticated than usual. there are several instances when they can become invalid:

  1. the head-data-hash given in them is not (or no longer) the hash of the parathread's head-data. this can happen because:
    a. it was old when we received it (should never be imported into pool)
    b. it was an alternative fork (should never be imported into pool)
    c. the parathread has progressed since we received it (should be thrown out as soon as this happens).
  2. two of them are for the same parathread with the same head-data-hash. in this case the one with the lower fee/priority should be discarded as the other will always supersede it.
  3. some number (registry::ThreadCount) of parathread progressions have already occurred in this block. in this case, the transactions should remain in the pool as long as their head-data-hash has not become outdated.

as i understand it, the pool should import them as long as the HDH is current, it will correctly construct blocks with them properly prioritised (this just uses the fees priority) and it will avoid putting in more than it should into a block.

to make it proactively discard duplicates, we presumably need to ensure that their provides includes the same tag, which should fulfil the needs for (2).

but in order to get (1c) working, we'd need to essentially re-check the validity after each block is imported. does the queue do this already?

@gavofyork gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jul 26, 2019
runtime/src/registrar.rs Outdated Show resolved Hide resolved
runtime/src/registrar.rs Outdated Show resolved Hide resolved
runtime/src/registrar.rs Outdated Show resolved Hide resolved
runtime/src/registrar.rs Outdated Show resolved Hide resolved
runtime/src/lib.rs Outdated Show resolved Hide resolved
///
/// Must be sent from a Signed origin that is able to have ParathreadDeposit reserved.
/// `code` and `initial_head_data` are used to initialize the parathread's state.
fn register_parathead(origin,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn register_parathead(origin,
fn register_parathread(origin,

Paras get(paras): map ParaId => Option<ParaInfo>;
}
add_extra_genesis {
config(parachains): Vec<(ParaId, Vec<u8>, Vec<u8>)>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use type aliases (as mentioned in previous comment https://github.com/paritytech/polkadot/pull/341/files#r308993502) to change Vec<u8>, Vec<u8> to something like CodeHash, InitialHeadData (if my assumption of what they represent is correct)?

Copy link
Contributor

@ltfschoen ltfschoen left a comment

Choose a reason for hiding this comment

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

various nitpicks

gavofyork and others added 10 commits October 9, 2019 15:54
Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>
Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>
Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>
@shawntabrizi
Copy link
Member

Some fixes to help this branch compile can be found here:
#470

Although, I ran into some issues and will need to talk to a rusty person.

@gavofyork
Copy link
Member Author

ensuring ordering of active_parachains matches expectations (the set_heads extrinsic expects them to be ordered), while on_finalized of registrar expects something else

Addressed in 78d6b53.

weight of deregistration is off - it is O(n)

It's part of a wider issue that existed before and i'm not sure if here is quite the right place to manage it.

storage leak of pending swap map (unless I missed something)

Fixed.

other nits and grumbles from myself and Shawn

Generally addressed.

* Some Fixes

* Fix queue_upward_messages

* Change back to const
Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

Looks good after re-review.
Fine with deferring the queue management to beyond this PR.

@rphmeier rphmeier changed the title Introduce Parathreads Introduce Parathreads (runtime) Oct 10, 2019
@gavofyork gavofyork merged commit 2213e91 into master Oct 11, 2019
@gavofyork gavofyork deleted the gav-parathreads branch October 11, 2019 06:27
gavofyork added a commit that referenced this pull request Oct 25, 2019
* add authority discovery module (#444)

* grandpa: set justification period to 512 blocks (#445)

* lock (#446)

* Kusama CC2 spec (#449)

* update readme for cc2 (#453)

* docs: add security policy (#450)

* docs: add security policy

* Update SECURITY.md

Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com>

* Enable `substrate-session` `std` feature and remove unused traits (#456)

* update (#457)

* Update latest substrate master (#462)

* update substrate to latest polkadot-master

* fix test runtime

* Fix compilation in wasm (#465)

Also fix the weird file structure by making `wasm_executor.rs` ->
`wasm_executor/mod.rs`.

* Make `ParachainContext` take self as mutable (#458)

This replicates changes to `Environment`/`Proposer` that are used
internally at Cumulus.

* wasm_executor: fix wasm signature checker (#471)

Signed-off-by: yjhmelody <465402634@qq.com>

* Changes for substrate#3699 (#466)

* change upstream and branch

* Fix build

* remove doc renamings

* Fix tests

* .

* Revert changing fork and branch

* Update Cargo.toml

* Update parachains.rs

* update branch name in Cargo.lock (#473)

* Introduce Parathreads (runtime) (#341)

* Rest of parathread draft implementation, parachain permissioning.

* Update Substrate

* Update Substrate again

* Integrate weight/fee stuff.

* Council

* Build fixes

* More fixes

* Minor additions

* fix some small errors

* Revert "fix some small errors"

This reverts commit 4fb52c8.

* Merge fix.

* do_swap -> on_swap

* Update depdendency to polkadot-master

* Fix more merge problems

* Some patching of errors

* Fix storage closure

* Actually fix storage. It builds!

* Tests run... but not successfully.

* Add `run_to_block` to get parachains active to start

* More `run_to_block`

* Fix build

* Queue up changes to threads

* Move registration test

* Fix regsiter/deregister test

* Retry queue.

* Minor refactor

* Refactor to avoid heavy storage items

* Make tests pass

* remove para on deregister, add events

* Remove println

* Fix register/deregister parathread test

* fix merge

* Parathread can be activated test

* Test auction

* Add `Debtors` storage item

I considered putting the debtor information in `ParaInfo`, but it did not make sense to me since this information only applies to parathreads, not `paras` in general.

* remove comment code

* Some new tests

* Fixes for removing threads when scheduled. Tests.

* Test progression of threads.

* Test that reschedule queuing works properly.

* Make test slightly more interesting

* whitespace

* Swap works properly.

* Update locks

* Build

* Rename can_swap

* Add test for funds to be correctly returned after a swap

Swap does not seem to have logic which correctly swaps the debtor account to the new parathread.

* Make tests consistant

* Add check that `PendingSwap` is cleaned up

* Update runtime/src/parachains.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

* Update runtime/src/registrar.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

* Some fixes/suggestions from review

* Docs

* Apply suggestions from code review

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>
Co-Authored-By: Shawn Tabrizi <shawntabrizi@gmail.com>

* Update network/src/gossip.rs

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

* Rename OnSwap

* Add missing `]`

* Rejig ordering semantics, making everything a bit slower but correct.

* Some Fixes to Parathread Compile (#470)

* Some Fixes

* Fix queue_upward_messages

* Change back to const

* Build fixes

* Fix tests

* Update to latest Substrate master (#472)

* Update to latest Substrate master

* Fix

* Fix compilation

* fix var name for post_upward_message (#474)

Signed-off-by: yjhmelody <465402634@qq.com>

* change versioning and tagging of releases (#476)

* change versioning and tagging of releases

* Make `ParaId` constructible from a const context (#483)

* Pass `client` and `task_executor` to `BuildParachainContext` (#484)

* Pass `client` and `task_executor` to `BuildParachainContext`

* Update `Cargo.lock`

* integrate minor weight/fee changes (#482)

* Update cargo files

* Make it build again.

* Fix build

* revert cargo file

* New lockfile

* Bump.

* Update to latest Substrate master (#486)

* Fixes required for Cumulus (#485)

* Collator node need to register all gossip validators as well

* Make sure that parachain authorities are only written once at genesis

* Fix test

* fix dockerfile build - upgrade image base from ubuntu 16 to 18 (#423)

* Only register one gossip validator for full nodes (#487)

* ci: fix publishing of ci builds (#488)

* Support `account_nextIndex` RPC. (#460)

* Use node-rpc extensions to support account_nextIndex.

* Remove todo.

* Update lock.

* Use new srml_system_rpc crate.

* Update to substrate=master

* Update lockfile.

* Update to polkadot-master.

* Apply suggestions from code review

Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>

* Update to latest substrate master (#491)

* update to latest substrate master

* Fix compilation

* Switch elections to Phragmen, enable them in PoA (#492)

* Switch elections to Phragmen, enable them in PoA

* Remove superfluous code.

* Build fixes

* Update to substrate master

* Build fixes

* Add warning

* Disable authority discovery for now

* Remove commented code

* Fix warning

* service: cleanup task spawning (#495)

* Update to latest substrate polkadot-master (#496)

* update to latest substrate polkadot-master

* Bump to 0.6.3

* service: don't use the grandpa observer (#494) (#498)

* service: don't use the grandpa observer

* service: remove unnecessary boxing

* service: fix indentation

* service: remove unnecessary on_exit

tasks spawned with `spawn_task`/`spawn_essential_task` are already
guarded by `on_exit`.

* Update service/src/lib.rs

Co-Authored-By: Gavin Wood <gavin@parity.io>
tomusdrw pushed a commit that referenced this pull request Mar 26, 2021
* renamed bin/node/runtime to bin/node/rialto-runtime

* replaced bridge-node-runtime references with rialto-runtime references

* separate folders for millau/rialto nodes+runtimes

* extracted pallet-shift-session-manager

* bridge-node -> bridge-node-runtime

* uninstall previous rust (temp solution???)

* fix dockerfile

* cargo fmt

* fix benchmarks check

* fix benchmarks again

* update LAST_RUST_UPDATE to clear the cache

* changed runtime comments

* move bin/node/* to bin/

* REVERT ME

* Revert "REVERT ME"

This reverts commit 7c335f946308ed11d9ed6ffec7c1c13dbe2743ed.

* specify container name

* REVERT ME

* container_name -> hostname

* fix typo

* aliases

* Revert "REVERT ME"

This reverts commit 0e74af5f8430f1975a3fc924d8b52079f266bda1.

* removed prefixes
imstar15 pushed a commit to imstar15/polkadot that referenced this pull request Aug 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants