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

Parity Ethereum 2.0.0 #9052

Merged
merged 19 commits into from
Jul 11, 2018
Merged

Parity Ethereum 2.0.0 #9052

merged 19 commits into from
Jul 11, 2018

Conversation

5chdn
Copy link
Contributor

@5chdn 5chdn commented Jul 5, 2018 •
edited
Loading

  • rename parity crate to parity-ethereum
  • bump version to 2.0.0

@5chdn 5chdn added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Jul 5, 2018
@5chdn 5chdn added this to the 1.12 milestone Jul 5, 2018
@5chdn 5chdn added the B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. label Jul 5, 2018
@debris
Copy link
Collaborator

debris commented Jul 5, 2018 •
edited
Loading

does 2.0.0 mean that we can break backwards compatibility? 🤔

@5chdn
Copy link
Contributor Author

5chdn commented Jul 5, 2018

does 2.0.0 mean that we can break backwards compatibility?

hehe, go ahead.

@5chdn 5chdn closed this Jul 5, 2018
@5chdn 5chdn reopened this Jul 5, 2018
5chdn
5chdn commented Jul 5, 2018
View reviewed changes
ethcore/sync/src/chain/propagator.rs Outdated
@@ -48,7 +48,7 @@ fn accepts_service_transaction(client_id: &str) -> bool {
// Parity versions starting from this will accept service-transactions
const SERVICE_TRANSACTIONS_VERSION: (u32, u32) = (1u32, 6u32);
// Parity client string prefix
const PARITY_CLIENT_ID_PREFIX: &'static str = "Parity/v";
const PARITY_CLIENT_ID_PREFIX: &'static str = "Parity-Ethereum/v";

if !client_id.starts_with(PARITY_CLIENT_ID_PREFIX) {
Copy link
Contributor Author

@5chdn 5chdn Jul 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Are we checking compatibility via version string here? If yes, do you want me to check for both?

const LEGACY_CLIENT_ID_PREFIX: &'static str = "Parity/v";
const PARITY_CLIENT_ID_PREFIX: &'static str = "Parity-Ethereum/v";
if !client_id.starts_with(LEGACY_CLIENT_ID_PREFIX) && !client_id.starts_with(PARITY_CLIENT_ID_PREFIX) {
    // ...
}

Ref #4215 CC @svyatonik

@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jul 5, 2018
dvdplm
dvdplm approved these changes Jul 10, 2018
View reviewed changes
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm with minor whitespace grumbles.

ethcore/sync/src/chain/propagator.rs Outdated
return false;
}
if client_id.starts_with(LEGACY_CLIENT_ID_PREFIX) {
let ver: Vec<u32> = client_id[LEGACY_CLIENT_ID_PREFIX.len()..].split('.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line should be indented (and the 4 following lines as well).

ethcore/sync/src/chain/propagator.rs Outdated
.filter_map(|s| s.parse().ok())
.collect();
ver.len() == 2 && (ver[0] > SERVICE_TRANSACTIONS_VERSION.0 || (ver[0] == SERVICE_TRANSACTIONS_VERSION.0 && ver[1] >= SERVICE_TRANSACTIONS_VERSION.1))
} else if client_id.starts_with(PARITY_CLIENT_ID_PREFIX) {
let ver: Vec<u32> = client_id[PARITY_CLIENT_ID_PREFIX.len()..].split('.')
Copy link
Collaborator

Choose a reason for hiding this comment

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

indentation like above

ngotchac
ngotchac reviewed Jul 10, 2018
View reviewed changes
ethcore/sync/src/chain/propagator.rs Outdated
.collect();
ver.len() == 2 && (ver[0] > SERVICE_TRANSACTIONS_VERSION.0 || (ver[0] == SERVICE_TRANSACTIONS_VERSION.0 && ver[1] >= SERVICE_TRANSACTIONS_VERSION.1))
} else if client_id.starts_with(PARITY_CLIENT_ID_PREFIX) {
let ver: Vec<u32> = client_id[PARITY_CLIENT_ID_PREFIX.len()..].split('.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be cleaner to do the split in the ifs, and then the rest outside :

let split = if ... { ...split() } else if { ...split() } else { return false };
let ver: Vec<u32> = ...

@5chdn
Copy link
Contributor Author

5chdn commented Jul 10, 2018 •
edited
Loading

Ok, this finally works. Thanks @ngotchac!

@5chdn 5chdn requested a review from ngotchac July 10, 2018 17:45
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 11, 2018
@5chdn 5chdn merged commit 484ecfa into master Jul 11, 2018
@5chdn 5chdn deleted the a5-parity-ethereum branch July 11, 2018 11:35
@5chdn 5chdn mentioned this pull request Jul 12, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ngotchac ngotchac ngotchac approved these changes

@dvdplm dvdplm dvdplm approved these changes

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. M4-core ⛓ Core client code / Rust. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Milestone
2.0 Ethereum
Development

Successfully merging this pull request may close these issues.
4 participants
@5chdn @debris @dvdplm @ngotchac