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

Properly handle check_epoch_end_signal errors #10015

Merged
merged 8 commits into from
Feb 7, 2019
Merged

Conversation

sorpaas
Copy link
Collaborator

@sorpaas sorpaas commented Dec 5, 2018

The function check_epoch_end_signal can fail, but currently its errors are silently ignored. This can cause other parts of engine to panic, and gets us issues like #9978. This PR moves check_epoch_end_signal out of commit_block so that its errors are properly handled.

When on error, the block is rejected. Nearly all times this is users' specification issue. With rejection, no further blocks will proceed thus allowing the user to rescue the chain without the need to kill the db.

@sorpaas sorpaas added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Dec 5, 2018
@sorpaas sorpaas added this to the 2.3 milestone Dec 5, 2018
Copy link
Contributor

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

LGTM

@ascjones ascjones added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 7, 2018
@5chdn
Copy link
Contributor

5chdn commented Jan 7, 2019

Conflict in client, and needs a 2nd review 👍

@5chdn 5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019
@5chdn
Copy link
Contributor

5chdn commented Jan 28, 2019

Could you have a look at this @ordian @debris ?

@ordian
Copy link
Collaborator

ordian commented Jan 28, 2019

@sorpaas please resolve conflicts

@5chdn
Copy link
Contributor

5chdn commented Jan 28, 2019

Does not compile

@5chdn 5chdn added B1-patch-beta 🕷🕷 B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jan 29, 2019
@5chdn 5chdn added the A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Feb 7, 2019
@5chdn
Copy link
Contributor

5chdn commented Feb 7, 2019

@sorpaas ?

@sorpaas
Copy link
Collaborator Author

sorpaas commented Feb 7, 2019

Sorry I missed this!

@sorpaas sorpaas removed the A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Feb 7, 2019
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Feb 7, 2019
@5chdn 5chdn merged commit 8fa56ad into master Feb 7, 2019
@5chdn 5chdn deleted the sp-commit-block-fail branch February 7, 2019 13:34
@5chdn 5chdn mentioned this pull request Feb 12, 2019
14 tasks
@5chdn 5chdn removed the B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. label Feb 12, 2019
@5chdn 5chdn mentioned this pull request Feb 12, 2019
18 tasks
5chdn pushed a commit that referenced this pull request Feb 12, 2019
* Make check_epoch_end_signal to only use immutable data

* Move check_epoch_end_signals out of commit_block

* Make check_epoch_end_signals possible to fail

* Actually return the error from check_epoch_end_signals

* Remove a clone

* Fix import error
5chdn added a commit that referenced this pull request Feb 13, 2019
* version: bump beta to 2.3.3

* import rpc transactions sequentially (#10051)

* import rpc transactions sequentially

* use impl trait in argument position, renamed ProspectiveDispatcher to WithPostSign

* grouped imports

* integrates PostSign with ProspectiveSigner

* fix spaces, removed unnecessary type cast and duplicate polling

* clean up code style

* Apply suggestions from code review

* Fix Windows build (#10284)

* Don't run the CPP example on CI (#10285)

* Don't run the CPP example on CI

* Add comment

* CI optimizations (#10297)

* CI optimizations

* fix stripping

* new dockerfile

* no need n submodule upd

* review

* moved dockerfile

* it becomes large

* onchain update depends on s3

* fix dependency

* fix cache status

* fix cache status

* new cache status

* fix publish job (#10317)

* fix publish job

* dashes and colonels

* Add Statetest support for Constantinople Fix (#10323)

* Update Ethereum tests repo to v6.0.0-beta.3 tag

* Add spec for St.Peter's / ConstantinopleFix statetests

* Properly handle check_epoch_end_signal errors (#10015)

* Make check_epoch_end_signal to only use immutable data

* Move check_epoch_end_signals out of commit_block

* Make check_epoch_end_signals possible to fail

* Actually return the error from check_epoch_end_signals

* Remove a clone

* Fix import error

* cargo: fix compilation

* fix(add helper for timestamp overflows) (#10330)

* fix(add helper timestamp overflows)

* fix(simplify code)

* fix(make helper private)

* Remove CallContract and RegistryInfo re-exports from `ethcore/client` (#10205)

* Remove re-export of `CallContract` and `RegistryInfo` from `ethcore/client`

* Remove CallContract and RegistryInfo re-exports again

This was missed while fixing merge conflicts

* fix(docker): fix not receives SIGINT (#10059)

* fix(docker): fix not receives SIGINT

* fix: update with reviews

* update with review

* update

* update

* snap: official image / test (#10168)

* official image / test

* fix / test

* bit more necromancy

* fix paths

* add source bin/df /test

* add source bin/df /test2

* something w paths /test

* something w paths /test

* add source-type /test

* show paths /test

* copy plugin /test

* plugin -> nil

* install rhash

* no questions while installing rhash

* publish snap only for release

* Don't add discovery initiators to the node table (#10305)

* Don't add discovery initiators to the node table

* Use enums for tracking state of the nodes in discovery

* Dont try to ping ourselves

* Fix minor nits

* Update timeouts when observing an outdated node

* Extracted update_bucket_record from update_node

* Fixed typo

* Fix two final nits from @todr

* Extract CallContract and RegistryInfo traits into their own crate (#10178)

* Create call-contract crate

* Add license

* First attempt at using extracted CallContract trait

* Remove unneeded `extern crate` calls

* Move RegistryInfo trait into call-contract crate

* Move service-transaction-checker from ethcore to ethcore-miner

* Update Cargo.lock file

* Re-export call_contract

* Merge CallContract and RegistryInfo imports

* Remove commented code

* Add documentation to call_contract crate

* Add TODO for removal of re-exports

* Update call-contract crate description

Co-Authored-By: HCastano <HCastano@users.noreply.github.com>

* Rename call-contract crate to ethcore-call-contract

* Remove CallContract and RegistryInfo re-exports from `ethcore/client` (#10205)

* Remove re-export of `CallContract` and `RegistryInfo` from `ethcore/client`

* Remove CallContract and RegistryInfo re-exports again

This was missed while fixing merge conflicts

* fixed: types::transaction::SignedTransaction; (#10229)

* fix daemonize dependency

* fix build

* change docker image based on debian instead of ubuntu due to the chan… (#10336)

* change docker image based on debian instead of ubuntu due to the changes of the build container

* role back docker build image and docker deploy image to ubuntu:xenial based (#10338)

* perform stripping during build (#10208)

* perform stripping during build (#10208)

* perform stripping during build

* var RUSTFLAGS
ordian added a commit that referenced this pull request Apr 5, 2019
* master:
  fix: parity-clib/examples/cpp/CMakeLists.txt (#10313)
  CI optimizations (#10297)
  Increase number of requested block bodies in chain sync (#10247)
  Deprecate account management (#10213)
  Properly handle check_epoch_end_signal errors (#10015)
  fix(osx and windows builds): bump parity-daemonize (#10291)
  Add missing step for  Using `systemd` service file (#10175)
  Call private contract methods from another private contract (read-only)  (#10086)
  update ring to 0.14 (#10262)
  fix(secret-store): deprecation warning (#10301)
  Update to jsonrpc-derive 10.0.2, fixes aliases bug (#10300)
  Convert to jsonrpc-derive, use jsonrpc-* from crates.io (#10298)
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.

5 participants