Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Receipt fixes and state trie clearing detail #62

Merged
merged 4 commits into from
Jun 17, 2019

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jun 15, 2019

Changes how the receipts are encoded and decoded to follow proper protocol of replacing intermediate state root with status encoding. Also performs state trie clearing at some appropriate points in a simulated system that was previously missed (see: state.IntermediateRoot(config.IsAtlantis(parent.Number()) like changes).

Since previous receipts would have had status encoded and saved into the database, I have made the decoding backwards compatible:

// DecodeRLP implements rlp.Decoder, and loads both consensus and implementation
// fields of a receipt from an RLP stream.
func (r *ReceiptForStorage) DecodeRLP(s *rlp.Stream) error {
	raw, err := s.Raw()
	if err != nil {
		return err
	}

	// Try decoding the receipt without Status first
	if err := decodeStoredReceiptRLP(r, raw); err == nil {
		return nil
	}

	return decodeOldStoredReceiptRLP(r, raw)
}

where decodeOldStoredReceiptRLP does not assume the ambiguous nature of PostStateOrStatus field of the Receipts and sets both fields specifically.

Also cleaned up the struct declaration for encoding/ decoding because encoding and decoding an untyped interface relying on positional variables and the confusing logic structure was very bad.

Edit: Resolves #55

@soc1c
Copy link
Contributor

soc1c commented Jun 17, 2019

Is this EIP-658?

@austinabell
Copy link
Contributor Author

Is this EIP-658?

yes, the change necessary to enable 658 is included in this PR:

rlp.Encode(w, []interface{}{r.statusEncoding(), r.CumulativeGasUsed, r.Bloom, r.Logs})

and all surrounding logic

from:

rlp.Encode(w, []interface{}{r.PostState, r.CumulativeGasUsed, r.Bloom, r.Logs})

@GregTheGreek
Copy link

GregTheGreek commented Jun 17, 2019

Failed contract creation should not have success = 1

Reproduce-able test below:

EDIT: Test was incorrect, I added a to field which no longer made it a contract deployment.

~> eth.sendTransaction({from:eth.coinbase, to:'0x8b282d2123e77b9bc4b83eb732175e0406f3354a', data: "60606040525b6000151560125760006000fd5b5b5b60338060216000396000f30060606040525bfe00a165627a7a723058203d54db7a632777e6c9add0977c74024489bc162189d3ae9588c83b3561a0d1c30029", gas:45000});

"0x0a3fed3cfbf56324977c4b2ec86d4201c6f3a7805909f1e3bff1b6b6de2ca3d6"

 > eth.getTransactionReceipt("0x0a3fed3cfbf56324977c4b2ec86d4201c6f3a7805909f1e3bff1b6b6de2ca3d6")

{
  blockHash: "0xd3db64f50db9e14935621202cef4189756f08c8cbb6a973adcf934967deddc27",
  blockNumber: 40357,
  contractAddress: null,
  cumulativeGasUsed: 26200,
  from: "0x6ac28d519781d6321da388358c79cf08de1dddce",
  gasUsed: 26200,
  logs: [],
  root: "",
  status: 1,
  to: "0x8b282d2123e77b9bc4b83eb732175e0406f3354a",
  transactionHash: "0x0a3fed3cfbf56324977c4b2ec86d4201c6f3a7805909f1e3bff1b6b6de2ca3d6",
  transactionIndex: 0
}

Bytecode is equivalent to this (use this version of remix to verify):

pragma solidity 0.4.10;

contract Test {
    
    function Test() payable {
        require(false);
    }
}

@noot
Copy link
Contributor

noot commented Jun 17, 2019

@GregTheGreek you have a to field in your transaction, contract deployments don't have to. you're sending a normal tx which should be successful

@GregTheGreek
Copy link

GregTheGreek commented Jun 17, 2019

@noot woops thanks for that catch, disregard previous comment this looks to be functioning correctly

> eth.sendTransaction({from:eth.coinbase, data: "60606040525b6000151560125760006000fd5b5b5b60338060216000396000f30060606040525bfe00a165627a7a723058203d54db7a632777e6c9add0977c74024489bc162189d3ae9588c83b3561a0d1c30029", gas:90000});

"0xf2645dd6cc063096a2e326cc65834af3567560bd5b1de824ca3e4b5f31977746"

> eth.getTransactionReceipt("0xf2645dd6cc063096a2e326cc65834af3567560bd5b1de824ca3e4b5f31977746")

{
  blockHash: "0x6de5d8ccac56a2b3e529cd3ebe29a6ff4c59262c337c57651bce043e4958d7af",
  blockNumber: 40379,
  contractAddress: "0x9d1d7c22db1b325dfeb7603bcad1f487ef093afe",
  cumulativeGasUsed: 58247,
  from: "0x6ac28d519781d6321da388358c79cf08de1dddce",
  gasUsed: 58247,
  logs: [],
  root: "",
  status: 0,
  to: null,
  transactionHash: "0xf2645dd6cc063096a2e326cc65834af3567560bd5b1de824ca3e4b5f31977746",
  transactionIndex: 0
}

@soc1c soc1c merged commit 8c43b48 into development Jun 17, 2019
@soc1c soc1c deleted the austin/stcreceiptfix branch June 17, 2019 20:45
dziabko pushed a commit that referenced this pull request Jun 20, 2019
* receipt and state trie clearing changes

* Fixed receipt storage and added backward compatible decoding
dziabko pushed a commit that referenced this pull request Jun 20, 2019
* receipt and state trie clearing changes

* Fixed receipt storage and added backward compatible decoding
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transactions causes split with Parity
4 participants