fix(rpc-types): replace uint and hash with `ethereum_types v0.4` #10217

niklasad1 · 2019-01-18T21:58:15Z

This PR removes the RPC wrapper types uint and hash because ethereum-types v0.4 implements serde. Also nice to remove all this conversions between ethereum-types to RpcTypes (I have not removed all those in this PR I will clean it up with another PR)

Most changes are imports with a few exceptions also I decided not to re-export ethereum-types instead each crate that is using them has to use ethereum-types instead!

niklasad1 · 2019-01-19T21:55:03Z

rpc/src/v1/helpers/subscribers.rs

@@ -37,7 +39,7 @@ impl str::FromStr for Id {
 }
 impl Id {
 	pub fn as_string(&self) -> String {
-		format!("0x{:?}", self.0)
+		format!("{:?}", self.0)


This should be changed to serde_json::to_string_pretty(&self.0) for performance reasons but I think it is out-of-scope of this PR because it is large enough!

niklasad1 · 2019-01-19T22:13:32Z

rpc/src/v1/traits/eth.rs

@@ -31,7 +31,7 @@ build_rpc_trait! {
 		#[rpc(name = "eth_protocolVersion")]
 		fn protocol_version(&self) -> Result<String>;

-		/// Returns an object with data about the sync status or false. (wtf?)
+		/// Returns an object with data about the sync status or false.


out-of-scope IMO but I couldn't resist.

we should not have such comments in the code base 🛩️

niklasad1 · 2019-01-19T22:16:46Z

rpc/src/v1/helpers/dispatch.rs

 					.map(ConfirmationResponse::Signature)
-					// TODO: is this correct? I guess the `token` is the wallet in this context


Out-of-scope from this PR but this comment was introduced by me for a long time ago and should be removed!

rpc/src/v1/traits/parity.rs

tomusdrw

lgtm, would be good to avoid the signature change of parity_verifySignature

rpc/src/v1/traits/parity.rs

seunlanlege · 2019-02-07T14:24:14Z

tons on conflicts. please rebase, would like to get this merged as soon as possible.

Most changes are imports with a few exceptions also I decided not to re-export ethereum-types instead each crate that is using them has to use ethereum-types instead!

not sure what this means

seunlanlege

Looks good

niklasad1 · 2019-02-07T15:30:30Z

Most changes are imports with a few exceptions also I decided not to re-export ethereum-types instead each crate that is using them has to use ethereum-types instead!

not sure what this means

I meant before we used the wrapper types and exported them from the rpc crate as for example use rpc::signer::U256. If you look at cli-signer crate it must explicitly import ethereum-types instead. Clear enough?

niklasad1 · 2019-02-07T16:55:39Z

This is PR is blocked because ethereum-types v0.4 contains the de-serialize bug that we recently fixed, see https://github.com/paritytech/primitives/blob/v0.4.0/serialize/src/lib.rs#L89

Cargo.lock

niklasad1 · 2019-02-13T10:48:07Z

@tomusdrw can you review this again? I think it is ready now, I bumped ethereum-types and added some tests in this codebase even if ethereum-types have those more or less !

soc1c · 2019-02-22T14:35:17Z

Needs a 2nd review 👍

cc @tomusdrw

tomusdrw

lgtm, two minor issues

rpc/src/v1/helpers/subscribers.rs

tomusdrw · 2019-02-22T15:07:11Z

rpc/src/v1/types/uint.rs

-}
-
-#[cfg(test)]
-mod tests {


Can we leave the tests to make sure that our assumptions are not broken when the library is updated? Also does 0.4 contain the fix for non-ascii characters?

Moved them to a new file rpc/src/v1/types/eth_types.rs) ok?

* revert change of `U64` -> `u64`

Use re-export from `ethereum-types` instead

) * fix(rpc-types): remove uint and hash wrappers * fix(tests) * fix(cleanup) * grumbles(rpc-api): revert `verify_signature` * revert change of `U64` -> `u64` * fix(cleanup after bad merge) * chore(bump ethereum-types) * fix(bad merge) * feat(tests ethereum-types): add tests * chore(update `ethereum-types` to 0.4.2) * feat(tests for h256) * chore(rpc): remove `ethbloom` import Use re-export from `ethereum-types` instead * fix(bad merge): remove `DefaultAccount` type * doc(add TODO with issue link)

* fix(rpc-types): replace uint and hash with `ethereum_types v0.4` (#10217) * fix(rpc-types): remove uint and hash wrappers * fix(tests) * fix(cleanup) * grumbles(rpc-api): revert `verify_signature` * revert change of `U64` -> `u64` * fix(cleanup after bad merge) * chore(bump ethereum-types) * fix(bad merge) * feat(tests ethereum-types): add tests * chore(update `ethereum-types` to 0.4.2) * feat(tests for h256) * chore(rpc): remove `ethbloom` import Use re-export from `ethereum-types` instead * fix(bad merge): remove `DefaultAccount` type * doc(add TODO with issue link) * chore(bump ethereum-types) (#10396) Fixes a de-serialization bug in `ethereum-tyes` * fix(light eth_gasPrice): ask network if not in cache (#10535) * fix(light eth_gasPrice): ask N/W if not in cache * fix(bad rebase) * fix(light account response): update `tx_queue` (#10545) * fix(bump dependencies) (#10540) * cargo update -p log:0.4.5 * cargo update -p regex:1.0.5 * cargo update -p parking_lot * cargo update -p serde_derive * cargo update -p serde_json * cargo update -p serde * cargo update -p lazy_static * cargo update -p num_cpus * cargo update -p toml # Conflicts: # Cargo.lock * tx-pool: check transaction readiness before replacing (#10526) * Update to vanilla tx pool error * Prevent a non ready tx replacing a ready tx * Make tests compile * Test ready tx not replaced by future tx * Transaction indirection * Use StateReadiness to calculate Ready in `should_replace` * Test existing txs from same sender are used to compute Readiness * private-tx: Wire up ShouldReplace * Revert "Use StateReadiness to calculate Ready in `should_replace`" This reverts commit af9e69c * Make replace generic so it works with private-tx * Rename Replace and add missing docs * ShouldReplace no longer mutable * tx-pool: update to transaction-pool 2.0 from crates.io * tx-pool: generic error type alias * Exit early for first unmatching nonce * Fix private-tx test, use existing write lock * Use read lock for pool scoring * fix #10390 (#10391) * private-tx: replace error_chain (#10510) * Update to vanilla tx pool error * private-tx: remove error-chain, implement Error, derive Display * private-tx: replace ErrorKind and bail! * private-tx: add missing From impls and other compiler errors * private-tx: use original tx-pool error * Don't be silly cargo

niklasad1 added the A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. label Jan 18, 2019

niklasad1 mentioned this pull request Jan 18, 2019

fix(rpc-types/hash): make Copy to unify with ethereum-types #10196

Closed

niklasad1 force-pushed the remove-rpc-wrappers branch from f339865 to fe9acb9 Compare January 19, 2019 20:48

niklasad1 commented Jan 19, 2019

View reviewed changes

niklasad1 force-pushed the remove-rpc-wrappers branch from e41b92e to c1a46de Compare January 19, 2019 22:19

niklasad1 commented Jan 19, 2019

View reviewed changes

rpc/src/v1/traits/parity.rs Outdated Show resolved Hide resolved

niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. M6-rpcapi 📣 RPC API. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Jan 19, 2019

niklasad1 added this to the 2.4 milestone Jan 19, 2019

niklasad1 changed the title ~~fix(rpc-types): remove uint and hash wrappers infavor of ethereum_types~~ fix(rpc-types): replace uint and hash with ethereum_types v0.4 Jan 20, 2019

tomusdrw reviewed Jan 21, 2019

View reviewed changes

rpc/src/v1/traits/parity.rs Outdated Show resolved Hide resolved

niklasad1 force-pushed the remove-rpc-wrappers branch from 5dfc54a to 54eb384 Compare January 23, 2019 22:57

seunlanlege approved these changes Feb 7, 2019

View reviewed changes

niklasad1 added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 7, 2019

niklasad1 force-pushed the remove-rpc-wrappers branch 3 times, most recently from e757506 to 550f8f1 Compare February 12, 2019 16:53

niklasad1 commented Feb 12, 2019

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

niklasad1 force-pushed the remove-rpc-wrappers branch from 550f8f1 to 6da3cec Compare February 13, 2019 09:42

niklasad1 added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Feb 13, 2019

5chdn modified the milestones: 2.5, 2.6 Feb 21, 2019

niklasad1 force-pushed the remove-rpc-wrappers branch 2 times, most recently from 940d4b2 to f319a68 Compare February 21, 2019 15:14

soc1c requested a review from tomusdrw February 22, 2019 14:35

tomusdrw approved these changes Feb 22, 2019

View reviewed changes

niklasad1 added 13 commits February 24, 2019 22:50

fix(rpc-types): remove uint and hash wrappers

acc0545

fix(tests)

e766a40

fix(cleanup)

3ce5144

grumbles(rpc-api): revert verify_signature

0529056

* revert change of `U64` -> `u64`

fix(cleanup after bad merge)

390070b

chore(bump ethereum-types)

6801c5e

fix(bad merge)

2a1c854

feat(tests ethereum-types): add tests

f2ddacb

chore(update ethereum-types to 0.4.2)

6aa654f

feat(tests for h256)

5b30381

chore(rpc): remove ethbloom import

6b7d773

Use re-export from `ethereum-types` instead

fix(bad merge): remove DefaultAccount type

8db4d7b

doc(add TODO with issue link)

3ff03c8

niklasad1 force-pushed the remove-rpc-wrappers branch from f319a68 to 3ff03c8 Compare February 24, 2019 22:03

niklasad1 merged commit c5c3fb6 into master Feb 25, 2019

niklasad1 deleted the remove-rpc-wrappers branch February 25, 2019 13:27

niklasad1 mentioned this pull request Mar 20, 2019

fix(rpc): fix a bunch of clippy lints #10493

Merged

niklasad1 mentioned this pull request Apr 1, 2019

2.4.4 more beta backports #10554

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rpc-types): replace uint and hash with `ethereum_types v0.4` #10217

fix(rpc-types): replace uint and hash with `ethereum_types v0.4` #10217

niklasad1 commented Jan 18, 2019 •

edited

Loading

niklasad1 Jan 19, 2019 •

edited

Loading

niklasad1 Jan 19, 2019 •

edited

Loading

niklasad1 Jan 19, 2019

tomusdrw left a comment

seunlanlege commented Feb 7, 2019 •

edited

Loading

seunlanlege left a comment

niklasad1 commented Feb 7, 2019

niklasad1 commented Feb 7, 2019

niklasad1 commented Feb 13, 2019

soc1c commented Feb 22, 2019

tomusdrw left a comment

tomusdrw Feb 22, 2019

niklasad1 Feb 22, 2019 •

edited

Loading

tomusdrw Feb 24, 2019

		.map(ConfirmationResponse::Signature)
		// TODO: is this correct? I guess the `token` is the wallet in this context

fix(rpc-types): replace uint and hash with ethereum_types v0.4 #10217

fix(rpc-types): replace uint and hash with ethereum_types v0.4 #10217

Conversation

niklasad1 commented Jan 18, 2019 • edited Loading

niklasad1 Jan 19, 2019 • edited Loading

Choose a reason for hiding this comment

niklasad1 Jan 19, 2019 • edited Loading

Choose a reason for hiding this comment

niklasad1 Jan 19, 2019

Choose a reason for hiding this comment

tomusdrw left a comment

Choose a reason for hiding this comment

seunlanlege commented Feb 7, 2019 • edited Loading

seunlanlege left a comment

Choose a reason for hiding this comment

niklasad1 commented Feb 7, 2019

niklasad1 commented Feb 7, 2019

niklasad1 commented Feb 13, 2019

soc1c commented Feb 22, 2019

tomusdrw left a comment

Choose a reason for hiding this comment

tomusdrw Feb 22, 2019

Choose a reason for hiding this comment

niklasad1 Feb 22, 2019 • edited Loading

Choose a reason for hiding this comment

tomusdrw Feb 24, 2019

Choose a reason for hiding this comment

fix(rpc-types): replace uint and hash with `ethereum_types v0.4` #10217

fix(rpc-types): replace uint and hash with `ethereum_types v0.4` #10217

niklasad1 commented Jan 18, 2019 •

edited

Loading

niklasad1 Jan 19, 2019 •

edited

Loading

niklasad1 Jan 19, 2019 •

edited

Loading

seunlanlege commented Feb 7, 2019 •

edited

Loading

niklasad1 Feb 22, 2019 •

edited

Loading