Receipt fixes

[benjamincburns]

Makes it so that receipts are returned when EVM runtime errors occur (e.g. REVERT). Adds missing status, from, and to fields to the transaction receipt.

[aludvik]

@benjamincburns Since this is a pretty substantial change (some previous transactions that were previously invalid are now valid), can you summarize the discussion from RocketChat about why this is the desired behavior, what the pros and cons are, and what use cases this support.

[benjamincburns]

Go formatting issues are now fixed.

[benjamincburns]

@aludvik sure thing.

To be clear, this change doesn't allow for contract state updates when transaction execution fails. It simply records the transaction execution failure in the transaction receipt format that is appropriate for the transaction family.

Prior to these changes there was no way for a client which submitted its transactions via Seth's RPC interface to determine whether the transaction was executed, and what the result of that execution was.

Via the Ethereum RPC interface, the usual transaction flow looks like this:

1. Client submits a message call transaction to a contract address via either the eth_sendTransaction or the eth_sendRawTransaction RPC call
2. Server responds with RPC response containing transaction hash in the result field
3. Client uses the transaction hash returned by the server to poll for the result of the transaction via eth_getTransactionReceipt.
4. Until the transaction is mined into a block, the server always returns null in response to the client's eth_getTransactionReceipt request.
5. When the transaction is mined into a block, regardless of whether the transaction succeeded or failed, the server responds to eth_getTransactionReceipt with a valid transaction receipt.
6. The client inspects the status field in eth_getTransactionReceipt to determine whether their transaction succeeded or failed.

If no receipt is produced when a message call to a contract address fails due to a runtime error (e.g. due to the REVERT opcode), then the client can't tell the difference between transaction failure, and the transaction never starting.

[benjamincburns]

As to what use-cases this supports, I'm working on integrating the Truffle toolsuite with Sawtooth-Seth. See the blocked tickets on the commit-referenced Jira issue for more info.

[benjamincburns]

Looks like I had some rust formatting issues as well - those should be fixed up now.

[knkski]

Looks like this line is causing issues in Jenkins atm:

https://build.sawtooth.me/job/Sawtooth-Hyperledger/job/sawtooth-seth/view/change-requests/job/PR-77/7/console

[knkski]

Some minor feedback, but overall LGTM

[knkski]

Might be worth writing this as cls.account_address_b = bytes.fromhex(cls.account_address) so that the two definitions don't get out of sync

[benjamincburns]

good point - I missed that helper function - will fix.

[knkski]

Wondering what the purpose of setattr is here vs just passing it into the constructor

[benjamincburns]

Sadly due to a limitation of python you can't use reserved words as keyword arguments for a function. The protobuf devs seem to be aware of that limitation, but it's as yet unresolved, and setattr is the only viable workaround I could find.

[benjamincburns]

I'll add a comment to make this more clear, since I'm already going to do that bytes.fromhex fix you mentioned elsewhere...

[knkski]

Cool, makes sense. Another option is that you could do is SethTransactionReceipt(**{'from': self.account_address_b, ...}), but either way works

[aludvik]

@benjamincburns Thanks for the response. Am I correct in understanding there are three types of validity with Ethereum transactions?

1. Totally valid - The transaction "object" was valid and the execution was a success
2. Valid but failed - The transaction "object" was valid but the execution failed
3. Invalid - The transaction "object" was invalid because of something like a bad signature, bad payload, incorrect nonce.

Is that characterization accurate? Does the nonce still get updated if the transaction execution fails?

[benjamincburns]

@aludvik sorry that I didn't see your question above! Yes, your characterization is accurate. The nonce gets updated in cases 1 and 2, as in these cases a transaction receipt is generated. Transaction receipts are included in block hash calculations, and as such they're stored on chain under the transaction receipt storage trie data structure.

[benjamincburns]

Noticed that this branch has gone a bit stale, so I fixed up the outstanding conflicts. I've also ruthlessly culled the projects/issues I'm watching, so with any luck I won't miss future notifications on this PR!

[benjamincburns]

FAIL: Missing Signed-Off Field

I suppose that's what I get for fixing conflicts with GitHub's internal tooling - will fix and force-push.

[vaporos]

@benjamincburns Looks great.

Can you rebase on top of master (instead of adding the merge commit) and force-push?

Also, in your commit messages, can you remove the STL- from the first line? When we reference bugs (which isn't often currently), we prefer to follow https://chris.beams.io/posts/git-commit/ which is to put "Resolves: xxx" at the bottom of the commit message.

[benjamincburns]

I'm not sure where this left off and it's been quite some time since the original submission. I've moved on to other projects in that time, so I think it's best if I go ahead and close.