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

Update to latest Substrate master.#353

Merged
gavofyork merged 45 commits intomasterfrom
kiz-update-sub-mater
Aug 12, 2019
Merged

Update to latest Substrate master.#353
gavofyork merged 45 commits intomasterfrom
kiz-update-sub-mater

Conversation

@kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Aug 6, 2019

Closes #221

TODO:

  • Fix all build / test errors.
  • FindAuthor.
  • Service and runtime are still using aura. Both should be replaced with babe now.
  • Integrate srml/im-online #342.
  • Fix initial authorities in chain_spec (needs quite some work).
  • system::CheckGenesis<Runtime>
  • fix and verify that a --dev chain works
    • compiles and imports blocks.
    • submit and process extrinsic
  • fix and verify that a alice/bob chain works
    • compiles and imports blocks.
    • submit and process extrinsic

cc @jacogr I am doing the testing with UI along the way but if you also want to give it a try would be great.

@parity-cla-bot

This comment has been minimized.

@parity-cla-bot

This comment has been minimized.

@kianenigma

This comment has been minimized.

@parity-cla-bot

This comment has been minimized.

@kianenigma
Copy link
Contributor Author

I will stall this for now halfway through integrating BABE until paritytech/substrate#3296 lands. There seems to a lot of assumptions made in the polkadot code about crypto concrete types which might be affected by the new key mgmt system.

@gavofyork gavofyork added the A3-in_progress Pull request is in progress. No review needed at this stage. label Aug 7, 2019
@gavofyork
Copy link
Member

@kianenigma will this include the content of #342 ?

@kianenigma
Copy link
Contributor Author

@kianenigma will this include the content of #342 ?

Ideally yes, It is on my todo list up top.

let authorities = try_fr!(api.authorities(&id));
// TODO: double check this. It makes sense bc other occurances of `authorities`
// has been refactored to this.
let authorities = try_fr!(api.validators(&id));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rphmeier is this okay?

/// that 32 bits may be multiplied with a balance in 128 bits without worrying about overflow.
pub type Balance = u128;

// TODO: I don't get why this was defined here. It was aura so I change it to babe but regardless of that
Copy link
Contributor Author

@kianenigma kianenigma Aug 7, 2019

Choose a reason for hiding this comment

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

yeah, I still agree with my note. If the crypto key type is defined inside the babe module and exported via babe::AuthorityId then all of them should have the same look. sth like:

pub type BabePair = babe::AuthorityPair;
pub type BabeId = babe::AuthorityId;

Copy link
Member

Choose a reason for hiding this comment

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

indeed.

timestamp = { package = "srml-timestamp", git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-master" }
treasury = { package = "srml-treasury", git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-master" }
version = { package = "sr-version", git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-master" }
babe = { package = "srml-babe", git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-master" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hate this myself. Should refactor them to:

  • babe crate should always point to the core portion
  • srml_babe should point to the srml portion.

let authorities = self.client.runtime_api().authorities(&id)?;
// TODO: This used to use AuraApi and now it uses validators from parachainHost, as
// recommended by Rob. Please double and triple check.
let authorities = self.client.runtime_api().validators(&id)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rphmeier any insights?

@kianenigma kianenigma 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 Aug 7, 2019
@rphmeier rphmeier 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 Aug 11, 2019
@rphmeier
Copy link
Contributor

I didn't manage to get an alice/bob chain validating after the key refactoring was put in. the chain spec needs a look taken at

@rphmeier
Copy link
Contributor

rphmeier commented Aug 12, 2019

I got alice authoring blocks. Didn't get her to connect to Bob, though. I'd suggest we merge this already and deal with anything else in a follow-up PR. There are other PRs blocked on unrelated changes within this.

Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

All good, for my part.

@gavofyork gavofyork merged commit af0d87a into master Aug 12, 2019
@gavofyork gavofyork deleted the kiz-update-sub-mater branch August 12, 2019 13:48
tomusdrw pushed a commit that referenced this pull request Mar 26, 2021
* verify justifications from runtime

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

Labels

A0-please_review Pull request needs code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change BlockNumber to u32

6 participants