Remove (almost all) panickers from trie module #1776

rphmeier · 2016-07-30T17:44:28Z

Those in the trie iterator still remain. Not sure if it's worth removing them since it would mean making the iterator item a Result<TrieItem>.

Trie errors will still immediately panic (probably better to fail fast than limp along in a broken state), but the infrastructure is in place to catch them and ask for trie nodes from the network.

coveralls · 2016-07-30T18:10:56Z

Coverage decreased (-0.4%) to 86.433% when pulling b560a2e on trie_errors into 53f1d7b on master.

gavofyork · 2016-07-30T22:13:14Z

tests failing...

rphmeier · 2016-07-31T15:56:15Z

appveyor failure is spurious

coveralls · 2016-07-31T16:12:15Z

Coverage decreased (-0.4%) to 86.406% when pulling ffffa2a on trie_errors into 53f1d7b on master.

gavofyork · 2016-08-01T15:15:14Z

ethcore/src/account.rs

@@ -136,7 +138,11 @@ impl Account {
 				SecTrieDBMut would not set it to an invalid state root. Therefore the root is valid and DB creation \
 				using it will not fail.");

-			(Filth::Clean, H256::from(db.get(key).map_or(U256::zero(), |v| -> U256 {decode(v)})))
+			let item: U256 = match db.get(key){
+				Ok(x) => x.map_or_else(|| U256::zero(), decode),


x.map_or_else(U256::zero, decode)?

or x.map_or_else(U256::default, decode) to avoid importing Uint trait :)

[ci:skip]

coveralls · 2016-08-01T15:50:28Z

Coverage decreased (-0.3%) to 86.507% when pulling 65c20ef on trie_errors into 53f1d7b on master.

coveralls · 2016-08-01T16:24:55Z

Coverage decreased (-0.4%) to 86.499% when pulling 65c20ef on trie_errors into 53f1d7b on master.

tomusdrw · 2016-08-02T08:23:53Z

ethcore/src/state.rs

+		db: &mut HashDB,
+		root: &mut H256,
+		accounts: &mut HashMap<Address,
+		Option<Account>>


Could stay on the same line.

it could, but the line stands at something like 130 characters plus a tab. it seems cleaner to indent it in some way.

coveralls · 2016-08-02T11:06:14Z

Coverage decreased (-0.4%) to 86.406% when pulling 99e2011 on trie_errors into 53f1d7b on master.

gavofyork · 2016-08-02T14:02:24Z

looks generally good.

would be nice to re-review against the current master - there are a lot of unrelated merges in the diff here making it difficult to see the forest for the trees sometimes.

coveralls · 2016-08-02T15:08:35Z

Coverage decreased (-0.6%) to 86.402% when pulling f412fd2 on trie_errors into 21c65a9 on master.

rphmeier · 2016-08-02T15:38:03Z

@gavofyork it is merged with the latest master -- i hadn't realized it was messing up the diff.

coveralls · 2016-08-03T11:31:30Z

Coverage decreased (-0.5%) to 86.498% when pulling e0ac2ff on trie_errors into 9fb5623 on master.

gavofyork · 2016-08-03T14:35:23Z

travis failing?

rphmeier · 2016-08-03T15:31:50Z

seems like it, not sure why -- all the tests are passing on my machine ;)
I wonder if it's spurious, since the test failure here is miner::should_get_price_info. completely unrelated to trie.

gavofyork · 2016-08-03T15:59:08Z

yeah - that one requires network access.

coveralls · 2016-08-03T16:16:53Z

Coverage decreased (-0.4%) to 86.561% when pulling 771fb50 on trie_errors into 8c88e2a on master.

rphmeier added 20 commits July 6, 2016 18:08

memorydb ub patch and other cleanup

fd188d3

fix denote invocations

45c2c46

merge with master

03bd823

move trie traits into trie module

07d48a1

replace "denote" with shim

2bf5587

triedb returns results and no longer panics

c631c6c

fix warnings

b1ff25d

get ethcore compiling

1f6f20c

warn on trie errors in ethcore

2d2c54a

remove unsafety from node decoder

473cfd0

merge with master

b10d230

restore broken denote behavior for this branch

99c2206

fix overlayrecent fallout

c4b233f

fix triedb tests

d8d6043

remove unwrap in state

888f9eb

merge with master

372ea4a

alter Trie::get to return Result<Option<_>>

760e6a4

merge with master, fail fast

2a2d17f

fix refcell error in require

a01b0fe

fix test warnings

b560a2e

rphmeier added the A0-pleasereview 🤓 Pull request needs code review. label Jul 30, 2016

rphmeier mentioned this pull request Jul 30, 2016

Remove panickers in Trie module #1397

Closed

rphmeier mentioned this pull request Jul 31, 2016

Undefined behavior in state.rs #1740

Closed

fix json tests

ffffa2a

gavofyork reviewed Aug 1, 2016
View reviewed changes

whitespace

82f7a0e

[ci:skip]

tomusdrw reviewed Aug 2, 2016
View reviewed changes

rphmeier added 2 commits August 2, 2016 12:39

Merge branch 'trie_errors' of github.com:ethcore/parity into trie_errors

8b54b4f

prettify map_or_else

99e2011

gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 2, 2016

Merge branch 'master' into trie_errors

f412fd2

rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Aug 2, 2016

gavofyork added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 2, 2016

rphmeier added 2 commits August 3, 2016 12:59

merge with master

bc70dde

remove test warning

e0ac2ff

rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Aug 3, 2016

gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 3, 2016

merge with master

771fb50

gavofyork merged commit 11b65ce into master Aug 3, 2016

gavofyork deleted the trie_errors branch August 3, 2016 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove (almost all) panickers from trie module #1776

Remove (almost all) panickers from trie module #1776

rphmeier commented Jul 30, 2016 •

edited

coveralls commented Jul 30, 2016 •

edited

gavofyork commented Jul 30, 2016

rphmeier commented Jul 31, 2016

coveralls commented Jul 31, 2016 •

edited

gavofyork Aug 1, 2016

debris Aug 2, 2016

coveralls commented Aug 1, 2016 •

edited

coveralls commented Aug 1, 2016 •

edited

tomusdrw Aug 2, 2016

rphmeier Aug 2, 2016

coveralls commented Aug 2, 2016 •

edited

gavofyork commented Aug 2, 2016 •

edited

coveralls commented Aug 2, 2016 •

edited

rphmeier commented Aug 2, 2016

coveralls commented Aug 3, 2016 •

edited

gavofyork commented Aug 3, 2016

rphmeier commented Aug 3, 2016

gavofyork commented Aug 3, 2016

coveralls commented Aug 3, 2016 •

edited

Remove (almost all) panickers from trie module #1776

Remove (almost all) panickers from trie module #1776

Conversation

rphmeier commented Jul 30, 2016 • edited

coveralls commented Jul 30, 2016 • edited

gavofyork commented Jul 30, 2016

rphmeier commented Jul 31, 2016

coveralls commented Jul 31, 2016 • edited

gavofyork Aug 1, 2016

Choose a reason for hiding this comment

debris Aug 2, 2016

Choose a reason for hiding this comment

coveralls commented Aug 1, 2016 • edited

coveralls commented Aug 1, 2016 • edited

tomusdrw Aug 2, 2016

Choose a reason for hiding this comment

rphmeier Aug 2, 2016

Choose a reason for hiding this comment

coveralls commented Aug 2, 2016 • edited

gavofyork commented Aug 2, 2016 • edited

coveralls commented Aug 2, 2016 • edited

rphmeier commented Aug 2, 2016

coveralls commented Aug 3, 2016 • edited

gavofyork commented Aug 3, 2016

rphmeier commented Aug 3, 2016

gavofyork commented Aug 3, 2016

coveralls commented Aug 3, 2016 • edited

rphmeier commented Jul 30, 2016 •

edited

coveralls commented Jul 30, 2016 •

edited

coveralls commented Jul 31, 2016 •

edited

coveralls commented Aug 1, 2016 •

edited

coveralls commented Aug 1, 2016 •

edited

coveralls commented Aug 2, 2016 •

edited

gavofyork commented Aug 2, 2016 •

edited

coveralls commented Aug 2, 2016 •

edited

coveralls commented Aug 3, 2016 •

edited

coveralls commented Aug 3, 2016 •

edited