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

Backports to beta #7660

Merged
merged 6 commits into from
Jan 23, 2018
Merged

Backports to beta #7660

merged 6 commits into from
Jan 23, 2018

Conversation

debris
Copy link
Collaborator

@debris debris commented Jan 22, 2018

andresilva and others added 6 commits January 22, 2018 11:30
* kvdb-rocksdb: update rust-rocksdb version

* kvdb-rocksdb: mark corruptions and attempt repair on db open

* kvdb-rocksdb: better corruption detection on open

* kvdb-rocksdb: add corruption_file_name const

* kvdb-rocksdb: rename mark_corruption to check_for_corruption
* Fixed delegatecall's from/to, closes #7166

* added tests for delegatecall traces, #7167
* Implement registrar.

* Implement eth_getCode

* Don't wait for providers.

* Don't wait for providers.

* Fix linting and wasm tests.
I was investigating issues I am having with Whisper support. I've
enabled Whisper on a custom test network and inserted traces into
Whisper handler implementation (Network<T> and NetworkProtocolHandler
for Network<T>) and I noticed that the handler was never invoked.

After further research on this matter, I found out that
AttachedProtocol's register function does nothing:
https://github.com/paritytech/parity/blob/master/sync/src/api.rs#L172
but there was an implementation originally:
99075ad#diff-5212acb6bcea60e9804ba7b50f6fe6ec and it did the actual
expected logic of registering the protocol in the NetworkService.

However, as of 16d84f8#diff-5212acb6bcea60e9804ba7b50f6fe6ec ("finished
removing ipc") this implementation is gone and only the no-op function
is left.

Which leads me to a conclusion that in fact Whisper's handler never gets
registered in the service and therefore two nodes won't communicate
using it.

Solution: Resurrect original non-empty `AttachedProtocols.register`
implementation

Resolves #7566
* Handle temporarily invalid blocks in sync.

* Fix tests.
@debris debris added A0-pleasereview 🤓 Pull request needs code review. A8-backport 🕸 Pull request is already reviewed well in another branch. labels Jan 22, 2018
@5chdn 5chdn added this to the 1.9 milestone Jan 22, 2018
@codecov-io
Copy link

codecov-io commented Jan 22, 2018

Codecov Report

❗ No coverage uploaded for pull request base (beta@97ed569). Click here to learn what that means.
The diff coverage is 84.43%.

Impacted file tree graph

@@           Coverage Diff           @@
##             beta    #7660   +/-   ##
=======================================
  Coverage        ?   81.45%           
=======================================
  Files           ?      714           
  Lines           ?    85323           
  Branches        ?      185           
=======================================
  Hits            ?    69502           
  Misses          ?    15821           
  Partials        ?        0
Impacted Files Coverage Δ
ethcore/evm/src/lib.rs 100% <ø> (ø)
ethcore/src/engines/instant_seal.rs 81.25% <ø> (ø)
parity/main.rs 100% <ø> (ø)
ethcore/light/src/net/request_set.rs 75.82% <ø> (ø)
ethcore/src/executed.rs 22.8% <ø> (ø)
ethcore/wasm/src/lib.rs 85.71% <ø> (ø)
ethcore/src/engines/tendermint/mod.rs 84.1% <ø> (ø)
json/src/spec/ethash.rs 99.24% <ø> (ø)
ethcore/src/verification/queue/mod.rs 89.72% <ø> (ø)
ipfs/src/lib.rs 90.9% <ø> (ø)
... and 203 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97ed569...44fa2cb. Read the comment docs.

@5chdn 5chdn added the M4-core ⛓ Core client code / Rust. label Jan 22, 2018
@5chdn 5chdn merged commit fa6a0a6 into beta Jan 23, 2018
@5chdn 5chdn deleted the backports_to_beta branch January 23, 2018 11:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. A8-backport 🕸 Pull request is already reviewed well in another branch. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants