Reset blockchain properly #10669

seunlanlege · 2019-05-16T10:26:25Z

Previously the reset blockchain feature wrongly assumed that the block hashes were the keys for BlockDetails in COL_EXTRA that has been corrected.
Also, there was a known issue with the reset feature where deleted blocks wouldn't be imported again because they were a "known child" of an existing block
that has also been corrected.

Should fix #10665

parity-cla-bot · 2019-05-16T10:26:28Z

It looks like @seunlanlege signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

ethcore/src/client/client.rs

tomusdrw

Looks sane, any chance we could get a test for it? We should really try to make something to prevent from breaking it in the future again.

ethcore/src/client/client.rs

seunlanlege · 2019-05-20T18:39:07Z

@tomusdrw

please could you look at this again?

ethcore/blockchain/src/blockchain.rs

ethcore/src/client/client.rs

ordian · 2019-05-20T20:44:31Z

ethcore/src/client/client.rs

+			&**self.db.read().key_value(),
+			::db::COL_EXTRA,
+			&*best_block_hash
+		).ok_or("block was previously imported; best_block_details should exist; qed")?;


can this block be removed in the meantime?

don't understand the question.

We do schedule it for deletion in db_transaction above, but it''s not yet comitted, so the block indeed should be there.
However this is an Err not expect, so a proof is not even needed in that case.

Sorry I meant if the block could be deleted from a separate thread/process. Looks like in this case we're using rocksdb, which doesn't allow read/write from multiple processes and this method is only used in cli, so this is the only thread doing mutation, right? But if change our db backend to lmdb, this is something to keep in mind.

tomusdrw

Looks good to me after @ordian's comments are addressed.

ethcore/src/client/client.rs

tomusdrw · 2019-05-21T08:09:23Z

ethcore/src/client/client.rs

+			&**self.db.read().key_value(),
+			::db::COL_EXTRA,
+			&*best_block_hash
+		).ok_or("block was previously imported; best_block_details should exist; qed")?;


We do schedule it for deletion in db_transaction above, but it''s not yet comitted, so the block indeed should be there.
However this is an Err not expect, so a proof is not even needed in that case.

ordian · 2019-05-21T08:36:03Z

to fix cargo audit, merge with master

seunlanlege · 2019-05-21T13:29:59Z

ethcore/src/client/client.rs

-			.ok_or("Attempted to reset past genesis block")?;
+		let mut blocks_to_delete = Vec::with_capacity(num as usize);
+		let mut best_block_hash = self.chain.read().best_block_hash();
+		let mut batch = DBTransaction::with_capacity(blocks_to_delete.len());


damn, this should've been DBTransaction::with_capacity(blocks_to_delete.capacity()), shouldn't be much of a problem. this code runs in cli so the cost of resizing the buffer is negligible.

* master: Don't panic if extra_data is longer than VANITY_LENGTH (#10682) docs: evmbin - Update Rust docs (#10658) Remove annoying compiler warnings (#10679) Reset blockchain properly (#10669)

* master: add_sync_notifier in EthPubSubClient holds on to a Client for too long (#10689) Don't panic if extra_data is longer than VANITY_LENGTH (#10682) docs: evmbin - Update Rust docs (#10658) Remove annoying compiler warnings (#10679) Reset blockchain properly (#10669)

* delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions

* version: bump beta to 2.5.2 * [CI] allow cargo audit to fail (#10676) * [CI] allow cargo audit to fail * [.gitlab-ci.yml] add a comment about cargo audit * [Cargo.lock] cargo update -p protobuf * Reset blockchain properly (#10669) * delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions * new image (#10673) * Update publishing (#10644) * docker images are now built on k8s: test run * copy check_sync.sh in build-linux job * copy scripts/docker/hub/* in build-linux job * removed cache var * cleanup, no more nightly dockers * cleanup in dockerfile * some new tags * removed sccsche debug log, cleanup * no_gits, new artifacts dir, changed scripts. Test run. * define version once * one source for TRACK * stop kovan onchain updates * moved changes for two images to a new branch * rename Dockerfile * no need in libudev-dev * enable lto for release builds (#10717) * Use RUSTFLAGS to set the optimization level (#10719) * Use RUSTFLAGS to set the optimization level Cargo has a [quirk]() in how configuration settings are propagated when `cargo test` runs: local code respect the settings in `[profile.test]` but all dependencies use the `[profile.dev]` settings. Here we force `opt-level=3` for all dependencies. * Remove unused profile settings * Maybe like this? * Turn off incremental compilation * Remove colors; try again with overflow-checks on * Use quiet CI machine * Turn overflow checking back on * Be explicit about what options we use * Remove "quiet machine" override * ethcore: enable ECIP-1054 for classic (#10731) * config: enable atlantis on ethereum classic * config: enable atlantis on morden classic * config: enable atlantis on morden classic * config: enable atlantis on kotti classic * ethcore: move kotti fork block to 0xAEF49 * ethcore: move morden fork block to 0x4829BA * ethcore: move classic fork block to 0x81B320 * remove trailing comma * remove trailing comma * fix chainspec * ethcore: move classic fork block to 0x7fffffffffffffff

CremaFR · 2019-06-14T14:15:13Z

@soc1c It seems v2.4.7-stable does not include the fix.
v2.5.2-beta has it and reset blocks properly

* delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions

* ethcore/res: activate atlantis classic hf on block 8772000 (#10766) * fix docker tags for publishing (#10741) * merge-backports * Update version * remove clique engine from backports * Reset blockchain properly (#10669) * delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions * adds rpc error message for --no-ancient-blocks (#10608) * adds error message for --no-ancient-blocks, closes #10261 * Apply suggestions from code review Co-Authored-By: seunlanlege <[email protected]> * Treat empty account the same as non-exist accounts in EIP-1052 (#10775) * fix: aura don't add `SystemTime::now()` (#10720) This commit does the following: - Prevent overflow in `verify_timestamp()` by not adding `now` to found faulty timestamp - Use explicit `CheckedSystemTime::checked_add` to prevent potential consensus issues because SystemTime is platform depedent - remove `#[cfg(not(time_checked_add))]` conditional compilation * DevP2p: Get node IP address and udp port from Socket, if not included in PING packet (#10705) * get node IP address and udp port from Socket, if not included in PING packet * prevent bootnodes from being added to host nodes * code corrections * code corrections * code corrections * code corrections * docs * code corrections * code corrections * Apply suggestions from code review Co-Authored-By: David <[email protected]> * Revert "fix: aura don't add `SystemTime::now()` (#10720)" This reverts commit f104784. * Add a way to signal shutdown to snapshotting threads (#10744) * Add a way to signal shutdown to snapshotting threads * Pass Progress to fat_rlps() so we can abort from there too. * Checking for abort in a single spot * Remove nightly-only weak/strong counts * fix warning * Fix tests * Add dummy impl to abort snapshots * Add another dummy impl for TestSnapshotService * Remove debugging code * Return error instead of the odd Ok(()) Switch to AtomicU64 * revert .as_bytes() change * fix build * fix build maybe

seunlanlege requested review from tomusdrw and ordian May 16, 2019 10:27

seunlanlege added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. M4-io 💾 Interaction with filesystem/databases. labels May 16, 2019

jam10o-new added B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. B1-patch-beta 🕷🕷 labels May 16, 2019

ordian reviewed May 16, 2019

View reviewed changes

ethcore/src/client/client.rs Outdated Show resolved Hide resolved

tomusdrw reviewed May 16, 2019

View reviewed changes

ethcore/src/client/client.rs Outdated Show resolved Hide resolved

ethcore/src/client/client.rs Outdated Show resolved Hide resolved

soc1c added this to the 2.6 milestone May 17, 2019

ordian reviewed May 20, 2019

View reviewed changes

tomusdrw approved these changes May 21, 2019

View reviewed changes

seunlanlege added 4 commits May 21, 2019 12:00

delete BlockDetails from COL_EXTRA

3be078c

better proofs

dc0d60e

added tests

0a79239

PR suggestions

5e59935

seunlanlege force-pushed the reset-blockchain branch from 3a20ab9 to 5e59935 Compare May 21, 2019 11:01

ordian approved these changes May 21, 2019

View reviewed changes

ordian merged commit 21a27fe into master May 21, 2019

ordian deleted the reset-blockchain branch May 21, 2019 11:52

seunlanlege commented May 21, 2019

View reviewed changes

This was referenced Jun 1, 2019

void #10708

Closed

void poanetwork/parity-ethereum#143

Closed

soc1c mentioned this pull request Jun 11, 2019

backports for stable 2.4.7 #10736

Merged

6 tasks

soc1c mentioned this pull request Jun 11, 2019

backports for beta 2.5.2 #10737

Merged

7 tasks

soc1c pushed a commit that referenced this pull request Jun 11, 2019

Reset blockchain properly (#10669)

83d2dee

* delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions

ghost mentioned this pull request Jun 11, 2019

void #10712

Closed

ordian pushed a commit that referenced this pull request Jun 24, 2019

Reset blockchain properly (#10669)

a6c03ea

* delete BlockDetails from COL_EXTRA * better proofs * added tests * PR suggestions

s3krit mentioned this pull request Jun 25, 2019

Stable 2.4.8 backports #10779

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reset blockchain properly #10669

Reset blockchain properly #10669

seunlanlege commented May 16, 2019 •

edited

Loading

parity-cla-bot commented May 16, 2019

tomusdrw left a comment •

edited

Loading

seunlanlege commented May 20, 2019

ordian May 20, 2019

seunlanlege May 20, 2019

tomusdrw May 21, 2019

ordian May 21, 2019

tomusdrw left a comment

tomusdrw May 21, 2019

ordian commented May 21, 2019

seunlanlege May 21, 2019

CremaFR commented Jun 14, 2019 •

edited

Loading

Reset blockchain properly #10669

Reset blockchain properly #10669

Conversation

seunlanlege commented May 16, 2019 • edited Loading

parity-cla-bot commented May 16, 2019

tomusdrw left a comment • edited Loading

Choose a reason for hiding this comment

seunlanlege commented May 20, 2019

ordian May 20, 2019

Choose a reason for hiding this comment

seunlanlege May 20, 2019

Choose a reason for hiding this comment

tomusdrw May 21, 2019

Choose a reason for hiding this comment

ordian May 21, 2019

Choose a reason for hiding this comment

tomusdrw left a comment

Choose a reason for hiding this comment

tomusdrw May 21, 2019

Choose a reason for hiding this comment

ordian commented May 21, 2019

seunlanlege May 21, 2019

Choose a reason for hiding this comment

CremaFR commented Jun 14, 2019 • edited Loading

seunlanlege commented May 16, 2019 •

edited

Loading

tomusdrw left a comment •

edited

Loading

CremaFR commented Jun 14, 2019 •

edited

Loading