From c4a662176ec11b9d5718904ccefee753637ab377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 26 Oct 2022 15:23:07 +0300 Subject: [PATCH] core, eth: for types with accurate size calcs, return uint64, not float (#26046) * core, eth: for types with accurate size calcs, return uint64, not float * core/types: proper tx size tests * core/types: extend tx size test with decoded sizes, fix error * core/txpool: fix linter Co-authored-by: Martin Holst Swende --- core/blockchain.go | 2 +- core/txpool/txpool.go | 2 +- core/types/block.go | 12 ++--- core/types/block_test.go | 6 +-- core/types/transaction.go | 29 +++++----- core/types/transaction_marshalling.go | 76 +++++++++++++-------------- core/types/transaction_test.go | 68 ++++++++++++++++++++++++ eth/downloader/queue.go | 2 +- eth/protocols/eth/broadcast.go | 2 +- les/downloader/queue.go | 2 +- 10 files changed, 137 insertions(+), 64 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 4965eeef35b4..a1d402517dad 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1913,7 +1913,7 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator) (i // Import all the pruned blocks to make the state available var ( blocks []*types.Block - memory common.StorageSize + memory uint64 ) for i := len(hashes) - 1; i >= 0; i-- { // Append the next block to our batch diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 2e02c46101da..8905140fdb69 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -595,7 +595,7 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { return core.ErrTxTypeNotSupported } // Reject transactions over defined size to prevent DOS attacks - if uint64(tx.Size()) > txMaxSize { + if tx.Size() > txMaxSize { return ErrOversizedData } // Transactions can't be negative. This may never happen using RLP decoded diff --git a/core/types/block.go b/core/types/block.go index 8942082b6e48..603a3f771208 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -263,7 +263,7 @@ func (b *Block) DecodeRLP(s *rlp.Stream) error { return err } b.header, b.uncles, b.transactions = eb.Header, eb.Uncles, eb.Txs - b.size.Store(common.StorageSize(rlp.ListSize(size))) + b.size.Store(rlp.ListSize(size)) return nil } @@ -322,14 +322,14 @@ func (b *Block) Body() *Body { return &Body{b.transactions, b.uncles} } // Size returns the true RLP encoded storage size of the block, either by encoding // and returning it, or returning a previously cached value. -func (b *Block) Size() common.StorageSize { +func (b *Block) Size() uint64 { if size := b.size.Load(); size != nil { - return size.(common.StorageSize) + return size.(uint64) } c := writeCounter(0) rlp.Encode(&c, b) - b.size.Store(common.StorageSize(c)) - return common.StorageSize(c) + b.size.Store(uint64(c)) + return uint64(c) } // SanityCheck can be used to prevent that unbounded fields are @@ -338,7 +338,7 @@ func (b *Block) SanityCheck() error { return b.header.SanityCheck() } -type writeCounter common.StorageSize +type writeCounter uint64 func (c *writeCounter) Write(b []byte) (int, error) { *c += writeCounter(len(b)) diff --git a/core/types/block_test.go b/core/types/block_test.go index 9e7f581b1dc4..49197c923764 100644 --- a/core/types/block_test.go +++ b/core/types/block_test.go @@ -53,7 +53,7 @@ func TestBlockEncoding(t *testing.T) { check("Hash", block.Hash(), common.HexToHash("0a5843ac1cb04865017cb35a57b50b07084e5fcee39b5acadade33149f4fff9e")) check("Nonce", block.Nonce(), uint64(0xa13a5a8c8f2bb1c4)) check("Time", block.Time(), uint64(1426516743)) - check("Size", block.Size(), common.StorageSize(len(blockEnc))) + check("Size", block.Size(), uint64(len(blockEnc))) tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), 50000, big.NewInt(10), nil) tx1, _ = tx1.WithSignature(HomesteadSigner{}, common.Hex2Bytes("9bea4c4daac7c7c52e093e6a4c35dbbcf8856f1af7b059ba20253e70848d094f8a8fae537ce25ed8cb5af9adac3f141af69bd515bd2ba031522df09b97dd72b100")) @@ -90,7 +90,7 @@ func TestEIP1559BlockEncoding(t *testing.T) { check("Hash", block.Hash(), common.HexToHash("c7252048cd273fe0dac09650027d07f0e3da4ee0675ebbb26627cea92729c372")) check("Nonce", block.Nonce(), uint64(0xa13a5a8c8f2bb1c4)) check("Time", block.Time(), uint64(1426516743)) - check("Size", block.Size(), common.StorageSize(len(blockEnc))) + check("Size", block.Size(), uint64(len(blockEnc))) check("BaseFee", block.BaseFee(), new(big.Int).SetUint64(params.InitialBaseFee)) tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), 50000, big.NewInt(10), nil) @@ -153,7 +153,7 @@ func TestEIP2718BlockEncoding(t *testing.T) { check("Root", block.Root(), common.HexToHash("ef1552a40b7165c3cd773806b9e0c165b75356e0314bf0706f279c729f51e017")) check("Nonce", block.Nonce(), uint64(0xa13a5a8c8f2bb1c4)) check("Time", block.Time(), uint64(1426516743)) - check("Size", block.Size(), common.StorageSize(len(blockEnc))) + check("Size", block.Size(), uint64(len(blockEnc))) // Create legacy tx. to := common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87") diff --git a/core/types/transaction.go b/core/types/transaction.go index 715ede15db2e..910c68aea363 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -131,7 +131,7 @@ func (tx *Transaction) DecodeRLP(s *rlp.Stream) error { var inner LegacyTx err := s.Decode(&inner) if err == nil { - tx.setDecoded(&inner, int(rlp.ListSize(size))) + tx.setDecoded(&inner, rlp.ListSize(size)) } return err default: @@ -142,7 +142,7 @@ func (tx *Transaction) DecodeRLP(s *rlp.Stream) error { } inner, err := tx.decodeTyped(b) if err == nil { - tx.setDecoded(inner, len(b)) + tx.setDecoded(inner, uint64(len(b))) } return err } @@ -158,7 +158,7 @@ func (tx *Transaction) UnmarshalBinary(b []byte) error { if err != nil { return err } - tx.setDecoded(&data, len(b)) + tx.setDecoded(&data, uint64(len(b))) return nil } // It's an EIP2718 typed transaction envelope. @@ -166,7 +166,7 @@ func (tx *Transaction) UnmarshalBinary(b []byte) error { if err != nil { return err } - tx.setDecoded(inner, len(b)) + tx.setDecoded(inner, uint64(len(b))) return nil } @@ -190,11 +190,11 @@ func (tx *Transaction) decodeTyped(b []byte) (TxData, error) { } // setDecoded sets the inner transaction and size after decoding. -func (tx *Transaction) setDecoded(inner TxData, size int) { +func (tx *Transaction) setDecoded(inner TxData, size uint64) { tx.inner = inner tx.time = time.Now() if size > 0 { - tx.size.Store(common.StorageSize(size)) + tx.size.Store(size) } } @@ -372,16 +372,21 @@ func (tx *Transaction) Hash() common.Hash { return h } -// Size returns the true RLP encoded storage size of the transaction, either by -// encoding and returning it, or returning a previously cached value. -func (tx *Transaction) Size() common.StorageSize { +// Size returns the true encoded storage size of the transaction, either by encoding +// and returning it, or returning a previously cached value. +func (tx *Transaction) Size() uint64 { if size := tx.size.Load(); size != nil { - return size.(common.StorageSize) + return size.(uint64) } c := writeCounter(0) rlp.Encode(&c, &tx.inner) - tx.size.Store(common.StorageSize(c)) - return common.StorageSize(c) + + size := uint64(c) + if tx.Type() != LegacyTxType { + size += 1 // type byte + } + tx.size.Store(size) + return size } // WithSignature returns a new transaction with the given signature. diff --git a/core/types/transaction_marshalling.go b/core/types/transaction_marshalling.go index aad31a5a97e2..2566d0b8d656 100644 --- a/core/types/transaction_marshalling.go +++ b/core/types/transaction_marshalling.go @@ -51,55 +51,55 @@ type txJSON struct { } // MarshalJSON marshals as JSON with a hash. -func (t *Transaction) MarshalJSON() ([]byte, error) { +func (tx *Transaction) MarshalJSON() ([]byte, error) { var enc txJSON // These are set for all tx types. - enc.Hash = t.Hash() - enc.Type = hexutil.Uint64(t.Type()) + enc.Hash = tx.Hash() + enc.Type = hexutil.Uint64(tx.Type()) // Other fields are set conditionally depending on tx type. - switch tx := t.inner.(type) { + switch itx := tx.inner.(type) { case *LegacyTx: - enc.Nonce = (*hexutil.Uint64)(&tx.Nonce) - enc.Gas = (*hexutil.Uint64)(&tx.Gas) - enc.GasPrice = (*hexutil.Big)(tx.GasPrice) - enc.Value = (*hexutil.Big)(tx.Value) - enc.Data = (*hexutil.Bytes)(&tx.Data) - enc.To = t.To() - enc.V = (*hexutil.Big)(tx.V) - enc.R = (*hexutil.Big)(tx.R) - enc.S = (*hexutil.Big)(tx.S) + enc.Nonce = (*hexutil.Uint64)(&itx.Nonce) + enc.Gas = (*hexutil.Uint64)(&itx.Gas) + enc.GasPrice = (*hexutil.Big)(itx.GasPrice) + enc.Value = (*hexutil.Big)(itx.Value) + enc.Data = (*hexutil.Bytes)(&itx.Data) + enc.To = tx.To() + enc.V = (*hexutil.Big)(itx.V) + enc.R = (*hexutil.Big)(itx.R) + enc.S = (*hexutil.Big)(itx.S) case *AccessListTx: - enc.ChainID = (*hexutil.Big)(tx.ChainID) - enc.AccessList = &tx.AccessList - enc.Nonce = (*hexutil.Uint64)(&tx.Nonce) - enc.Gas = (*hexutil.Uint64)(&tx.Gas) - enc.GasPrice = (*hexutil.Big)(tx.GasPrice) - enc.Value = (*hexutil.Big)(tx.Value) - enc.Data = (*hexutil.Bytes)(&tx.Data) - enc.To = t.To() - enc.V = (*hexutil.Big)(tx.V) - enc.R = (*hexutil.Big)(tx.R) - enc.S = (*hexutil.Big)(tx.S) + enc.ChainID = (*hexutil.Big)(itx.ChainID) + enc.AccessList = &itx.AccessList + enc.Nonce = (*hexutil.Uint64)(&itx.Nonce) + enc.Gas = (*hexutil.Uint64)(&itx.Gas) + enc.GasPrice = (*hexutil.Big)(itx.GasPrice) + enc.Value = (*hexutil.Big)(itx.Value) + enc.Data = (*hexutil.Bytes)(&itx.Data) + enc.To = tx.To() + enc.V = (*hexutil.Big)(itx.V) + enc.R = (*hexutil.Big)(itx.R) + enc.S = (*hexutil.Big)(itx.S) case *DynamicFeeTx: - enc.ChainID = (*hexutil.Big)(tx.ChainID) - enc.AccessList = &tx.AccessList - enc.Nonce = (*hexutil.Uint64)(&tx.Nonce) - enc.Gas = (*hexutil.Uint64)(&tx.Gas) - enc.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap) - enc.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap) - enc.Value = (*hexutil.Big)(tx.Value) - enc.Data = (*hexutil.Bytes)(&tx.Data) - enc.To = t.To() - enc.V = (*hexutil.Big)(tx.V) - enc.R = (*hexutil.Big)(tx.R) - enc.S = (*hexutil.Big)(tx.S) + enc.ChainID = (*hexutil.Big)(itx.ChainID) + enc.AccessList = &itx.AccessList + enc.Nonce = (*hexutil.Uint64)(&itx.Nonce) + enc.Gas = (*hexutil.Uint64)(&itx.Gas) + enc.MaxFeePerGas = (*hexutil.Big)(itx.GasFeeCap) + enc.MaxPriorityFeePerGas = (*hexutil.Big)(itx.GasTipCap) + enc.Value = (*hexutil.Big)(itx.Value) + enc.Data = (*hexutil.Bytes)(&itx.Data) + enc.To = tx.To() + enc.V = (*hexutil.Big)(itx.V) + enc.R = (*hexutil.Big)(itx.R) + enc.S = (*hexutil.Big)(itx.S) } return json.Marshal(&enc) } // UnmarshalJSON unmarshals from JSON. -func (t *Transaction) UnmarshalJSON(input []byte) error { +func (tx *Transaction) UnmarshalJSON(input []byte) error { var dec txJSON if err := json.Unmarshal(input, &dec); err != nil { return err @@ -268,7 +268,7 @@ func (t *Transaction) UnmarshalJSON(input []byte) error { } // Now set the inner transaction. - t.setDecoded(inner, 0) + tx.setDecoded(inner, 0) // TODO: check hash here? return nil diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 8e8ee595c971..4b96c6b91a01 100644 --- a/core/types/transaction_test.go +++ b/core/types/transaction_test.go @@ -531,3 +531,71 @@ func assertEqual(orig *Transaction, cpy *Transaction) error { } return nil } + +func TestTransactionSizes(t *testing.T) { + signer := NewLondonSigner(big.NewInt(123)) + key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + to := common.HexToAddress("0x01") + for i, txdata := range []TxData{ + &AccessListTx{ + ChainID: big.NewInt(123), + Nonce: 0, + To: nil, + Value: big.NewInt(1000), + Gas: 21000, + GasPrice: big.NewInt(100000), + }, + &LegacyTx{ + Nonce: 1, + GasPrice: big.NewInt(500), + Gas: 1000000, + To: &to, + Value: big.NewInt(1), + }, + &AccessListTx{ + ChainID: big.NewInt(123), + Nonce: 1, + GasPrice: big.NewInt(500), + Gas: 1000000, + To: &to, + Value: big.NewInt(1), + AccessList: AccessList{ + AccessTuple{ + Address: common.HexToAddress("0x01"), + StorageKeys: []common.Hash{common.HexToHash("0x01")}, + }}, + }, + &DynamicFeeTx{ + ChainID: big.NewInt(123), + Nonce: 1, + Gas: 1000000, + To: &to, + Value: big.NewInt(1), + GasTipCap: big.NewInt(500), + GasFeeCap: big.NewInt(500), + }, + } { + tx, err := SignNewTx(key, signer, txdata) + if err != nil { + t.Fatalf("test %d: %v", i, err) + } + bin, _ := tx.MarshalBinary() + + // Check initial calc + if have, want := int(tx.Size()), len(bin); have != want { + t.Errorf("test %d: size wrong, have %d want %d", i, have, want) + } + // Check cached version too + if have, want := int(tx.Size()), len(bin); have != want { + t.Errorf("test %d: (cached) size wrong, have %d want %d", i, have, want) + } + // Check unmarshalled version too + utx := new(Transaction) + if err := utx.UnmarshalBinary(bin); err != nil { + t.Fatalf("test %d: failed to unmarshal tx: %v", i, err) + } + if have, want := int(utx.Size()), len(bin); have != want { + t.Errorf("test %d: (unmarshalled) size wrong, have %d want %d", i, have, want) + } + } +} diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 0b500484b860..60a83a7fb0d9 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -369,7 +369,7 @@ func (q *queue) Results(block bool) []*fetchResult { size += receipt.Size() } for _, tx := range result.Transactions { - size += tx.Size() + size += common.StorageSize(tx.Size()) } q.resultSize = common.StorageSize(blockCacheSizeWeight)*size + (1-common.StorageSize(blockCacheSizeWeight))*q.resultSize diff --git a/eth/protocols/eth/broadcast.go b/eth/protocols/eth/broadcast.go index 6fc15f136bff..0afe01b1ce15 100644 --- a/eth/protocols/eth/broadcast.go +++ b/eth/protocols/eth/broadcast.go @@ -82,7 +82,7 @@ func (p *Peer) broadcastTransactions() { for i := 0; i < len(queue) && size < maxTxPacketSize; i++ { if tx := p.txpool.Get(queue[i]); tx != nil { txs = append(txs, tx) - size += tx.Size() + size += common.StorageSize(tx.Size()) } hashesCount++ } diff --git a/les/downloader/queue.go b/les/downloader/queue.go index fe08c810a11f..5b7054cf35cb 100644 --- a/les/downloader/queue.go +++ b/les/downloader/queue.go @@ -373,7 +373,7 @@ func (q *queue) Results(block bool) []*fetchResult { size += receipt.Size() } for _, tx := range result.Transactions { - size += tx.Size() + size += common.StorageSize(tx.Size()) } q.resultSize = common.StorageSize(blockCacheSizeWeight)*size + (1-common.StorageSize(blockCacheSizeWeight))*q.resultSize