From f8aa62353666a6368fb3f1a378bd0a82d1542052 Mon Sep 17 00:00:00 2001 From: Roberto Bayardo Date: Tue, 25 Apr 2023 23:37:11 -0700 Subject: [PATCH] core/types: fix discrepancy in receipt.EffectiveGasPrice json encoding tags (#27114) Regenerate receipt json code to remove omit empty. Previously, there was a discrepancy between the generated code and the source. --------- Co-authored-by: lightclient@protonmail.com Co-authored-by: Martin Holst Swende --- cmd/evm/t8n_test.go | 7 +-- cmd/evm/testdata/1/exp.json | 1 + cmd/evm/testdata/13/exp2.json | 2 + cmd/evm/testdata/23/exp.json | 1 + cmd/evm/testdata/24/exp.json | 2 + cmd/evm/testdata/25/exp.json | 1 + cmd/evm/testdata/3/exp.json | 1 + core/types/gen_receipt_json.go | 4 +- core/types/receipt.go | 3 +- core/types/receipt_test.go | 83 ++++++++++++++++++++++++---------- 10 files changed, 76 insertions(+), 29 deletions(-) diff --git a/cmd/evm/t8n_test.go b/cmd/evm/t8n_test.go index 067660c7e..0ef3fac40 100644 --- a/cmd/evm/t8n_test.go +++ b/cmd/evm/t8n_test.go @@ -275,7 +275,8 @@ func TestT8n(t *testing.T) { tt.Run("evm-test", args...) // Compare the expected output, if provided if tc.expOut != "" { - want, err := os.ReadFile(fmt.Sprintf("%v/%v", tc.base, tc.expOut)) + file := fmt.Sprintf("%v/%v", tc.base, tc.expOut) + want, err := os.ReadFile(file) if err != nil { t.Fatalf("test %d: could not read expected output: %v", i, err) } @@ -283,9 +284,9 @@ func TestT8n(t *testing.T) { ok, err := cmpJson(have, want) switch { case err != nil: - t.Fatalf("test %d, json parsing failed: %v", i, err) + t.Fatalf("test %d, file %v: json parsing failed: %v", i, file, err) case !ok: - t.Fatalf("test %d: output wrong, have \n%v\nwant\n%v\n", i, string(have), string(want)) + t.Fatalf("test %d, file %v: output wrong, have \n%v\nwant\n%v\n", i, file, string(have), string(want)) } } tt.WaitExit() diff --git a/cmd/evm/testdata/1/exp.json b/cmd/evm/testdata/1/exp.json index 7d3805012..d1351e5b7 100644 --- a/cmd/evm/testdata/1/exp.json +++ b/cmd/evm/testdata/1/exp.json @@ -28,6 +28,7 @@ "transactionHash": "0x0557bacce3375c98d806609b8d5043072f0b6a8bae45ae5a67a00d3a1a18d673", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x5208", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" } diff --git a/cmd/evm/testdata/13/exp2.json b/cmd/evm/testdata/13/exp2.json index cbad6552c..babce3592 100644 --- a/cmd/evm/testdata/13/exp2.json +++ b/cmd/evm/testdata/13/exp2.json @@ -16,6 +16,7 @@ "transactionHash": "0xa98a24882ea90916c6a86da650fbc6b14238e46f0af04a131ce92be897507476", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x84d0", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" }, @@ -29,6 +30,7 @@ "transactionHash": "0x36bad80acce7040c45fd32764b5c2b2d2e6f778669fb41791f73f546d56e739a", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x84d0", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x1" } diff --git a/cmd/evm/testdata/23/exp.json b/cmd/evm/testdata/23/exp.json index e51f37d9c..22dde0a27 100644 --- a/cmd/evm/testdata/23/exp.json +++ b/cmd/evm/testdata/23/exp.json @@ -15,6 +15,7 @@ "transactionHash": "0x72fadbef39cd251a437eea619cfeda752271a5faaaa2147df012e112159ffb81", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x520b", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" } diff --git a/cmd/evm/testdata/24/exp.json b/cmd/evm/testdata/24/exp.json index d8cec59d6..ac571d149 100644 --- a/cmd/evm/testdata/24/exp.json +++ b/cmd/evm/testdata/24/exp.json @@ -31,6 +31,7 @@ "transactionHash": "0x92ea4a28224d033afb20e0cc2b290d4c7c2d61f6a4800a680e4e19ac962ee941", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0xa861", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" }, @@ -43,6 +44,7 @@ "transactionHash": "0x16b1d912f1d664f3f60f4e1b5f296f3c82a64a1a253117b4851d18bc03c4f1da", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x5aa5", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x1" } diff --git a/cmd/evm/testdata/25/exp.json b/cmd/evm/testdata/25/exp.json index a9c310a1e..1cb521794 100644 --- a/cmd/evm/testdata/25/exp.json +++ b/cmd/evm/testdata/25/exp.json @@ -27,6 +27,7 @@ "transactionHash": "0x92ea4a28224d033afb20e0cc2b290d4c7c2d61f6a4800a680e4e19ac962ee941", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x5208", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" } diff --git a/cmd/evm/testdata/3/exp.json b/cmd/evm/testdata/3/exp.json index 71b3d2f55..7230dca2c 100644 --- a/cmd/evm/testdata/3/exp.json +++ b/cmd/evm/testdata/3/exp.json @@ -28,6 +28,7 @@ "transactionHash": "0x72fadbef39cd251a437eea619cfeda752271a5faaaa2147df012e112159ffb81", "contractAddress": "0x0000000000000000000000000000000000000000", "gasUsed": "0x521f", + "effectiveGasPrice": null, "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "transactionIndex": "0x0" } diff --git a/core/types/gen_receipt_json.go b/core/types/gen_receipt_json.go index 8d85dd5b9..d83be1447 100644 --- a/core/types/gen_receipt_json.go +++ b/core/types/gen_receipt_json.go @@ -25,7 +25,7 @@ func (r Receipt) MarshalJSON() ([]byte, error) { TxHash common.Hash `json:"transactionHash" gencodec:"required"` ContractAddress common.Address `json:"contractAddress"` GasUsed hexutil.Uint64 `json:"gasUsed" gencodec:"required"` - EffectiveGasPrice *hexutil.Big `json:"effectiveGasPrice,omitempty"` + EffectiveGasPrice *hexutil.Big `json:"effectiveGasPrice"` BlockHash common.Hash `json:"blockHash,omitempty"` BlockNumber *hexutil.Big `json:"blockNumber,omitempty"` TransactionIndex hexutil.Uint `json:"transactionIndex"` @@ -59,7 +59,7 @@ func (r *Receipt) UnmarshalJSON(input []byte) error { TxHash *common.Hash `json:"transactionHash" gencodec:"required"` ContractAddress *common.Address `json:"contractAddress"` GasUsed *hexutil.Uint64 `json:"gasUsed" gencodec:"required"` - EffectiveGasPrice *hexutil.Big `json:"effectiveGasPrice,omitempty"` + EffectiveGasPrice *hexutil.Big `json:"effectiveGasPrice"` BlockHash *common.Hash `json:"blockHash,omitempty"` BlockNumber *hexutil.Big `json:"blockNumber,omitempty"` TransactionIndex *hexutil.Uint `json:"transactionIndex"` diff --git a/core/types/receipt.go b/core/types/receipt.go index 6300886c0..8fc9ec989 100644 --- a/core/types/receipt.go +++ b/core/types/receipt.go @@ -62,7 +62,7 @@ type Receipt struct { TxHash common.Hash `json:"transactionHash" gencodec:"required"` ContractAddress common.Address `json:"contractAddress"` GasUsed uint64 `json:"gasUsed" gencodec:"required"` - EffectiveGasPrice *big.Int `json:"effectiveGasPrice"` + EffectiveGasPrice *big.Int `json:"effectiveGasPrice"` // required, but tag omitted for backwards compatibility // Inclusion information: These fields provide information about the inclusion of the // transaction corresponding to this receipt. @@ -77,6 +77,7 @@ type receiptMarshaling struct { Status hexutil.Uint64 CumulativeGasUsed hexutil.Uint64 GasUsed hexutil.Uint64 + EffectiveGasPrice *hexutil.Big BlockNumber *hexutil.Big TransactionIndex hexutil.Uint } diff --git a/core/types/receipt_test.go b/core/types/receipt_test.go index 00031ad56..168f36208 100644 --- a/core/types/receipt_test.go +++ b/core/types/receipt_test.go @@ -82,27 +82,15 @@ var ( }, Type: DynamicFeeTxType, } -) -func TestDecodeEmptyTypedReceipt(t *testing.T) { - input := []byte{0x80} - var r Receipt - err := rlp.DecodeBytes(input, &r) - if err != errShortTypedReceipt { - t.Fatal("wrong error:", err) - } -} - -// Tests that receipt data can be correctly derived from the contextual infos -func TestDeriveFields(t *testing.T) { // Create a few transactions to have receipts for - to2 := common.HexToAddress("0x2") - to3 := common.HexToAddress("0x3") - to4 := common.HexToAddress("0x4") - to5 := common.HexToAddress("0x5") - to6 := common.HexToAddress("0x6") - to7 := common.HexToAddress("0x7") - txs := Transactions{ + to2 = common.HexToAddress("0x2") + to3 = common.HexToAddress("0x3") + to4 = common.HexToAddress("0x4") + to5 = common.HexToAddress("0x5") + to6 = common.HexToAddress("0x6") + to7 = common.HexToAddress("0x7") + txs = Transactions{ NewTx(&LegacyTx{ Nonce: 1, Value: big.NewInt(1), @@ -161,18 +149,19 @@ func TestDeriveFields(t *testing.T) { }), } - blockNumber := big.NewInt(1) - blockTime := uint64(2) - blockHash := common.BytesToHash([]byte{0x03, 0x14}) + blockNumber = big.NewInt(1) + blockTime = uint64(2) + blockHash = common.BytesToHash([]byte{0x03, 0x14}) // Create the corresponding receipts - receipts := Receipts{ + receipts = Receipts{ &Receipt{ Status: ReceiptStatusFailed, CumulativeGasUsed: 1, Logs: []*Log{ { Address: common.BytesToAddress([]byte{0x11}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, // derived fields: BlockNumber: blockNumber.Uint64(), TxHash: txs[0].Hash(), @@ -182,6 +171,7 @@ func TestDeriveFields(t *testing.T) { }, { Address: common.BytesToAddress([]byte{0x01, 0x11}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, // derived fields: BlockNumber: blockNumber.Uint64(), TxHash: txs[0].Hash(), @@ -205,6 +195,7 @@ func TestDeriveFields(t *testing.T) { Logs: []*Log{ { Address: common.BytesToAddress([]byte{0x22}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, // derived fields: BlockNumber: blockNumber.Uint64(), TxHash: txs[1].Hash(), @@ -214,6 +205,7 @@ func TestDeriveFields(t *testing.T) { }, { Address: common.BytesToAddress([]byte{0x02, 0x22}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, // derived fields: BlockNumber: blockNumber.Uint64(), TxHash: txs[1].Hash(), @@ -296,7 +288,19 @@ func TestDeriveFields(t *testing.T) { TransactionIndex: 6, }, } +) +func TestDecodeEmptyTypedReceipt(t *testing.T) { + input := []byte{0x80} + var r Receipt + err := rlp.DecodeBytes(input, &r) + if err != errShortTypedReceipt { + t.Fatal("wrong error:", err) + } +} + +// Tests that receipt data can be correctly derived from the contextual infos +func TestDeriveFields(t *testing.T) { // Re-derive receipts. basefee := big.NewInt(1000) derivedReceipts := clearComputedFieldsOnReceipts(receipts) @@ -310,6 +314,7 @@ func TestDeriveFields(t *testing.T) { if err != nil { t.Fatal("error marshaling input receipts:", err) } + r2, err := json.MarshalIndent(derivedReceipts, "", " ") if err != nil { t.Fatal("error marshaling derived receipts:", err) @@ -320,6 +325,38 @@ func TestDeriveFields(t *testing.T) { } } +// Test that we can marshal/unmarshal receipts to/from json without errors. +// This also confirms that our test receipts contain all the required fields. +func TestReceiptJSON(t *testing.T) { + for i := range receipts { + b, err := receipts[i].MarshalJSON() + if err != nil { + t.Fatal("error marshaling receipt to json:", err) + } + r := Receipt{} + err = r.UnmarshalJSON(b) + if err != nil { + t.Fatal("error unmarshaling receipt from json:", err) + } + } +} + +// Test we can still parse receipt without EffectiveGasPrice for backwards compatibility, even +// though it is required per the spec. +func TestEffectiveGasPriceNotRequired(t *testing.T) { + r := *receipts[0] + r.EffectiveGasPrice = nil + b, err := r.MarshalJSON() + if err != nil { + t.Fatal("error marshaling receipt to json:", err) + } + r2 := Receipt{} + err = r2.UnmarshalJSON(b) + if err != nil { + t.Fatal("error unmarshaling receipt from json:", err) + } +} + // TestTypedReceiptEncodingDecoding reproduces a flaw that existed in the receipt // rlp decoder, which failed due to a shadowing error. func TestTypedReceiptEncodingDecoding(t *testing.T) {