Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove medstate from receipts #98

Closed
vbuterin opened this issue Apr 26, 2016 · 6 comments
Closed

Remove medstate from receipts #98

vbuterin opened this issue Apr 26, 2016 · 6 comments

Comments

@vbuterin
Copy link
Contributor

vbuterin commented Apr 26, 2016

Parameters

  • METROPOLIS_FORK_BLKNUM: TBA

Specification

Option 1: For blocks where block.number >= METROPOLIS_FORK_BLKNUM, the receipt should remove the intermediate state root parameter.

Option 2: For blocks where block.number >= METROPOLIS_FORK_BLKNUM, the intermediate state root parameter in the receipt should be set to zero instead of the actual state root.

Option 3 (update 2017.07.28: we are going with this one): For blocks where block.number >= METROPOLIS_FORK_BLKNUM, the intermediate state root parameter in the receipt should be set to a \x01 byte if the outermost code execution succeeded, or a zero byte if the outermost code execution failed.

Rationale

Not calculating the state root after each transaction allows for the process of computing the state root to be parallelized, and for fewer merkle tree branches to be calculated because repeated updates can be hashed. Additionally, it sets the stage for future scalability upgrades where transaction processing itself is done in parallel.

This change DOES mean that if a miner creates a block where one state transition is processed incorrectly, then it is impossible to make a fraud proof specific to that transaction; instead, the fraud proof must consist of the entire block. However, (1) block times are low and we expect block times to reduce in the future with Casper, and (2) we have already accepted the principle that light clients must be capable of downloading entire blocks if need be, because of how verifying bloom filter entries works.

@kumavis
Copy link
Member

kumavis commented Jan 25, 2017

(2) we have already accepted the principle that light clients must be capable of downloading entire blocks if need be, because of how verifying bloom filter entries works.

We would also need to load state touched by all txs in the block up to the tx you want to replay. Since we don't know what it will touch ahead of time it, it is a serial process frequently blocked by getting state from the network. Just want to make sure we are properly enumerating the impacts on the light client strategy.

@gumb0
Copy link
Member

gumb0 commented Apr 6, 2017

Other Metropolis EIPs are specified for block.number >= METROPOLIS_FORK_BLKNUM, so start with fork block.

@Souptacular
Copy link
Contributor

@vbuterin does this EIP have an associated PR?

@Souptacular
Copy link
Contributor

EIP #98 corresponds to #658. We will call PR #658 EIP #98 and incorporate EIP #98 changes into PR #658.

@Souptacular
Copy link
Contributor

Closing issue. Please direct discussion of EIP 98 to pull request 658 per this comment.

@Arachnid
Copy link
Contributor

This EIP is referenced by #658, which has already been merged. As such, it needs to either be finalised and merged, or the dependency in the existing EIP needs to be removed.

sabondano added a commit to blockscout/blockscout that referenced this issue Sep 19, 2018
Why:

* Issue #757 was created after we noticed a few constraint violations
for the `error` column in the `transactions` table. The constraint
violations came about as we attempted to update transactions with a
successful status but some value for the error field. The constraint
is violated in this scenario because we expect successful transactions
to not have an error. (EIP
658)[ethereum/EIPs#658], which apparently is
also called (EIP
98)[ethereum/EIPs#98 (comment)]
, says that the status should be set to 1 (success) if the outermost
code execution succeeded (internal transaction with index 0), or it
should be set to 0 (failure) if the outermost code execution failed.
We're currently deriving the status from the last internal transaction
when in fact we should be deriving it from the first internal
transaction.
* Issue link: #757

This change addresses the need by:

* Editing `Explorer.Chain.Import.update_transactions/2` to set the error
for a transaction equal to the error, if any, of the internal
transaction with index 0.
* Editing `Explorer.Chain.Import.update_transactions/2` to set a
transaction's status as successful if it's internal transaction at index
0 doesn't have an error.
sabondano added a commit to blockscout/blockscout that referenced this issue Sep 19, 2018
Why:

* Issue #757 was created after we noticed a few constraint violations
for the `error` column in the `transactions` table. The constraint
violations came about as we attempted to update transactions with a
successful status but some value for the error field. The constraint
is violated in this scenario because we expect successful transactions
to not have an error. [EIP 658](ethereum/EIPs#658),
which apparently is also called [EIP 98](ethereum/EIPs#98 (comment))
, says that the status should be set to 1 (success) if the outermost
code execution succeeded (internal transaction with index 0), or it
should be set to 0 (failure) if the outermost code execution failed.
We're currently deriving the status from the last internal transaction
when in fact we should be deriving it from the first internal
transaction.
* Issue link: #757

This change addresses the need by:

* Editing `Explorer.Chain.Import.update_transactions/2` to set the error
for a transaction equal to the error, if any, of the internal
transaction with index 0.
* Editing `Explorer.Chain.Import.update_transactions/2` to set a
transaction's status as successful if it's internal transaction at index
0 doesn't have an error.
sabondano added a commit to blockscout/blockscout that referenced this issue Sep 19, 2018
Why:

* Issue #757 was created after we noticed a few constraint violations
for the `error` column in the `transactions` table. The constraint
violations came about as we attempted to update transactions with a
successful status but some value for the error field. The constraint
is violated in this scenario because we expect successful transactions
to not have an error. [EIP 658](ethereum/EIPs#658),
which apparently is also called [EIP 98](ethereum/EIPs#98 (comment))
, says that the status should be set to 1 (success) if the outermost
code execution succeeded (internal transaction with index 0), or it
should be set to 0 (failure) if the outermost code execution failed.
We're currently deriving the status from the last internal transaction
when in fact we should be deriving it from the first internal
transaction.
* Issue link: #757

This change addresses the need by:

* Editing `Explorer.Chain.Import.update_transactions/2` to set the error
for a transaction equal to the error, if any, of the internal
transaction with index 0.
* Editing `Explorer.Chain.Import.update_transactions/2` to set a
transaction's status as successful if it's internal transaction at index
0 doesn't have an error, and as failed if it does. This only applies to
pre-Byzantium and Ethereum Classic, for which transaction status is not
set from receipts.
meowsbits added a commit to etclabscore/multi-geth-fork that referenced this issue Dec 3, 2019
meowsbits added a commit to etclabscore/multi-geth-fork that referenced this issue Dec 16, 2019
meowsbits added a commit to etclabscore/multi-geth-fork that referenced this issue Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants