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

Data loss during MarshalBinary and UnmarshalBinary for Receipt #24073

Closed
quentinlesceller opened this issue Dec 6, 2021 · 3 comments
Closed
Labels

Comments

@quentinlesceller
Copy link

quentinlesceller commented Dec 6, 2021

Hi everyone,

I believe that since #22806 and the release of geth v1.10.10, I noticed that some data in the Receipt type are lost during marshalling. Previously the test below used to work correctly.

System information

Geth version: >=1.10.10
OS & Version: Tested on Linux and macOS

Expected behaviour

Receipt should be marshalled and unmarshaled without data loss.

Actual behaviour

When using MarshalBinary then UnmarshalBinary, the following fields are lost.

type Receipt struct {
        ....
        ...
	// Implementation fields: These fields are added by geth when processing a transaction.
	// They are stored in the chain database.
	TxHash          common.Hash    `json:"transactionHash" gencodec:"required"`
	ContractAddress common.Address `json:"contractAddress"`
	GasUsed         uint64         `json:"gasUsed" gencodec:"required"`

	// Inclusion information: These fields provide information about the inclusion of the
	// transaction corresponding to this receipt.
	BlockHash        common.Hash `json:"blockHash,omitempty"`
	BlockNumber      *big.Int    `json:"blockNumber,omitempty"`
	TransactionIndex uint        `json:"transactionIndex"`
}

Steps to reproduce the behaviour

Add the following test in core/types/receipt_test.go:

func TestReceiptMarshalBinary2(t *testing.T) {
	legacyFullReceipt := Receipt{
		Type:              1,
		PostState:         nil,
		Status:            ReceiptStatusSuccessful,
		CumulativeGasUsed: 30047541,
		TxHash:            common.HexToHash("0xf9a460cd85018d7ae76af7c170596d24c6c95fac959c15ac3237bf563b277348"),
		ContractAddress:   common.HexToAddress("0xf9a460cd85018d7ae76af7c170596d24c6c95fac959c15ac3237bf563b277348"),
		GasUsed:           46507,
		BlockHash:         common.HexToHash("0x64182164092a4d0d80e04d905b6298f8bb14625f2830466bffb26b583bcbdeb5"),
		BlockNumber:       big.NewInt(13753711),
		TransactionIndex:  368,
	}
	legacyReceipt.Bloom = CreateBloom(Receipts{&legacyFullReceipt})
	have, err := legacyFullReceipt.MarshalBinary()
	if err != nil {
		t.Fatalf("marshal binary error: %v", err)
	}
	gotLegacyFullReceipt := new(Receipt)
	if err := gotLegacyFullReceipt.UnmarshalBinary(have); err != nil {
		t.Fatalf("unmarshal binary error: %v", err)
	}
	if !reflect.DeepEqual(gotLegacyFullReceipt, legacyFullReceipt) {
		t.Errorf("receipt unmarshalled from binary mismatch, got %v want %v", gotLegacyFullReceipt, legacyFullReceipt)
	}
}

Backtrace

Running tool: /opt/homebrew/bin/go test -timeout 30s -run ^TestReceiptMarshalBinary2$ github.com/ethereum/go-ethereum/core/types

--- FAIL: TestReceiptMarshalBinary2 (0.00s)
    /Users/quentin/go/src/github.com/ethereum/go-ethereum/core/types/receipt_test.go:391: receipt unmarshalled from binary mismatch, got &{1 [] 1 30047541 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] [] 0x0000000000000000000000000000000000000000000000000000000000000000 0x0000000000000000000000000000000000000000 0 0x0000000000000000000000000000000000000000000000000000000000000000 <nil> 0} want {1 [] 1 30047541 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] [] 0xf9a460cd85018d7ae76af7c170596d24c6c95fac959c15ac3237bf563b277348 0x70596d24C6c95FAC959c15aC3237Bf563B277348 46507 0x64182164092a4d0d80e04d905b6298f8bb14625f2830466bffb26b583bcbdeb5 13753711 368}
@quentinlesceller quentinlesceller changed the title Data Loss during MarshalBinary and UnmarshalBinary for Receipt Data loss during MarshalBinary and UnmarshalBinary for Receipt Dec 6, 2021
@quentinlesceller
Copy link
Author

After reading some more on this. It seems like this is the expected behaviour? That's unfortunate because we used these values internally and I don't see at first glance a way to serialise it without losing data.

@rjl493456442
Copy link
Member

rjl493456442 commented Dec 7, 2021

From the description of the function // MarshalBinary returns the consensus encoding of the receipt. it's aimed to return the consensus encoding. Thus it omits the "unnecessary" fields when marshaling the receipt to derive a binary with consensus definition.

What's more, there are a few other receipt encodings in Geth, like storage encoding(which aims to store the minimal fields and recover all other fields in runtime) and json encoding. You might be interested in this https://github.com/ethereum/go-ethereum/blob/master/core/types/gen_receipt_json.go

Also in order to get all receipts fields, you need to use https://github.com/ethereum/go-ethereum/blob/master/core/rawdb/accessors_chain.go#L560 this API.

@quentinlesceller
Copy link
Author

Hi @rjl493456442,

Thank you for your answer. The main problem that we had is that the new encoding broke our usage since we use go rpc.
I guess that if it is a consensus thing we will probably use jsonrpc. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants