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

private-tx: replace error_chain#10510

Merged
soc1c merged 6 commits into
masterfrom
aj-private-tx-error
Mar 27, 2019
Merged

private-tx: replace error_chain#10510
soc1c merged 6 commits into
masterfrom
aj-private-tx-error

Conversation

@ascjones
Copy link
Copy Markdown
Contributor

@ascjones ascjones commented Mar 22, 2019

rel: #10302

  • Replaces error_chain with vanilla Error enum
  • Error::source manually implemented
  • From impls manually implemented
  • Display boilerplate derived using derive_more crate.

If we consider this to be a reasonable approach we can apply this to other errors.

@ascjones ascjones added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Mar 22, 2019
@soc1c soc1c added this to the 2.5 milestone Mar 22, 2019
@ascjones ascjones mentioned this pull request Mar 22, 2019
Closed
niklasad1
niklasad1 reviewed Mar 25, 2019
View reviewed changes
Comment thread ethcore/private-tx/src/encryptor.rs
}
let contract_address_signature = self.sign_contract_address(contract_address)?;
let requester = self.config.key_server_account.ok_or_else(|| ErrorKind::KeyServerAccountNotSet)?;
let requester = self.config.key_server_account.ok_or_else(|| Error::KeyServerAccountNotSet)?;
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 Mar 25, 2019

Choose a reason for hiding this comment

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

Unrelated to this PR but this is a needless closure should be ok_or instead Error::KeyServerAccountNotSet is just value

niklasad1
niklasad1 reviewed Mar 25, 2019
View reviewed changes
Comment thread ethcore/private-tx/src/encryptor.rs

if !response.is_success() {
bail!(ErrorKind::Encrypt(response.status().canonical_reason().unwrap_or("unknown").into()));
return Err(Error::Encrypt(response.status().canonical_reason().unwrap_or("unknown").into()));
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 Mar 25, 2019

Choose a reason for hiding this comment

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

Unrelated: unwrap_or_else

niklasad1
niklasad1 reviewed Mar 25, 2019
View reviewed changes
Comment thread ethcore/private-tx/src/encryptor.rs

fn sign_contract_address(&self, contract_address: &Address) -> Result<Signature, Error> {
let key_server_account = self.config.key_server_account.ok_or_else(|| ErrorKind::KeyServerAccountNotSet)?;
let key_server_account = self.config.key_server_account.ok_or_else(|| Error::KeyServerAccountNotSet)?;
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 Mar 25, 2019

Choose a reason for hiding this comment

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

Unrelated: ok_or

niklasad1
niklasad1 reviewed Mar 25, 2019
View reviewed changes
Comment thread ethcore/private-tx/src/lib.rs
// Create public transaction
let signer_account = self.signer_account.ok_or_else(|| ErrorKind::SignerAccountNotSet)?;
let state = self.client.state_at(BlockId::Latest).ok_or(ErrorKind::StatePruned)?;
let signer_account = self.signer_account.ok_or_else(|| Error::SignerAccountNotSet)?;
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 Mar 25, 2019

Choose a reason for hiding this comment

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

Unrelated: ok_or

niklasad1
niklasad1 reviewed Mar 25, 2019
View reviewed changes
Comment thread ethcore/private-tx/src/private_transactions.rs
/// Adds received signature for the stored private transaction
pub fn add_signature(&mut self, private_hash: &H256, signature: Signature) -> Result<(), Error> {
let desc = self.transactions.get_mut(private_hash).ok_or_else(|| ErrorKind::PrivateTransactionNotFound)?;
let desc = self.transactions.get_mut(private_hash).ok_or_else(|| Error::PrivateTransactionNotFound)?;
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 Mar 25, 2019

Choose a reason for hiding this comment

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

Unrelated: ok_or

@niklasad1 niklasad1 added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Mar 25, 2019
@ascjones ascjones mentioned this pull request Mar 26, 2019
Merged
@soc1c soc1c merged commit 7d26a82 into master Mar 27, 2019
@soc1c soc1c deleted the aj-private-tx-error branch March 27, 2019 13:46
ascjones added a commit that referenced this pull request Apr 1, 2019
@ascjones
private-tx: replace error_chain (#10510)
b63599e 
* 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 niklasad1 mentioned this pull request Apr 1, 2019
Merged
8 tasks
soc1c pushed a commit that referenced this pull request Apr 1, 2019
@niklasad1 @soc1c
2.4.4 more beta backports (#10554)
8f6911a 
* 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
ordian added a commit that referenced this pull request Apr 5, 2019
@ordian
Merge branch 'master' into ao-lmdb
075b8d4 
* master: (48 commits)
  ethcore: remove eth social and easthub chain configs (#10531)
  Initial support sccache for windows build (#10520)
  fix(light): make `OnDemand` generic instead of using the concrete type (#10514)
  private-tx: replace error_chain (#10510)
  Add trace information to eth_estimateGas (#10519)
  ethcore: add clique engine (#9981)
  verbose flag for cpp tests (#10524)
  Add a more realistic Batch test (#10511)
  docs: add changelogs for 2.3.{6,7,8} and 2.4.{1,2,3} (#10494)
  fix(rpc): fix a bunch of clippy lints (#10493)
  fix Sha3/keccak256 hash calculation for binaries (#10509)
  Add additional request tests (#10503)
  whisper/cli: add p2p port and ip parameters (#10057)
  fix(time-utils): add missing license (#10497)
  fix(extract `timestamp_checked_add` as lib) (#10383)
  fix(rpc): lint `unused_extern_crates` + fix warns (#10489)
  fix win&mac build (#10486)
  Сaching through docker volume (#10477)
  OpenBlock::new take IntoIterator instead of mutable ref to Iterator (#10480)
  simplify block module and usage (#10479)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@niklasad1 niklasad1 niklasad1 approved these changes

@ordian ordian ordian approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.

Projects

None yet

Milestone

2.5

Development

Successfully merging this pull request may close these issues.

4 participants

@ascjones @ordian @niklasad1 @soc1c