Fetch `parity-common` crates from crates.io #9410

dvdplm · 2018-08-24T10:05:38Z

Also removes transaction-pool and trace-time which now are in parity-common (and published).

~~Blocked on crate ownership issues: plain_hasher (cc @debris)~~

~~Blocked on https://github.com/paritytech/ethabi/pull/117 (also see https://github.com/paritytech/ethabi/issues/118)~~

~~Blocked on paritytech/rust-rocksdb#16~~

sorpaas · 2018-08-24T11:38:41Z

I would worry about "atomic PR" issues, like the one we faced in #9257.

Have we considered the git submodule approach?

dvdplm · 2018-08-24T11:59:09Z

Have we considered the git submodule approach?

I for one really (really) dislike git submodules and I think once we're "over the hump" and all parity-common crates are published we'll be mostly fine. I think we overdid it a little on #9257 tbh.

Fix/update a few deps

* master: evmbin: Fix gas_used issue in state root mismatch and handle output better (#9418) Update hardcoded sync (#9421) Add block reward contract config to ethash and allow off-chain contracts (#9312) Private packets verification and queue refactoring (#8715) Update tobalaba.json (#9419) docs: add parity ethereum logo to readme (#9415) build: update rocksdb crate (#9414) Updating the CI system (#8765) Better support for eth_getLogs in light mode (#9186) Add update docs script to CI (#9219) `gasleft` extern implemented for WASM runtime (kip-6) (#9357) block view! removal in progress (#9397) Prevent sync restart if import queue full (#9381) nonroot CentOS Docker image (#9280) ethcore: kovan: delay activation of strict score validation (#9406)

* master: evmbin: escape newlines in json errors (#9458) use kvdb-* and parity-snappy crates from crates.io (#9441) Add EIP-1014 transition config flag (#9268) add tags for runner selection of build-linux jobs (#9451) Remove unused BlockStatus::Pending (#9447) ci: only include local paths in coverage script (except target) (#9437) Add POA Networks: Core and Sokol (#9413) docker: install missing dependencies in arm target dockerfiles (#9436) Random small cleanups (#9423)

registrar/src/registrar.rs

andresilva

LGTM! Just minor style grumbles. Feel free to ignore the missing newlines if you don't fancy it. Building a binary out of this branch just to perform a smoke test.

andresilva · 2018-09-04T15:13:36Z

ethcore/evm/src/interpreter/mod.rs

@@ -795,7 +795,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 						TWO_POW_96 => a >> 96,
 						TWO_POW_224 => a >> 224,
 						TWO_POW_248 => a >> 248,
-						_ => a.overflowing_div(b).0,
+						_ => a/b,


why is it changed?

@dvdplm Please change all those cases back. Div panics on default but we need overflowing behavior here, otherwise it breaks consensus.

It seems like these methods were removed when going from bigint to uint, which makes sense since for unsigned integers div and rem should never overflow.

I also had the same question though so it's better if @dvdplm confirms.

Yeah right. I think it's this: paritytech/parity-common#32

Confirmed, that's the PR. I think the reason they were there to begin with was an attempt to fulfill a trait that is no longer in rust (removed ~1.13?).

andresilva · 2018-09-04T15:17:38Z

ethcore/evm/src/interpreter/mod.rs

@@ -821,7 +821,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 				} else if a == min && b == !U256::zero() {
 					min
 				} else {
-					let c = a.overflowing_div(b).0;
+					let c = a/b;


andresilva · 2018-09-04T15:22:03Z

registrar/src/registrar.rs

@@ -52,7 +52,8 @@ impl Registrar {
 		};

 		let address_fetcher = self.registrar.functions().get_address();
-		let id = address_fetcher.input(keccak(key), DNS_A_RECORD);
+		let hashed_key : [u8; 32] = keccak(key).into();


let hashed_key: [u8; 32] = ...

andresilva · 2018-09-04T15:25:15Z

util/keccak-hasher/Cargo.toml

+hashdb = "0.2.1"
+plain_hasher = "0.2"


Can we add a newline at the end of this file?

andresilva · 2018-09-04T15:25:42Z

util/patricia-trie-ethereum/Cargo.toml

+[dev-dependencies]
+memorydb = "0.2.1"
+keccak-hash = "0.1.2"


Can we add a newline here as well?

andresilva · 2018-09-04T15:26:13Z

util/triehash-ethereum/Cargo.toml

-triehash = { git = "https://github.com/paritytech/parity-common" }
-ethereum-types = "0.3"
+triehash = { version = "0.2.3", features = ["ethereum"] }
+ethereum-types = "0.4"
 keccak-hasher = { path = "../keccak-hasher" }


Also newline at end of file?

debris

LGTM! Just a minor question about changes in interpreter

debris · 2018-09-04T15:30:44Z

ethcore/evm/src/interpreter/mod.rs

@@ -795,7 +795,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 						TWO_POW_96 => a >> 96,
 						TWO_POW_224 => a >> 224,
 						TWO_POW_248 => a >> 248,
-						_ => a.overflowing_div(b).0,
+						_ => a/b,


why is it changed?

sorpaas

Overflowing changes in EVM seems not right. I think we need to change them back before merging, otherwise it breaks consensus.

I still have some doubts on the approach here. It indeed feels a little bit harder right now to look into the dependency's code, which is something I need to do often. But that's certainly not "over-my-dead-body" and the solution looks fine. :)

However, @dvdplm one thing that we should do I think is to make sure all dependencies have proper version tags on parity-common repo or other places. I think we will soon have situations where the version we use in parity-ethereum is slightly older compared with parity-common/primitives/other repos. In that case, those tags will be crucial to look into the code for actual code used in parity-ethereum.

sorpaas · 2018-09-04T15:38:39Z

ethcore/evm/src/interpreter/mod.rs

@@ -795,7 +795,7 @@ impl<Cost: CostType> Interpreter<Cost> {
 						TWO_POW_96 => a >> 96,
 						TWO_POW_224 => a >> 224,
 						TWO_POW_248 => a >> 248,
-						_ => a.overflowing_div(b).0,
+						_ => a/b,


@dvdplm Please change all those cases back. Div panics on default but we need overflowing behavior here, otherwise it breaks consensus.

I was wrong about Div/Rem. They seems alright!

dvdplm · 2018-09-04T15:58:12Z

all dependencies have proper version tags on parity-common repo or other places.

I agree with this strongly and have slapped myself several times already for not having done this already. I will go through parity-common and make sure it's done and make sure others do the same. In parity-common I have to make tags like kvdb-v0.1 and it'll apply to the whole repo ofc but that's not a big problem I think.

andresilva · 2018-09-04T16:44:17Z

I built a binary from this branch and synced mainnet (my node was about 4000 blocks away from the chain head) and I also warp-synced on kovan.

Fetch parity-common crates from crates.io

e7cfd13

dvdplm self-assigned this Aug 24, 2018

dvdplm added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M5-dependencies 🖇 Dependencies. F5-task 🎬 Generic label for things that just have to get done but are no issues in first place. labels Aug 24, 2018

5chdn added this to the 2.1 milestone Aug 24, 2018

Add doc tests from patricia-trie to patricia-trie-ethereum

1cfb1ac

Fix/update a few deps

This was referenced Aug 25, 2018

Rename to parity-rocksdb and publish to crates paritytech/rust-rocksdb#16

Closed

Fetch parity-common dependencies from crates paritytech/substrate#617

Merged

dvdplm added 12 commits August 28, 2018 08:24

[ethkey] upgrade ethereum-types

e581603

[whisper] update deps

5f4503d

[network] deps

683472d

[network-devp2p] deps

dcee52f

[journaldb] deps

f1e2aed

[fastmap] deps

a6782cb

[miner] deps and test fixes

d7c276e

[machine] deps

77967b1

[json] deps

e43f580

[hw] deps

d329db2

[ethash] deps

02c9999

[registrar] deps

7fbc9a3

This was referenced Aug 28, 2018

Fix v5.1 rust-ethereum/ethabi#120

Closed

Backport fixes and update deps at v5.1 rust-ethereum/ethabi#121

Merged

dvdplm added 5 commits August 28, 2018 15:41

Update a few more dependencies with new ethabi-*

442fb56

[updater] Update deps

6ce1e66

deps

37c3528

[ethcore] Update deps

78b3b32

Use new parity-snappy and parity-rocksdb crates

e678b0b

dvdplm added 5 commits August 30, 2018 07:50

Updated submodules

b0acc34

Use parity-snappy 0.1

51d405b

Use kvdb-rocksdb 0.1.2

dd305a0

Don't use latest ethereum/tests

1c53be0

5chdn removed the F5-task 🎬 Generic label for things that just have to get done but are no issues in first place. label Aug 30, 2018

dvdplm added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Aug 30, 2018

dvdplm requested review from andresilva and sorpaas August 31, 2018 06:15

dvdplm added 2 commits September 4, 2018 09:45

Fix merge conflicts errors

14ed092

gnunicorn approved these changes Sep 4, 2018

View reviewed changes

registrar/src/registrar.rs Outdated Show resolved Hide resolved

Remove superseeded comment

cade563

andresilva suggested changes Sep 4, 2018

View reviewed changes

debris reviewed Sep 4, 2018

View reviewed changes

sorpaas previously requested changes Sep 4, 2018

View reviewed changes

Address grumbles: add newlines, add/remove spaces

bedc7fd

andresilva approved these changes Sep 4, 2018

View reviewed changes

dvdplm merged commit 72fd1fa into master Sep 4, 2018

5chdn deleted the dp/chore/fetch-dependencies-from-crates branch September 5, 2018 11:07

5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 5, 2018

5chdn mentioned this pull request Sep 12, 2018

Backports for 2.0.5 stable #9519

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch `parity-common` crates from crates.io #9410

Fetch `parity-common` crates from crates.io #9410

dvdplm commented Aug 24, 2018 •

edited

Loading

sorpaas commented Aug 24, 2018

dvdplm commented Aug 24, 2018

andresilva left a comment •

edited

Loading

andresilva Sep 4, 2018

debris Sep 4, 2018

sorpaas Sep 4, 2018 •

edited

Loading

andresilva Sep 4, 2018

andresilva Sep 4, 2018

sorpaas Sep 4, 2018

dvdplm Sep 4, 2018

andresilva Sep 4, 2018

andresilva Sep 4, 2018

andresilva Sep 4, 2018

andresilva Sep 4, 2018

andresilva Sep 4, 2018

debris left a comment

debris Sep 4, 2018

sorpaas left a comment •

edited

Loading

sorpaas Sep 4, 2018 •

edited

Loading

dvdplm commented Sep 4, 2018

andresilva commented Sep 4, 2018

		hashdb = "0.2.1"
		plain_hasher = "0.2"

Fetch parity-common crates from crates.io #9410

Fetch parity-common crates from crates.io #9410

Conversation

dvdplm commented Aug 24, 2018 • edited Loading

sorpaas commented Aug 24, 2018

dvdplm commented Aug 24, 2018

andresilva left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorpaas Sep 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

debris left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorpaas left a comment • edited Loading

Choose a reason for hiding this comment

sorpaas Sep 4, 2018 • edited Loading

Choose a reason for hiding this comment

dvdplm commented Sep 4, 2018

andresilva commented Sep 4, 2018

Fetch `parity-common` crates from crates.io #9410

Fetch `parity-common` crates from crates.io #9410

dvdplm commented Aug 24, 2018 •

edited

Loading

andresilva left a comment •

edited

Loading

sorpaas Sep 4, 2018 •

edited

Loading

sorpaas left a comment •

edited

Loading

sorpaas Sep 4, 2018 •

edited

Loading