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

updating v0.3 to use substrate v0.10 #146

Merged
merged 19 commits into from
Feb 25, 2019
Merged

Conversation

gterzian
Copy link
Contributor

Fix #133

@gterzian gterzian added the A3-in_progress Pull request is in progress. No review needed at this stage. label Feb 20, 2019
@gterzian
Copy link
Contributor Author

gterzian commented Feb 20, 2019

 --> service/src/lib.rs:150:1
    |
150 | / construct_service_factory! {
151 | |     struct Factory {
152 | |         Block = Block,
153 | |         RuntimeApi = RuntimeApi,
...   |
275 | |     }
276 | | }
    | |_^ the trait `sr_primitives::traits::ProvideRuntimeApi` is not implemented for `grandpa::import::GrandpaBlockImport<substrate_client_db::Backend<sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>>, client::LocalCallExecutor<substrate_client_db::Backend<sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>>, substrate_executor::native_executor::NativeExecutor<polkadot_executor::Executor>>, sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>, polkadot_runtime::RuntimeApi, client::Client<substrate_client_db::Backend<sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>>, client::LocalCallExecutor<substrate_client_db::Backend<sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>>, substrate_executor::native_executor::NativeExecutor<polkadot_executor::Executor>>, sr_primitives::generic::Block<sr_primitives::generic::Header<u64, sr_primitives::traits::BlakeTwo256, sr_primitives::generic::DigestItem<primitives::H256, primitives::Ed25519AuthorityId>>, polkadot_primitives::UncheckedExtrinsic>, polkadot_runtime::RuntimeApi>>`
    |
    = note: required because of the requirements on the impl of `substrate_consensus_common::import_queue::Verifier

Anyone can make-out what this means? @gnunicorn ?

@rphmeier
Copy link
Contributor

rphmeier commented Feb 20, 2019

We used to forward these traits on GrandpaBlockImport but it seems we don't anymore. Maybe we need to do that again upstream -- @sorpaas removed them because they weren't being used.

I think this has to do with the construction of AuraImportQueue -- the impl Verifier for AuraVerifier requires the generic param C there to be C: ProvideRuntimeApi, C::Api: BlockBuilderApi<B>. We really need the AuraImportQueue to import stuff through the GRANDPA worker, but since this is only for the Verifier, we should just be able to pass the client.

@rphmeier rphmeier changed the title updating to substrate v0.10 updating v0.3 to use substrate v0.10 Feb 20, 2019
@gterzian
Copy link
Contributor Author

gterzian commented Feb 21, 2019

@bkchr Would you have any pointers with regards to updating polkadot in the light of paritytech/substrate@53bf81e#diff-2d4fe59416e64524ebe5d49fb1cd82c2

For example, how to implement ProvideInherent for polkadot?

Also, Polkadot had an struct looking like:

InherentData {
			timestamp: self.believed_minimum_timestamp,
			parachains: candidates,
			aura_expected_slot: self.consensus_data.slot,
		};

How could I re-implement this using inherents::InherentData?

Other areas that needs to be updated are propose and propose_with

@bkchr
Copy link
Member

bkchr commented Feb 21, 2019

Polkadot is already using the new inherent data interface in master.

@gterzian gterzian added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Feb 21, 2019
{
type Error = Error;
type Create = Either<
CreateProposal<C, TxApi>,
future::FutureResult<Block, Error>,
>;

fn propose(&self, consensus_data: AuraConsensusData) -> Self::Create {
fn propose(&self, inherent_data: InherentData, max_duration: Duration) -> Self::Create {
Copy link
Contributor

Choose a reason for hiding this comment

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

@bkchr can you review this function to make sure the inherent data is backwards compatible with before?

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to produce blocks with the new node and the old runtime? If yes, then this is not compatible. For syncing, this code should not be touched.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks for the clarification. I think we can work around this. What would happen if the old code tried to author on the new runtime?

Copy link
Member

Choose a reason for hiding this comment

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

Create inherent will fail and thus the block production will fail.

@@ -49,6 +50,7 @@ std = [
"polkadot-primitives/std",
"parity-codec/std",
"parity-codec-derive/std",
"substrate-inherents/std",
Copy link
Contributor

Choose a reason for hiding this comment

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

funky indentation

let executor = task_executor.clone();

// spin up a task in the background that processes all incoming statements
// TODO: propagate statements on a timer?
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems in duplicate with the above comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Also, "no commit of TODO/FIXME without a Link" - I'd vote for removing that ToDo-comment at all)

// spin up a task in the background that processes all incoming statements
// TODO: propagate statements on a timer?
self.network
.with_spec(move |spec, ctx| {
Copy link
Contributor

Choose a reason for hiding this comment

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

this whole block could be moved up a level of indentation if we didn't do a newline on .with_spec

Ok(futures::Async::Ready(mut inner)) => {
let poll_result = inner.poll();
self.inner = Some(inner);
poll_result.map_err(|_| NetworkDown)
Copy link
Contributor

Choose a reason for hiding this comment

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

seems more resilient to return self.poll() here.

@@ -180,14 +191,30 @@ impl<P, E> Network for ConsensusNetwork<P,E> where
pub struct NetworkDown;

/// A future that resolves when a collation is received.
pub struct AwaitingCollation(::futures::sync::oneshot::Receiver<Collation>);
pub struct AwaitingCollation {
outer: ::futures::sync::oneshot::Receiver<::futures::sync::oneshot::Receiver<Collation>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend bringing Receiver into scope.

AwaitingCollation(
self.network.with_spec(|spec, _| spec.await_collation(relay_parent, parachain))
)
let (tx, rx) = ::futures::sync::oneshot::channel();
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps oneshot being in scope is reasonable.

let collation = spec.await_collation(relay_parent, parachain);
let _ = tx.send(collation);
});
AwaitingCollation{outer: rx, inner: None}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: AwaitingCollation { ... } (note the spaces)

Copy link
Contributor

@gnunicorn gnunicorn left a comment

Choose a reason for hiding this comment

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

A few minor fixes to address.

parachains: candidates,
aura_expected_slot: self.consensus_data.slot,
};
let mut inherent_data = self.inherent_data.take().expect("CreateProposal is not polled after finishing; qed");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's please wrap these...

spec.new_consensus(ctx, parent_hash, CurrentConsensus {
knowledge,
local_session_key,
});

MessageProcessTask {
let inner_stream = match rx.try_recv() {
Copy link
Contributor

Choose a reason for hiding this comment

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

why so complicated rather than a rx.try_recv().expect(...?

sr-version = { git = "https://github.com/paritytech/substrate", branch = "alexander-backports", default-features = false }
sr-std = { git = "https://github.com/paritytech/substrate", branch = "alexander-backports", default-features = false }
sr-primitives = { git = "https://github.com/paritytech/substrate", branch = "alexander-backports", default-features = false }
parity-codec = "3.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

3.0 still has std on per default but I don't think we want std in here...

Suggested change
parity-codec = "3.0"
parity-codec = { version = "3.0", default-features = false }

@@ -58,7 +58,7 @@ decl_storage! {
add_extra_genesis {
config(parachains): Vec<(ParaId, Vec<u8>, Vec<u8>)>;
config(_phdata): PhantomData<T>;
build(|storage: &mut sr_primitives::StorageMap, _: &mut ChildrenStorageMap, config: &GenesisConfig<T>| {
build(|storage: &mut sr_primitives::StorageOverlay, _: &mut ChildrenStorageOverlay, config: &GenesisConfig<T>| {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's wrap.

let executor = task_executor.clone();

// spin up a task in the background that processes all incoming statements
// TODO: propagate statements on a timer?
Copy link
Contributor

Choose a reason for hiding this comment

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

(Also, "no commit of TODO/FIXME without a Link" - I'd vote for removing that ToDo-comment at all)

primitives/Cargo.toml Outdated Show resolved Hide resolved
runtime/wasm/Cargo.toml Outdated Show resolved Hide resolved
runtime/wasm/Cargo.toml Outdated Show resolved Hide resolved
test-parachains/adder/collator/src/main.rs Outdated Show resolved Hide resolved
gterzian and others added 11 commits February 25, 2019 18:17
Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>
Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>
Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>
Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>
@gterzian
Copy link
Contributor Author

@rphmeier rphmeier added A8-looksgood and removed A0-please_review Pull request needs code review. labels Feb 25, 2019
@rphmeier rphmeier merged commit 461ea31 into paritytech:v0.3 Feb 25, 2019
@gterzian gterzian deleted the v0.3 branch February 26, 2019 02:36
rphmeier pushed a commit that referenced this pull request Mar 28, 2019
* Rebuild runtime

* Remove invalid value from chainspec (#68)

* service: use grandpa block import for locally sealed aura blocks (#85)

* bump version to v0.3.1

* Update lock file.

* limit number of transactions when building blocks (#91)

* Update to latest Substrate

* Bump to 0.3.2

* Actually bump.

* v0.3.2 (#98)

* bump substrate version

* fix polkadot-collator

* point to alexander-backports of substrate

* bump version

* cli: fix node shutdown (#100)

* update to latest substrate, change to v0.3.4

* update to latest substrate, bump version to 0.3.5

* v0.3.6

* try to build on every v0.3 commit and update alexander-backports

* bump to v0.3.7

* bump to 0.3.8

* Bump to 0.3.9: network and pruning improvements

* Bump to 0.3.10: reduce network bandwidth usage

* Use libp2p-kad 0.3.2 (#122)

* Bump libp2p-identify to 0.3.1 (#123)

* Bump to 0.3.12 (#127)

* Update Substrate again (#128)

* update substrate and bump version to v0.3.13

* bump version to v0.3.14: fix --reserved-nodes

* add a manually curated grandpa module (#136)

* updating v0.3 to use substrate v0.10 (#146)

* updating to latest substrate v0.10

* better handling of outer poll

* nit

* fix tests

* remove comment

* reduce indentation

* use self.poll

* bring oneshot into scope

* spaces

* wrap

* remove match

* wrap

* Update primitives/Cargo.toml

Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>

* Update runtime/wasm/Cargo.toml

Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>

* Update runtime/wasm/Cargo.toml

Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>

* Update test-parachains/adder/collator/src/main.rs

Co-Authored-By: gterzian <2792687+gterzian@users.noreply.github.com>

* indent

* add paranthese

* config: fix wrong ip for alexander bootnode (#161)

* fix curated-grandpa and rebuild wasm (#162)

* [v0.3] Integrates new gossip system into Polkadot (#166)

* new gossip validation in network

* integrate new gossip into service

* network: guard validation network future under exit signal (#168)

* bump version to v0.3.15: substrate v0.10

* [v0.3] update to substrate master (#175)

* update to substrate master

* fix test

* service: fix telemetry endpoints on alexander chainspec (#169) (#178)

* Update v0.3 to latest Substrate master (#177)

* update substrate v0.3 to latest master

* bump spec version

* update to latest master: remove fees module

* update runtime blobs

* bump version to 0.3.16

* replace sr25519 accountid with anysigner

* bump version to v0.3.17

* Some PoC-3 GRANDPA tweaks (#181)

* call on_finalise after triggering curated_grandpa change

* make grandpa rounds shorter for faster finalization

* use authorities when calculating duty roster (#185)

* [v0.3] Update to substrate master (#183)

* update to latest substrate master

* bump version to 0.3.18

* update to latest substrate master

* bump spec version

* update runtime wasm blobs

* remove current_offline_slash from chain spec

* update to substrate master: bump version to v0.3.19 (#188)

* update to substrate master: bump version to v0.3.19

libp2p network improvements

* network: replace NodeIndex with PeerId

* network: fix tests

* polkadot v0.3.20 (#190)

* update to substrate master: bump version to 0.3.20

* runtime: add offchain worker trait

* runtime: rebuild wasm blobs

* bump spec version (#191)

* Fix compilation

* Update version to 0.4.0

* Switch to use `polkadot-master` branch from substrate

* Remove unused struct

* Remove `grandpa::SyncedAuthorities` from `OnSessionChange`
tomusdrw pushed a commit that referenced this pull request Mar 26, 2021
Bumps [derive_more](https://github.com/JelteF/derive_more) from 0.99.7 to 0.99.8.
- [Release notes](https://github.com/JelteF/derive_more/releases)
- [Changelog](https://github.com/JelteF/derive_more/blob/master/CHANGELOG.md)
- [Commits](JelteF/derive_more@v0.99.7...v0.99.8)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
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.

5 participants