Skip to content

internal/ethapi: add status code to receipt rpc return #15042

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

Merged
merged 2 commits into from
Oct 2, 2017

Conversation

rjl493456442
Copy link
Member

No description provided.

@mi2zha
Copy link

mi2zha commented Aug 28, 2017

@rjl493456442
hi,
Version: 1.7.0-unstable
curl http://localhost:8545 -X POST --data '{"jsonrpc":"2.0","method":"eth_getTransactionReceipt","params":["0x839517dc0f8a73f6e259745ba60badcf8016c0a7ee783ee6246ccf1157408cb3"],"id":1}'

result:

{"jsonrpc":"2.0","id":1,"result":{"blockHash":"0xf81f9f905ac74e2832fc93c50566827055b4f0df6db2e12ce092f66ec154b813","blockNumber":"0x3ffca4","contractAddress":null,"cumulativeGasUsed":"0x5dde46","from":"0x3aeda7405558a096b620315c01d3d2af383632ec","gasUsed":"0x8fb1","logs":[],"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","root":"0xcdb37dc9126181d14d1d4846f5246750672242fa460d84b4315c0b6edc246590","to":"0xa74476443119a942de498590fe1f2454d7d4ac0d","transactionHash":"0x839517dc0f8a73f6e259745ba60badcf8016c0a7ee783ee6246ccf1157408cb3","transactionIndex":"0x34"}}

txHash "0x839517dc0f8a73f6e259745ba60badcf8016c0a7ee783ee6246ccf1157408cb3" is Bad jump destination

I use commit b25f260
receipt.postState is no nil, why?
Thank you for your help

@rjl493456442
Copy link
Member Author

@mi2zha Hey, since status field is added only after metropolish fork. So before that postState field is still remained and statusCode won't show.

@mi2zha
Copy link

mi2zha commented Aug 28, 2017

@rjl493456442
hi,
so how do you get a tx to succeed or fail

[out of gas] [Bad jump destination] or successful

@rjl493456442
Copy link
Member Author

As Ethereum EIP658 described, a transaction is regarded as failed only if the outermost execution failed. Any error returned (includes OutOfGas, Bad jump destination you mentioned) by the outermost call/create will make it.

@mi2zha
Copy link

mi2zha commented Aug 28, 2017

@rjl493456442

thank you for your patient answer

how did etherscan.io acquire the state of failure

debug.traceTransaction is ok? testing

@rjl493456442
Copy link
Member Author

Yeah, ethereum provides lots of tools to trace the execution state of the virtual machine. You can use debug.traceTransaction or evm tool to obtain detailed vm logs.

@rjl493456442
Copy link
Member Author

Actually i have added the failed flag into the returned result for traceTransaction rpc in this pr. But it has not been merged yet.

@mi2zha
Copy link

mi2zha commented Aug 28, 2017

@rjl493456442
internal/ethapi: add status code to receipt rpc return #15042

is this ?

@mi2zha
Copy link

mi2zha commented Aug 28, 2017

@rjl493456442
debug.traceTransaction("0x99a5e03e3cc7fd7764dfd97a357bfc1e689fddc4da2dc038ddc489662a8b6d22")

failed: true is failed,false is successful

thank u

if receipt.Failed {
fields["statusCode"] = receiptStatusFailed
}
}
Copy link
Member

@bas-vk bas-vk Aug 31, 2017

Choose a reason for hiding this comment

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

Receipt already has statusEncoding(). Can't we expose that one and use it instead?

Also, the EIP describes the status as a single byte, either 0 or 1. In the RPC bytes are hex encoded so this should be something like: fields["statusCode"] = hexutil.Bytes(receipt.StatusEncoding()). Even if it is be a number they need to be hex encoded as per RPC spec. The EIP also doesn't mention that the field name is statusCode, that should probably be amended to the EIP.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bas-vk, I had purposed a new EIP about adding a new field named statusCode to rpc returned. And as you mentioned, all rpc fields should be hex encoded, i think it is correct, and i will make change to satisfy this requirement.

@rjl493456442
Copy link
Member Author

@bas-vk Because receipt.statusEncoding function is not only related with receipt status field, but also with postState field. So in my opinion, it will be more clear to assign rpc return statusCode with recepit's Failed field.

PTAL

@mi2zha
Copy link

mi2zha commented Sep 1, 2017

@rjl493456442

but, there are multiple failures
[Out of gas] or [Bad jump destination]
how does statutscode express

@rjl493456442
Copy link
Member Author

@mi2zha The statusCode could be only 1 or 0, the former represents transaction executuon successful and the latter represents failed. If you want obtain the detailed error, i think tracer is a good helper.

@mi2zha
Copy link

mi2zha commented Sep 1, 2017

@rjl493456442
thank you very much

i will use debug.traceTransaction

thank you again

@bas-vk
Copy link
Member

bas-vk commented Sep 4, 2017

I don't think creating a new EIP is the way to go. The original EIP should imo specify how this new field is exposed. There is also a separate repository (https://github.com/ethereum/interfaces) that describes RPC changes.

Also I prefer to have the new field called outcome or result since it isn't really a status.

@Arachnid
Copy link
Contributor

Arachnid commented Sep 4, 2017

@bas-vk RPC interfaces aren't properly defined in core EIPs, IMO. There's no reason to name the status field there.

if receipt.PostState != nil {
fields["root"] = hexutil.Bytes(receipt.PostState)
} else {
fields["statusCode"] = hexutil.Uint(receiptStatusSuccessful)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use status instead

@holiman
Copy link
Contributor

holiman commented Sep 22, 2017

Updated docs: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionreceipt
LMK if it's incorrect

@rjl493456442
Copy link
Member Author

rjl493456442 commented Sep 22, 2017

@holiman The wiki has the inaccurate part. The status should be either '0x0' or '0x1'. Since all rpc responses are encoded in hex format

Copy link
Member

@bas-vk bas-vk left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution.

I have also updated the wiki.

@karalabe karalabe added this to the 1.7.1 milestone Oct 2, 2017
@karalabe
Copy link
Member

karalabe commented Oct 2, 2017

The PR was missing the parsing part of the status field. Adding that was not really trivial because we've previously used a Failed field in the receipt, which gets messy when parsing into from a status JSON field.

After a few variations and feedback from @fjl, the consensus was to have Receipt represent the true format of the consensus type, and replace Failed bool to Status uint. My followup commit on this PR does that + the necessary code integrations to make everything work together correctly.

@karalabe
Copy link
Member

karalabe commented Oct 2, 2017

@bas-vk PTAL

@bas-vk
Copy link
Member

bas-vk commented Oct 2, 2017

@karalabe LGTM

@karalabe karalabe merged commit f86c417 into ethereum:master Oct 2, 2017
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

Successfully merging this pull request may close these issues.

6 participants