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

Weird behaviour of HeaderRLP decoding #30841

Closed
mralj opened this issue Dec 1, 2024 · 2 comments
Closed

Weird behaviour of HeaderRLP decoding #30841

mralj opened this issue Dec 1, 2024 · 2 comments
Assignees

Comments

@mralj
Copy link

mralj commented Dec 1, 2024

Hello, I've been looking into the Header RLP enc/dec.
My understanding is that RLP encoding is code generated and located here, and the as far as decoder goes, there is no special logic (ie. since every field of the Header struct is RLP decodable so is the whole struct).

Encoding logic

Per encoding logic, we have this

	_tmp1 := obj.BaseFee != nil
	_tmp2 := obj.WithdrawalsHash != nil
	_tmp3 := obj.BlobGasUsed != nil
	_tmp4 := obj.ExcessBlobGas != nil
	_tmp5 := obj.ParentBeaconRoot != nil
	_tmp6 := obj.RequestsHash != nil
	_tmp7 := len(obj.GasLimits) > 0
	_tmp8 := len(obj.GasUsedVector) > 0
	_tmp9 := len(obj.ExcessGas) > 0
	if _tmp1 || _tmp2 || _tmp3 || _tmp4 || _tmp5 || _tmp6 || _tmp7 || _tmp8 || _tmp9 {
		if obj.BaseFee == nil {
			w.Write(rlp.EmptyString)
		} else {
			if obj.BaseFee.Sign() == -1 {
				return rlp.ErrNegativeBigInt
			}
			w.WriteBigInt(obj.BaseFee)
		}
	}
	if _tmp2 || _tmp3 || _tmp4 || _tmp5 || _tmp6 || _tmp7 || _tmp8 || _tmp9 {
		if obj.WithdrawalsHash == nil {
			w.Write([]byte{0x80})
		} else {
			w.WriteBytes(obj.WithdrawalsHash[:])
		}
	}
	if _tmp3 || _tmp4 || _tmp5 || _tmp6 || _tmp7 || _tmp8 || _tmp9 {
		if obj.BlobGasUsed == nil {
			w.Write([]byte{0x80})
		} else {
			w.WriteUint64((*obj.BlobGasUsed))
		}
	}  
  

This is to say if eg. BlobGas "exists", and even if BaseFee and WithdrawalHash are nil this header will be properly Encoded, but per my tests, it won't be decoded.

Tests

	header := &Header{
		ParentHash:  common.HexToHash("0x1234567890"),
		UncleHash:   common.HexToHash("0x9876543210"),
		Coinbase:    common.HexToAddress("0x1234567890123456789012345678901234567890"),
		Root:        common.HexToHash("0xabcdef0123"),
		TxHash:      common.HexToHash("0x4567890abc"),
		ReceiptHash: common.HexToHash("0x7890abcdef"),
		Difficulty:  big.NewInt(12345),
		Number:      big.NewInt(67890),
		GasLimit:    1000000,
		GasUsed:     500000,
		Time:        1234567890,
		Extra:       []byte("test extra data"),
		MixDigest:   common.HexToHash("0x1234567890abcdef"),
		Nonce:       [8]byte{1, 2, 3, 4, 5, 6, 7, 8},
	}

	testCases := []struct {
		name   string
		header *Header
	}{
		{
			name:   "WithWithdrawalsHash",
			header: (func() *Header { h := CopyHeader(header); h.WithdrawalsHash = &common.Hash{1, 2, 3}; return h })(),
		},
		{
			name: "WithBlobGasFields",
			header: (func() *Header {
				h := CopyHeader(header)

				blobGasUsed := uint64(2000)
				excessBlobGas := uint64(3000)
				h.BlobGasUsed = &blobGasUsed
				h.ExcessBlobGas = &excessBlobGas
				return h
			})(),
		},
         }
	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			encoded, err := rlp.EncodeToBytes(tc.header)
			if err != nil {
				t.Fatalf("Failed to encode header: %v", err)
			}

			var decoded Header
			if err := rlp.DecodeBytes(encoded, &decoded); err != nil {
				t.Fatalf("Failed to decode header: %v", err)
			}

			if !reflect.DeepEqual(*tc.header, decoded) {
				t.Fatalf("Decoded header not equal to original. \n Expected %v, \n got %v", *tc.header, decoded)
			}
		})
	}

Here both test cases will fail:

  1. "WithWithdrawalHash" fails because header.BaseFee is initially nil but it is decoded as 0, and while it might sound like bikeshedding, but 0 != nil
  2. "WithBlobGasFields" will fail with the following error Failed to decode header: rlp: input string too short for common.Hash, decoding into (types.Header).WithdrawalsHash

Questions

My questions are:

  1. Are my test correct (I think they are, I don't see any obvious bugs)
  2. Assuming they are, is this expected behaviour?
  3. If yes, how come/why? I would assume that if header was managed to be encoded successfully that it can be decoded as well
@fjl
Copy link
Contributor

fjl commented Jan 20, 2025

This code is created by rlpgen. Let me explain why it works this way:

Go structs are encoded as RLP lists. In package rlp, we have the concept of 'optional fields'. These fields can only be at the tail end of the struct. If an optional field is set, all preceding optional fields must also be encoded into the output to ensure the output list has the correct length. These preceding fields must be encoded even if they are zero-valued.

In types.Header, optional fields are used for attributes that have been added in hard forks. During block processing, the ordering of these fields is verified. Thus, even if it was theoretically possible to create a block header that has a BlobGasUsed but no BaseFee, such headers are invalid under the rules of Ethereum consensus, and will not exist in practice.

@mralj
Copy link
Author

mralj commented Jan 20, 2025

Hey!
Thank you very much for taking the time to explain this to me, it makes sense now :)

@mralj mralj closed this as completed Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants