core/state, core, miner: handle missing root error from state.New #1881

Gustav-Simonsson · 2015-10-06T14:36:58Z

This turned into

After a lot of discussion and feedback from @fjl this should now be ready for review again:

Bubble up error from state.New if missing state root
If this error is hit in miner, short-circuit commitNewWork
In tx pool, delay init of pendingState until new block or tx arrives.

robotally · 2015-10-06T14:36:59Z

Vote	Count	Reviewers
👍	1	@fjl
👎	0

Updated: Fri Oct 16 10:05:29 UTC 2015

fjl · 2015-10-06T15:02:57Z

core/chain_makers.go

You can panic here. GenerateChain is not supposed to fail.

obscuren · 2015-10-07T08:55:59Z

Are you looking for a try { } catch { } @Gustav-Simonsson :-P

obscuren · 2015-10-07T08:58:51Z

This has turned in to quite a mess. Is this required just for tests or are there requirements for normal operation as well? It works fine currently without the need to return errors.

If this is just for tests I'll suggestion something nasty: panic with a recover :-D

Gustav-Simonsson · 2015-10-07T09:04:22Z

@obscuren hehe, just wanted to see how big the change would be if naively bubbling up this error, as start of discussion of how to fix it :) will see how small the changeset becomes if changing state.Copy per @fjl suggestion. I can't say how likely this error is during normal operation, @fjl knows more there since he build the new trie.

fjl · 2015-10-07T09:11:40Z

trie.New now returns an error if the root node is missing from the database.
This PR is supposed to ensure that a missing root node causes chain insertion to fail.
It's not just for tests

fjl · 2015-10-07T09:52:01Z

xeth/xeth.go

@obscuren This is one of the places where this change is actually useful.
Consider a full node that is at block 33000 and has state roots after 30000.
If someone requests a state value from block 20000 (for which the node doesn't have the state)
you'd rather make that visible somehow and return a proper error.

fjl · 2015-10-07T10:57:01Z

core/state/managed_state.go

Please revert this change.

fjl · 2015-10-16T08:44:51Z

👍

codecov-io · 2015-10-16T10:05:28Z

Current coverage is `47.77%`

Merging #1881 into develop will decrease coverage by -0.10% as of 44e3d3c

Powered by Codecov. Updated on successful CI builds.

core/state, core, miner: handle missing root error from state.New

* chore: MadhugiriPro hf * params: use stable tag * chore: set new HF heights * Reinforce Precompile Check on any new HF (ethereum#1881) * Include missing check of P256 on Amoy (ethereum#1877) * checks p256 instruction * lint fix * reinforce precompilers check --------- Co-authored-by: Lucca Martins <lucca_martins30@yahoo.com.br>

* Parallel block import (ethereum#1662) * core: naive first implmenetation of parallel block import in InsertChainStateless * core: add logic to handle dependency while parallel import * core: handle unknown ancestor + init some UTs * core: implement alternative way to check for unknown ancestor * core: defer ValidateWitnessPreState * zero commit * core: do sequential insert if the importing chain is small * core: fix error handling * core: update UTs * core,eth,internal/cli: add config to enable/disable parallel import + refactor * core: add metrics for seq and parallel import * internal/cli: moved parallelstatelessimport flag to the witness config * core: changed debug import log to info * core: added 2 new metrics to track the number of blocks processed in sequential and parallel stateless imports * core: init benchmarks * core: fix benchmarks * core: use worker pool for parallel block import * core: try parallel import for smaller batch * core: dedup header verification logic * zero commit * core,eth,internal/cli: improvements based on comments * core: add adversarial tests for stateless insert * core: re-add empty chain insertion case * zero commit to trigger CI * core,eth/catalyst: report correct execution stats for stateless insert * CI: temp update for running stateless tests * zero commit to trigger CI * zero commit to trigger CI * CI: try running on stateless branch * Revert "CI: try running on stateless branch" This reverts commit 98b5704fabcc6855225cc03e8f84e77f913fda08. * zero commit to trigger CI * core: add log for deferred exec of blocks due to invalid root err in parallel import * core: changed log to debug * core,eth,internal/ethapi: suppress error log for state sync when parallel import is enabled * eth/filters: fix gomock * eth: add missing configs * core: retry execution in parallel import for block validator errors * core: commit block before retrying failed execution * core: rm redundant witness in execResult * core: try flushing state db asap post execution * core: add temp log when committing code * core: enable retry for first block in batch * Revert "core: add temp log when committing code" This reverts commit cf63cc344ce7c0981c3410320dea04ba74dc448f. * core: cap workers spun * Revert "core: cap workers spun" This reverts commit 96367eebfe94dfc76e74dced4d55b71ce8a50896. * core: ensure statedb cleanup * core: add temp debug logs * core: add temp debug logs * core: add temp log to track sidechain * core: add temp log for contract code in commitAndFlush * core: add retry for all processing and internal state db errors * core: add state db error validation during deferred exec * core: add TestParallelStateless_ContractDeployedThenCalled * Revert "core: add temp log for contract code in commitAndFlush" This reverts commit 174765f14ced2ccff8cd8570935b04bda478342c. * Revert "core: add temp log to track sidechain" This reverts commit 29b33667a61016a7f2e71817c2bd287a27852f06. * Revert "core: add temp debug logs" This reverts commit 2c4f8407de9c4535565d092a4310576e6aab87b0. * internal/cli: turn off parallel import by default * internal/cli: update flag identation * CI: update comment * zero commit to trigger CI * internal/cli: rm witness prune flags --------- Co-authored-by: Pratik Patil <pratikspatil024@gmail.com> * emit event when write block in stateless node (ethereum#1840) * Address Biased Trie Cache (ethereum#1837) * Address-biased trie cache * Stop preload early * Print more stats * Increase cache size * Store by depth * More customizable * Add metrics * Fix key format * Async preload * Simplify * Fix linter * Allow interrupt * Remove free disk step in CI (ethereum#1843) Remove this step since the runner has 600G of disk space. * chore: bump kurtosis-pos (ethereum#1845) * chore: bump kurtosis-pos * chore: bump to v1.2.1 * PIP-74: state-sync txs inclusion (ethereum#1726) * first working version of new state sync tx * remove logs * fix lint * fix integration tests * test fixes * lint fix * fix parallel processor * type improvement and unit tests * remove duplicates * ignore data test output * fixing method calls and fields * build fix * remove bor filter for after hf blocks * sort logs just when necessary * ssTxs: fix receiptsHash mismatch (ethereum#1829) * ssTxs: fix receiptsHash mismatch * chore: remove block hash and number * revert: add block number * core(tx): fix Hash method for StateSyncTxType (ethereum#1830) * eth: fixes in receipt handling via p2p post HF (ethereum#1825) * eth: include bor receipts in ReceiptHash post HF * eth: rename to receiptListHash * eth: extrat typecasting logic for better testing, fix type matching issue * eth: add e2e tests for receipt delivery * eth/protocols/eth: apply HF logic while handling receipt query over eth69 * core: skip split receipts post HF * tests/bor: extend e2e test to check presence of state-sync in block * core/types: return nil for chainID() call over state-sync tx * internal/ethapi: handle bor txs and receipts post HF (ethereum#1834) * fix: cumulativeGasUsed in insertStateSyncTransactionAndCalculateReceipt (ethereum#1835) * fix: sort logs and remove duplicate append in Finalize (ethereum#1836) * fix: append tx only in FinalizeAndAssemble and use state instead of wrappedState * consensus/bor: sort logs before extracting state-sync logs * chore: revert statedb changes --------- Co-authored-by: Manav Darji <manavdarji.india@gmail.com> * chore: nit (ethereum#1838) * chore: typos * fix: lint * Renaming to Madhugiri HF * chore: nits * (fix): handle pointers to receipt list received via p2p * (chore): rename tests with HF name * chore: more nits * core: add blocktime in bor receipt logs (ethereum#1848) * core/types: derive bloom for bor receipts --------- Co-authored-by: kamuikatsurgi <shahkrishang11@gmail.com> Co-authored-by: Krishang Shah <109511742+kamuikatsurgi@users.noreply.github.com> Co-authored-by: Manav Darji <manavdarji.india@gmail.com> * core, miner, params, cmd: implement EIP-7823, EIP-7825 and EIP-7883 (ethereum#1842) * first working version of new state sync tx * remove logs * fix lint * fix integration tests * test fixes * lint fix * fix parallel processor * type improvement and unit tests * remove duplicates * ignore data test output * fixing method calls and fields * build fix * remove bor filter for after hf blocks * sort logs just when necessary * ssTxs: fix receiptsHash mismatch (ethereum#1829) * ssTxs: fix receiptsHash mismatch * chore: remove block hash and number * revert: add block number * core(tx): fix Hash method for StateSyncTxType (ethereum#1830) * eth: fixes in receipt handling via p2p post HF (ethereum#1825) * eth: include bor receipts in ReceiptHash post HF * eth: rename to receiptListHash * eth: extrat typecasting logic for better testing, fix type matching issue * eth: add e2e tests for receipt delivery * eth/protocols/eth: apply HF logic while handling receipt query over eth69 * core: skip split receipts post HF * tests/bor: extend e2e test to check presence of state-sync in block * core/types: return nil for chainID() call over state-sync tx * internal/ethapi: handle bor txs and receipts post HF (ethereum#1834) * fix: cumulativeGasUsed in insertStateSyncTransactionAndCalculateReceipt (ethereum#1835) * fix: sort logs and remove duplicate append in Finalize (ethereum#1836) * fix: append tx only in FinalizeAndAssemble and use state instead of wrappedState * consensus/bor: sort logs before extracting state-sync logs * chore: revert statedb changes --------- Co-authored-by: Manav Darji <manavdarji.india@gmail.com> * chore: nit (ethereum#1838) * chore: typos * fix: lint * Renaming to Madhugiri HF * chore: nits * (fix): handle pointers to receipt list received via p2p * (chore): rename tests with HF name * chore: more nits * core: implement eip7883 * core: implement eip7825 * fix lint * fix isOsaka condition * fix lint and add comment for int test failing * fix failing test / address comments * core: add blocktime in bor receipt logs (ethereum#1848) * core/types: derive bloom for bor receipts * prioritize ss hf / remove todo * fix lint * split precompiled contracts and addresses for madhugiri HF from osaka * revert err msg to align with geth * eth/gasestimator: check ErrGasLimitTooHigh conditions (ethereum#32348) This PR makes 2 changes to how [EIP-7825](ethereum#31824) behaves. When `eth_estimateGas` or `eth_createAccessList` is called without any gas limit in the payload, geth will choose the block's gas limit or the `RPCGasCap`, which can be larger than the `maxTxGas`. When this happens for `estimateGas`, the gas estimation just errors out and ends, when it should continue doing binary search to find the lowest possible gas limit. This PR will: - Add a check to see if `hi` is larger than `maxTxGas` and cap it to `maxTxGas` if it's larger. And add a special case handling for gas estimation execute when it errs with `ErrGasLimitTooHigh` --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com> * internal/ethapi: skip tx gas limit check for calls (ethereum#32641) This disables the tx gaslimit cap for eth_call and related RPC operations. I don't like how this fix works. Ideally we'd be checking the tx gaslimit somewhere else, like in the block validator, or any other place that considers block transactions. Doing the check in StateTransition means it affects all possible ways of executing a message. The challenge is finding a place for this check that also triggers correctly in tests where it is wanted. So for now, we are just combining this with the EOA sender check for transactions. Both are disabled for call-type messages. * internal/ethapi: use gas from gaspool for call defaults * eth/gasestimator: check for madhugiri HF * consensus/bor: honour MaxTxGas for system calls --------- Co-authored-by: Lucca Martins <lucca_martins30@yahoo.com.br> Co-authored-by: kamuikatsurgi <shahkrishang11@gmail.com> Co-authored-by: Krishang Shah <109511742+kamuikatsurgi@users.noreply.github.com> Co-authored-by: Manav Darji <manavdarji.india@gmail.com> Co-authored-by: Minhyuk Kim <kimminhyuk1004@gmail.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com> * (chore): update madhugiri block number for amoy and update consensus block time (ethereum#1851) * first working version of new state sync tx * remove logs * fix lint * fix integration tests * test fixes * lint fix * fix parallel processor * type improvement and unit tests * remove duplicates * ignore data test output * fixing method calls and fields * build fix * remove bor filter for after hf blocks * sort logs just when necessary * ssTxs: fix receiptsHash mismatch (ethereum#1829) * ssTxs: fix receiptsHash mismatch * chore: remove block hash and number * revert: add block number * core(tx): fix Hash method for StateSyncTxType (ethereum#1830) * eth: fixes in receipt handling via p2p post HF (ethereum#1825) * eth: include bor receipts in ReceiptHash post HF * eth: rename to receiptListHash * eth: extrat typecasting logic for better testing, fix type matching issue * eth: add e2e tests for receipt delivery * eth/protocols/eth: apply HF logic while handling receipt query over eth69 * core: skip split receipts post HF * tests/bor: extend e2e test to check presence of state-sync in block * core/types: return nil for chainID() call over state-sync tx * internal/ethapi: handle bor txs and receipts post HF (ethereum#1834) * fix: cumulativeGasUsed in insertStateSyncTransactionAndCalculateReceipt (ethereum#1835) * fix: sort logs and remove duplicate append in Finalize (ethereum#1836) * fix: append tx only in FinalizeAndAssemble and use state instead of wrappedState * consensus/bor: sort logs before extracting state-sync logs * chore: revert statedb changes --------- Co-authored-by: Manav Darji <manavdarji.india@gmail.com> * chore: nit (ethereum#1838) * chore: typos * fix: lint * Renaming to Madhugiri HF * chore: nits * (fix): handle pointers to receipt list received via p2p * (chore): rename tests with HF name * chore: more nits * core: add blocktime in bor receipt logs (ethereum#1848) * core/types: derive bloom for bor receipts * (chore): update madhugiri block for amoy * Change consensus block time to 1s at Madhugiri HF (ethereum#1852) --------- Co-authored-by: Lucca Martins <lucca_martins30@yahoo.com.br> Co-authored-by: kamuikatsurgi <shahkrishang11@gmail.com> Co-authored-by: Krishang Shah <109511742+kamuikatsurgi@users.noreply.github.com> Co-authored-by: Jerry <jerrycgh@gmail.com> * Completed reset prefetcher (ethereum#1853) * internal/ethapi: restore original RPC gas cap (ethereum#1856) * Restore original RPC gas cap * Fix test * chore: fix wrong function name in comment (ethereum#1841) Signed-off-by: reddaisyy <reddaisy@outlook.jp> * Revert "(chore): update madhugiri block number for amoy and update consensus block time (ethereum#1851)" (ethereum#1857) This reverts commit 103f1f4. * params: bump version to v2.4.0-beta * Dont OpenTrie because HistoricDatabase doesnt implement trie (ethereum#1858) * chore: use 2^25 as the MaxTxGas (ethereum#1859) * params: bump version to v2.4.0-beta2 * chore: set madhugiri block for amoy and consensus block time (ethereum#1867) * chore: set madhugiri block for amoy and consensus block time * params: bump version * Disable txn indexer in stateless mode Since stateless nodes are using a different pruner from the regular pruner in geth, the transaction indexer will have problem with pruned blocks, resulting hanging goroutines in memory. This change prevents stateless node from running indexer on pruned blocks. * core: init PrecompiledAddressesMadhugiri * params: bump version * params: bump version for stable release * chore: set madhugiri block for mainnet and consensus block time * chore: fix lint * Include missing check of P256 on Amoy (ethereum#1877) * checks p256 instruction * lint fix * v2.5.0 candidate (ethereum#1878) * chore: MadhugiriPro hf * params: use stable tag * chore: set new HF heights * Reinforce Precompile Check on any new HF (ethereum#1881) * Include missing check of P256 on Amoy (ethereum#1877) * checks p256 instruction * lint fix * reinforce precompilers check --------- Co-authored-by: Lucca Martins <lucca_martins30@yahoo.com.br> --------- Signed-off-by: reddaisyy <reddaisy@outlook.jp> Co-authored-by: Raneet Debnath <35629432+Raneet10@users.noreply.github.com> Co-authored-by: Pratik Patil <pratikspatil024@gmail.com> Co-authored-by: Lucca Martins <lucca_martins30@yahoo.com.br> Co-authored-by: Jerry <jerrycgh@gmail.com> Co-authored-by: Krishang Shah <109511742+kamuikatsurgi@users.noreply.github.com> Co-authored-by: kamuikatsurgi <shahkrishang11@gmail.com> Co-authored-by: Manav Darji <manavdarji.india@gmail.com> Co-authored-by: Minhyuk Kim <kimminhyuk1004@gmail.com> Co-authored-by: Gary Rong <garyrong0905@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: reddaisyy <reddaisy@outlook.jp> Co-authored-by: Angel Valkov <avalkov@polygon.technology>

obscuren added the in progress label Oct 6, 2015

Gustav-Simonsson added review and removed in progress labels Oct 6, 2015

fjl reviewed Oct 6, 2015
View reviewed changes

core/chain_makers.go Outdated

Copy link

Contributor

fjl Oct 6, 2015

Choose a reason for hiding this comment

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

You can panic here. GenerateChain is not supposed to fail.

Gustav-Simonsson force-pushed the state_new_error branch 2 times, most recently from a4fb066 to 65d4982 Compare October 7, 2015 09:43

fjl reviewed Oct 7, 2015
View reviewed changes

Gustav-Simonsson force-pushed the state_new_error branch from 65d4982 to 3d78dac Compare October 7, 2015 10:50

Gustav-Simonsson changed the title ~~many packages: bubble up error from new state.New~~ core/state: bubble up error from state.New if missing state root Oct 7, 2015

fjl reviewed Oct 7, 2015
View reviewed changes

core/state/managed_state.go Outdated

Copy link

Contributor

fjl Oct 7, 2015

Choose a reason for hiding this comment

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

Please revert this change.

Gustav-Simonsson force-pushed the state_new_error branch 4 times, most recently from 1af9dbe to fc9010a Compare October 7, 2015 14:51

Gustav-Simonsson changed the title ~~core/state: bubble up error from state.New if missing state root~~ core/state, core, miner: handle missing root error from state.New Oct 7, 2015

fjl added the core label Oct 7, 2015

fjl added this to the 1.3.0 milestone Oct 7, 2015

core/state, core, miner: handle missing root error from state.New

1b1f293

Gustav-Simonsson force-pushed the state_new_error branch from fc9010a to 1b1f293 Compare October 16, 2015 09:58

obscuren added a commit that referenced this pull request Oct 16, 2015

Merge pull request #1881 from Gustav-Simonsson/state_new_error

b747754

core/state, core, miner: handle missing root error from state.New

obscuren merged commit b747754 into ethereum:develop Oct 16, 2015

obscuren removed the review label Oct 16, 2015

obscuren modified the milestones: 2.0.0, 1.3.0 Oct 29, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

core/state, core, miner: handle missing root error from state.New #1881

core/state, core, miner: handle missing root error from state.New #1881

Uh oh!

Gustav-Simonsson commented Oct 6, 2015

Uh oh!

robotally commented Oct 6, 2015

Uh oh!

fjl Oct 6, 2015

Uh oh!

obscuren commented Oct 7, 2015

Uh oh!

obscuren commented Oct 7, 2015

Uh oh!

Gustav-Simonsson commented Oct 7, 2015

Uh oh!

fjl commented Oct 7, 2015

Uh oh!

fjl Oct 7, 2015

Uh oh!

fjl Oct 7, 2015

Uh oh!

fjl commented Oct 16, 2015

Uh oh!

codecov-io commented Oct 16, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

core/state, core, miner: handle missing root error from state.New #1881

core/state, core, miner: handle missing root error from state.New #1881

Uh oh!

Conversation

Gustav-Simonsson commented Oct 6, 2015

Uh oh!

robotally commented Oct 6, 2015

Uh oh!

fjl Oct 6, 2015

Choose a reason for hiding this comment

Uh oh!

obscuren commented Oct 7, 2015

Uh oh!

obscuren commented Oct 7, 2015

Uh oh!

Gustav-Simonsson commented Oct 7, 2015

Uh oh!

fjl commented Oct 7, 2015

Uh oh!

fjl Oct 7, 2015

Choose a reason for hiding this comment

Uh oh!

fjl Oct 7, 2015

Choose a reason for hiding this comment

Uh oh!

fjl commented Oct 16, 2015

Uh oh!

codecov-io commented Oct 16, 2015

Current coverage is 47.77%

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Current coverage is `47.77%`