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.

Sign up for GitHub

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

Embedding transaction status code in receipts #658

Merged
merged 6 commits into from
Nov 30, 2017
Merged

Embedding transaction status code in receipts #658

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
378af99
Create eip-draft-returndata.md
Arachnid Jun 30, 2017
327c81c
Update eip-draft-returndata.md
Arachnid Jul 14, 2017
df687f5
Update eip-draft-returndata.md
Arachnid Sep 4, 2017
947e765
Update eip-draft-returndata.md
Arachnid Oct 9, 2017
47e30db
Update eip-draft-returndata.md
Arachnid Nov 20, 2017
d1ae915
Rename eip-draft-returndata.md to eip-658.md
Arachnid Nov 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions EIPS/eip-draft-returndata.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
## Preamble

EIP: <to be assigned>
Copy link
Member

Choose a reason for hiding this comment

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

Number 658 seems to have been assigned to this one.

Title: Embedding transaction return data in receipts
Copy link
Member

Choose a reason for hiding this comment

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

I guess the title should now be changed to Embedding transaction status code in receipts.

Author: Nick Johnson <[email protected]>
Type: Standard Track
Category Core
Status: Draft
Copy link
Member

Choose a reason for hiding this comment

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

Status should now be Accepted.

Copy link
Member

Choose a reason for hiding this comment

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

No, more like Final.

Created: 2017-06-30
Requires: 140
Replaces: 98


## Abstract
This EIP replaces the intermediate state root field of the receipt with a status code indicating if the top-level call succeeded or failed.

## Motivation
With the introduction of the REVERT opcode in EIP140, it is no longer possible for users to assume that a transaction failed iff it consumed all gas. As a result, there is no clear mechanism for callers to determine whether a transaction succeeded and the state changes contained in it were applied.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: iff

Copy link
Member

Choose a reason for hiding this comment

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

I believe it's intended ("if and only if")

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a transaction that consumed all gas and didn't fail.

It was never possible to make the assumption to begin with. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't, but people still did.


Full nodes can provide RPCs to get a transaction return status and value by replaying the transaction, but fast nodes can only do this for nodes after their pivot point, and light nodes cannot do this at all, making a non-consensus solution impractical.

Instead, we propose to replace the intermediate state root, already obsoleted by EIP98, with the return status (1 for success, 0 for failure). This both allows callers to determine success status, and remedies the previous omission of return data from the receipt.
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand (from #658 (comment)), this particular EIP will be given number 98. If so, it will be confusing if this EIP references EIP 98 as if it was a different one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should read fine IMO if , already obsoleted by EIP98, is removed.

IMO the following (1 for success, 0 for failure) can be dropped, too, since this detail is elaborated on in section Specification, just a few lines below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remind me, what's RPCs?

Copy link
Member

Choose a reason for hiding this comment

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

Regarding the mention of EIP98, I would add a link to the relevant issue or pull-request. I believe the Motivation section can talk about the outer world rather freely.


## Specification
For blocks where block.number >= METROPOLIS_FORK_BLKNUM, the intermediate state root is replaced by a status code, a single byte with 0 indicating failure (due to any operation that can cause the transaction or top-level call to revert) and 1 indicating success.
Copy link
Member

Choose a reason for hiding this comment

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

"a single byte with 0 or 1" is somewhat confusing because RLP representation of 0 (number) is empty byte array (i.e. zero bytes).
This should say either "number 0 or 1" or something like "array of size 1 with 0 or 1 byte"

cpp currently implemented it as a number, I think it's more natural.

Copy link
Member

Choose a reason for hiding this comment

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

Now the activation should be BYZANTIUM_FORK_BLKNUM.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pirapira Are there actually any tests for this EIP? If so, can you mention them here? Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

For instance, this receipt trie should be calculated using the new receipt format https://github.com/ethereum/tests/blob/develop/BlockchainTests/bcStateTests/blockhashTests.json#L22


## Rationale
This constitutes a minimal possible change that permits fetching the success/failure state of transactions, preserving existing capabilities with minimum disruption or additional work for Metropolis.

## Copyright
Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).