refactor(core/rawdb/accessors): Remove storedReceiptsRLP struct #735
Conversation
Add multiple raw DB receipt hex test vectors and iterate over them using t.Run to create subtests. Decode each receipt inside the subtest and remove hardcoded assertions. Add fmt import.
| err = rlp.DecodeBytes(rawDBReceiptBytes, sr) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
Note that this test actually still passes with https://github.com/ethereum-optimism/op-geth/pull/726/files#r2543824260.
This may not mean it is totally safe, since the test data here may not be sufficient to expose any problems.
There was a problem hiding this comment.
For a time I had thought this is because the decoder would fallback to decodeLegacyOptimismReceiptRLP for these test cases, but on further investigation that is not the case. We need a concrete example of a legacy receipt to add to this test.
Replace the local storedReceiptRLP with core/types.ReceiptForStorage in decoding code. Update tests to decode into ReceiptForStorage and compare FeeScalar as a big.Float. Use ReceiptForStorage, remove duplicate struct Mark legacy receipt fields as rlp:"optional" so mixed-format receipts can be parsed without errors. Update unit tests to decode legacy receipts into ReceiptForStorage and compare FeeScalar by parsing it into a big.Float to match the stored representation
geoknee
changed the title
| rawDBReceiptBytes, err := hexutil.Decode(rawDBReceiptHex) | ||
| require.NoError(t, err) | ||
|
|
||
| sr := new([]receiptLogs) |
There was a problem hiding this comment.
I think this should be
| sr := new([]receiptLogs) | |
| sr := make([]*types.ReceiptForStorage, 0) |
There was a problem hiding this comment.
I think it would trigger the same RLP decoder. I think I chose to do it this way since #726 just changes the implementation of the decoder on receiptLogs to use ReceiptForStorage.
op-geth/core/rawdb/accessors_chain.go
Lines 673 to 681 in 4722287
Preparatory work for #726