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

Backporting to beta #3149

Merged
merged 25 commits into from Nov 3, 2016
Merged

Backporting to beta #3149

merged 25 commits into from Nov 3, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Nov 3, 2016

No description provided.

@arkpar arkpar added the A8-backport 🕸 Pull request is already reviewed well in another branch. label Nov 3, 2016
@rphmeier
Copy link
Contributor

rphmeier commented Nov 3, 2016

This looks like it includes the changes in next-hardfork -- we shouldn't backport any of those until they're in.

@arkpar arkpar added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Nov 3, 2016
@@ -0,0 +1,339 @@
// Copyright 2015, 2016 Ethcore (UK) Ltd.
Copy link
Contributor

Choose a reason for hiding this comment

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

Banning functionality shouldn't be making it into 1.3 series.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merg leftover. It is not even included in the build. Removed anyway

@gavofyork
Copy link
Contributor

looks like the banning queue functionality was accidentally included here. it's not part of the EIPs and shouldn't be making it into 1.3 series.

@gavofyork gavofyork added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. A8-backport 🕸 Pull request is already reviewed well in another branch. labels Nov 3, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling fb3f385 on backporting into * on beta*.

@gavofyork gavofyork added the M4-core ⛓ Core client code / Rust. label Nov 3, 2016
@@ -18,6 +18,6 @@ extern crate ethcore_ipc_codegen;

fn main() {
ethcore_ipc_codegen::derive_binary("src/types/mod.rs.in").unwrap();
ethcore_ipc_codegen::derive_ipc("src/client/traits.rs").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember changing this file...

@@ -15,12 +15,10 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

use util::numbers::*;
use ipc::{IpcConfig, BinaryConvertError};
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember changing this file

@@ -30,13 +30,17 @@ pub use self::test_client::{TestBlockChainClient, EachBlockWith};
pub use types::trace_filter::Filter as TraceFilter;
pub use executive::{Executed, Executive, TransactOptions};
pub use env_info::{LastHashes, EnvInfo};
pub use self::chain_notify::{ChainNotify, ChainNotifyClient};
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember these changes.

@@ -21,55 +21,6 @@
//! transaction's nonce and next nonce expected from this sender). If nonces are equal transaction's gas price is used
//! for comparison (higher gas price = higher priority).
//!
//! # Usage Example
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember removing all of this

@NikVolf
Copy link
Contributor

NikVolf commented Nov 3, 2016

@gavofyork you used Option<u8> in one of the new methods, and it cannot be serialized for ipc, so this ipc-related changes are backported from master where ipc codegen is not executed until feature is on (unlike it is in beta, where it is always executed)

@arkpar
Copy link
Collaborator Author

arkpar commented Nov 3, 2016

I've disabled IPC code generation in this PR because #2976 brakes it. IPC codegen is already disabled in master.

@@ -162,7 +162,8 @@ mod test {
#[test]
fn should_create_new_account() {
// given
let dir = env::temp_dir();
let mut dir = env::temp_dir();
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember changing this

@@ -49,7 +49,7 @@ include!(concat!(env!("OUT_DIR"), "/lib.rs"));
include!("lib.rs.in");

#[cfg(feature = "with-syntex")]
pub fn register(reg: &mut syntex::Registry) {
pub fn register_cleaner(reg: &mut syntex::Registry) {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remember changing this file.

@@ -34,7 +34,7 @@ pub struct HypervisorService {
check_list: RwLock<HashMap<IpcModuleId, bool>>,
}

#[derive(Ipc)]
#[ipc]
Copy link
Contributor

Choose a reason for hiding this comment

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

this and the similar changes following were not EIP related.

@@ -119,11 +118,6 @@ fn start() -> Result<String, String> {
fn main() {
// Always print backtrace on panic.
::std::env::set_var("RUST_BACKTRACE", "1");
// just redirect to the sync::main()
if std::env::args().nth(1).map_or(false, |arg| arg == "sync") {
Copy link
Contributor

Choose a reason for hiding this comment

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

not EIP related.

@@ -25,10 +25,6 @@ use self::ipc_deps::*;
use ethcore_logger::Config as LogConfig;

pub mod service_urls {
pub const CLIENT: &'static str = "ipc:///tmp/parity-chain.ipc";
Copy link
Contributor

Choose a reason for hiding this comment

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

not EIP related

pub network_id: U256,
pub network_id: usize,
/// Main "eth" subprotocol name.
pub subprotocol_name: [u8; 3],
Copy link
Contributor

@gavofyork gavofyork Nov 3, 2016

Choose a reason for hiding this comment

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

extra subprotocol_name field is from unrelated EXP PR, i think

@@ -49,7 +51,8 @@ impl Default for SyncConfig {
fn default() -> SyncConfig {
SyncConfig {
max_download_ahead_blocks: 20000,
network_id: U256::from(1),
network_id: 1,
subprotocol_name: *b"eth",
Copy link
Contributor

@gavofyork gavofyork Nov 3, 2016

Choose a reason for hiding this comment

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

subprotocol_name was from the EXP PR, i think

@@ -90,8 +93,6 @@ impl EthSync {
}
}

#[derive(Ipc)]
#[ipc(client_ident="SyncClient")]
Copy link
Contributor

Choose a reason for hiding this comment

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

not from the EIP PR

@@ -185,8 +186,6 @@ pub trait ManageNetwork : Send + Sync {
}


#[derive(Ipc)]
Copy link
Contributor

Choose a reason for hiding this comment

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

not from the EIP PR

@@ -91,7 +91,7 @@ mod api {
include!(concat!(env!("OUT_DIR"), "/api.rs"));
}

pub use api::{EthSync, SyncProvider, SyncClient, NetworkManagerClient, ManageNetwork, SyncConfig,
pub use api::{EthSync, SyncProvider, ManageNetwork, SyncConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

not from the EIP PR

@@ -1,11 +1,11 @@
#!/bin/sh
# Running Parity Full Test Sute

FEATURES="json-tests ipc"
FEATURES="json-tests"
Copy link
Contributor

Choose a reason for hiding this comment

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

not from the EIP PR


case $1 in
--no-json)
FEATURES="ipc"
FEATURES=""
Copy link
Contributor

Choose a reason for hiding this comment

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

not from the EIP PR

@gavofyork
Copy link
Contributor

gavofyork commented Nov 3, 2016

quite a few changes from the EXP and some IPC changes seem to have been brought in - was that intentional?

@gavofyork
Copy link
Contributor

seems the IPC changes were intentional - the partial EXP changes not so much?

@arkpar
Copy link
Collaborator Author

arkpar commented Nov 3, 2016

EXP support is already in beta. Looks like it was not properly merged before.

@gavofyork
Copy link
Contributor

fair enough

@gavofyork gavofyork closed this Nov 3, 2016
@gavofyork gavofyork reopened this Nov 3, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 3, 2016
@gavofyork gavofyork merged commit 6a4408c into beta Nov 3, 2016
@gavofyork gavofyork deleted the backporting branch November 3, 2016 21:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants