From 7a57e38679cfd3ef4c5038643a19dff148ae21b7 Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Mon, 14 Aug 2023 20:43:42 -0300 Subject: [PATCH 1/5] Block Size Limit (5mb, gingerbreadP2) --- core/block_validator.go | 9 +++++++++ core/bytesblock.go | 38 ++++++++++++++++++++++++++++++++++++ core/error.go | 4 ++++ core/state_processor_test.go | 19 ++++++++++++++++++ miner/block.go | 27 ++++++++++++++++++++----- params/protocol_params.go | 3 ++- 6 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 core/bytesblock.go diff --git a/core/block_validator.go b/core/block_validator.go index a96ffca124..b6f6e84dfd 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -64,6 +64,15 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { } return consensus.ErrPrunedAncestor } + if v.config.IsGingerbreadP2(block.Number()) { + bytesBlock := new(BytesBlock).AddBytes(params.MaxTxDataPerBlock) + + for _, tx := range block.Transactions() { + if err := bytesBlock.SubBytes(uint64(tx.Size())); err != nil { + return err + } + } + } return nil } diff --git a/core/bytesblock.go b/core/bytesblock.go new file mode 100644 index 0000000000..d40ef1eff7 --- /dev/null +++ b/core/bytesblock.go @@ -0,0 +1,38 @@ +package core + +import ( + "fmt" + "math" +) + +// BytesBlock tracks the amount of bytes available during execution of the transactions +// in a block. The zero value is a block with zero bytes available. +type BytesBlock uint64 + +// AddBytes makes bytes available to use. +func (bp *BytesBlock) AddBytes(amount uint64) *BytesBlock { + if uint64(*bp) > math.MaxUint64-amount { + panic("block's bytes pushed above uint64") + } + *(*uint64)(bp) += amount + return bp +} + +// SubBytes deducts the given amount from the block if enough bytes are +// available and returns an error otherwise. +func (bp *BytesBlock) SubBytes(amount uint64) error { + if uint64(*bp) < amount { + return ErrGasLimitReached + } + *(*uint64)(bp) -= amount + return nil +} + +// BytesLeft returns the amount of gas remaining in the pool. +func (bp *BytesBlock) BytesLeft() uint64 { + return uint64(*bp) +} + +func (bp *BytesBlock) String() string { + return fmt.Sprintf("%d", *bp) +} diff --git a/core/error.go b/core/error.go index 7e4f7f77d9..e968ec9ad5 100644 --- a/core/error.go +++ b/core/error.go @@ -55,6 +55,10 @@ var ( // by a transaction is higher than what's left in the block. ErrGasLimitReached = errors.New("gas limit reached") + // ErrBytesLimitReached is returned if the amount of bytes required + // by a transaction is higher than what's left in the block. + ErrBytesLimitReached = errors.New("block's bytes limit reached") + // ErrInsufficientFundsForTransfer is returned if the transaction sender doesn't // have enough funds for transfer(topmost call only). // Note that the check for this is done after buying the gas. diff --git a/core/state_processor_test.go b/core/state_processor_test.go index bf0336053e..08826ad394 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -210,6 +210,13 @@ func TestStateProcessorErrors(t *testing.T) { }, want: "could not apply tx 0 [0xd82a0c2519acfeac9a948258c47e784acd20651d9d80f9a1c67b4137651c3a24]: insufficient funds for gas * price + value + gatewayFee: address 0x71562b71999873DB5b286dF957af199Ec94617F7 have 1000000000000000000 want 2431633873983640103894990685182446064918669677978451844828609264166175722438635000", }, + { + name: "ErrBytesLimitReached", + txs: []*types.Transaction{ + makeTx(0, common.Address{}, big.NewInt(0), params.TxGas+params.TxDataZeroGas*params.MaxTxDataPerBlock, big.NewInt(875000000), nil, nil, nil, getBigData(int(params.MaxTxDataPerBlock))), + }, + want: ErrBytesLimitReached.Error(), + }, } { block := GenerateBadBlock(genesis, mockEngine.NewFaker(), tt.txs, gspec.Config) _, err := blockchain.InsertChain(types.Blocks{block}) @@ -348,8 +355,20 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr receipts = append(receipts, receipt) cumulativeGas += tx.Gas() } + header.GasUsed = cumulativeGas header.Root = common.BytesToHash(hasher.Sum(nil)) // Assemble and return the final block for sealing return types.NewBlock(header, txs, receipts, nil, trie.NewStackTrie(nil)) +} + +func fillZero(slice []byte, length int) { + for i := 0; i < length && i < len(slice); i++ { + slice[i] = 0x00 + } +} +func getBigData(length int) []byte { + f := make([]byte, length) + fillZero(f, length) + return f } diff --git a/miner/block.go b/miner/block.go index 82dbed5c0e..5192bf7d21 100644 --- a/miner/block.go +++ b/miner/block.go @@ -41,11 +41,12 @@ import ( type blockState struct { signer types.Signer - state *state.StateDB // apply state changes here - tcount int // tx count in cycle - gasPool *core.GasPool // available gas used to pack transactions - gasLimit uint64 - sysCtx *core.SysContractCallCtx + state *state.StateDB // apply state changes here + tcount int // tx count in cycle + gasPool *core.GasPool // available gas used to pack transactions + bytesBlock *core.BytesBlock // available bytes used to pack transactions + gasLimit uint64 + sysCtx *core.SysContractCallCtx header *types.Header txs []*types.Transaction @@ -121,6 +122,9 @@ func prepareBlock(w *worker) (*blockState, error) { parentVmRunner := w.chain.NewEVMRunner(parent.Header(), state.Copy()) header.BaseFee = misc.CalcBaseFee(w.chainConfig, parent.Header(), parentVmRunner) } + if w.chainConfig.IsGingerbreadP2(header.Number) { + b.bytesBlock = new(core.BytesBlock).AddBytes(params.MaxTxDataPerBlock) + } b.sysCtx = core.NewSysContractCallCtx(header, state.Copy(), w.chain) // Play our part in generating the random beacon. @@ -255,6 +259,12 @@ loop: txs.Pop() continue } + // Same short-circuit of the gas above, but for bytes in the block (b.bytesBlock != nil => GingerbreadP2) + if b.bytesBlock != nil && b.bytesBlock.BytesLeft() < uint64(tx.Size()) { + log.Trace("Skipping transaction which requires more bytes than is left in the block", "hash", tx.Hash(), "bytes", b.bytesBlock.BytesLeft(), "txbytes", uint64(tx.Size())) + txs.Pop() + continue + } // Error may be ignored here. The error has already been checked // during transaction acceptance is the transaction pool. // @@ -298,6 +308,13 @@ loop: // Everything ok, collect the logs and shift in the next transaction from the same account coalescedLogs = append(coalescedLogs, logs...) b.tcount++ + // bytesBlock != nil => GngerbreadP2 + if b.bytesBlock != nil { + if err := b.bytesBlock.SubBytes(uint64(tx.Size())); err != nil { + // This should never happen because we are validating before that we have enough space + return err + } + } txs.Shift() default: diff --git a/params/protocol_params.go b/params/protocol_params.go index d5e19ffaf5..61ccc35bdf 100644 --- a/params/protocol_params.go +++ b/params/protocol_params.go @@ -120,7 +120,8 @@ const ( ElasticityMultiplier = 2 // Bounds the maximum gas limit an EIP-1559 block may have. InitialBaseFee = 1000000000 // Initial base fee for EIP-1559 blocks. - MaxCodeSize = 65536 // Maximum bytecode to permit for a contract (2^16) + MaxCodeSize = 65536 // Maximum bytecode to permit for a contract (2^16) + MaxTxDataPerBlock uint64 = 1024 * 1024 * 5 // Maximum size of all the rlp encoded transactions of a block (5mb) [Gingerbread] // Precompiled contract gas prices From a9b00814134d066080b48b3073c6916955c15edf Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Mon, 14 Aug 2023 21:48:48 -0300 Subject: [PATCH 2/5] Validate proposal --- consensus/istanbul/backend/backend.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/consensus/istanbul/backend/backend.go b/consensus/istanbul/backend/backend.go index b2b257f5c2..14992c5e30 100644 --- a/consensus/istanbul/backend/backend.go +++ b/consensus/istanbul/backend/backend.go @@ -621,6 +621,17 @@ func (sb *Backend) Verify(proposal istanbul.Proposal) (*istanbulCore.StateProces } } + if sb.chain.Config().IsGingerbreadP2(block.Number()) { + bytesBlock := new(core.BytesBlock).AddBytes(params.MaxTxDataPerBlock) + + for _, tx := range block.Transactions() { + if err := bytesBlock.SubBytes(uint64(tx.Size())); err != nil { + sb.logger.Error("verify - Error in validating txs block size", "err", err) + return nil, 0, err + } + } + } + // Apply this block's transactions to update the state receipts, logs, usedGas, err := sb.processBlock(block, state) if err != nil { From d8d8e3aa9b8acbd0ac6c68f048fb5fb9eb46e738 Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Mon, 14 Aug 2023 23:53:08 -0300 Subject: [PATCH 3/5] Fix correct error --- core/bytesblock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/bytesblock.go b/core/bytesblock.go index d40ef1eff7..9a4adcab28 100644 --- a/core/bytesblock.go +++ b/core/bytesblock.go @@ -22,7 +22,7 @@ func (bp *BytesBlock) AddBytes(amount uint64) *BytesBlock { // available and returns an error otherwise. func (bp *BytesBlock) SubBytes(amount uint64) error { if uint64(*bp) < amount { - return ErrGasLimitReached + return ErrBytesLimitReached } *(*uint64)(bp) -= amount return nil From e75f6d61de0bae96087b51cd382cce1dcb456bfe Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Wed, 16 Aug 2023 12:08:40 -0300 Subject: [PATCH 4/5] Fix typo in comment --- miner/block.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miner/block.go b/miner/block.go index 5192bf7d21..82124441ff 100644 --- a/miner/block.go +++ b/miner/block.go @@ -308,7 +308,7 @@ loop: // Everything ok, collect the logs and shift in the next transaction from the same account coalescedLogs = append(coalescedLogs, logs...) b.tcount++ - // bytesBlock != nil => GngerbreadP2 + // bytesBlock != nil => GingerbreadP2 if b.bytesBlock != nil { if err := b.bytesBlock.SubBytes(uint64(tx.Size())); err != nil { // This should never happen because we are validating before that we have enough space From 755a9be22e85c57c9ed554d083cc5ca2b8ff4a0a Mon Sep 17 00:00:00 2001 From: Gaston Ponti Date: Thu, 17 Aug 2023 18:50:43 -0300 Subject: [PATCH 5/5] Refactor code --- consensus/istanbul/backend/backend.go | 10 +++------- core/block_validator.go | 17 ++++++++++++----- core/bytesblock.go | 4 ++-- core/state_processor_test.go | 10 +--------- miner/block.go | 2 +- 5 files changed, 19 insertions(+), 24 deletions(-) diff --git a/consensus/istanbul/backend/backend.go b/consensus/istanbul/backend/backend.go index 14992c5e30..b33da9618b 100644 --- a/consensus/istanbul/backend/backend.go +++ b/consensus/istanbul/backend/backend.go @@ -622,13 +622,9 @@ func (sb *Backend) Verify(proposal istanbul.Proposal) (*istanbulCore.StateProces } if sb.chain.Config().IsGingerbreadP2(block.Number()) { - bytesBlock := new(core.BytesBlock).AddBytes(params.MaxTxDataPerBlock) - - for _, tx := range block.Transactions() { - if err := bytesBlock.SubBytes(uint64(tx.Size())); err != nil { - sb.logger.Error("verify - Error in validating txs block size", "err", err) - return nil, 0, err - } + if err := core.ValidateBlockSize(block, params.MaxTxDataPerBlock); err != nil { + sb.logger.Error("verify - Error in validating txs block size", "err", err) + return nil, 0, err } } diff --git a/core/block_validator.go b/core/block_validator.go index b6f6e84dfd..c62de63bbe 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -65,12 +65,19 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { return consensus.ErrPrunedAncestor } if v.config.IsGingerbreadP2(block.Number()) { - bytesBlock := new(BytesBlock).AddBytes(params.MaxTxDataPerBlock) + if err := ValidateBlockSize(block, params.MaxTxDataPerBlock); err != nil { + return err + } + } + return nil +} + +func ValidateBlockSize(block *types.Block, blockSize uint64) error { + bytesBlock := new(BytesBlock).SetLimit(blockSize) - for _, tx := range block.Transactions() { - if err := bytesBlock.SubBytes(uint64(tx.Size())); err != nil { - return err - } + for _, tx := range block.Transactions() { + if err := bytesBlock.SubBytes(uint64(tx.Size())); err != nil { + return err } } return nil diff --git a/core/bytesblock.go b/core/bytesblock.go index 9a4adcab28..32dcc85df4 100644 --- a/core/bytesblock.go +++ b/core/bytesblock.go @@ -9,8 +9,8 @@ import ( // in a block. The zero value is a block with zero bytes available. type BytesBlock uint64 -// AddBytes makes bytes available to use. -func (bp *BytesBlock) AddBytes(amount uint64) *BytesBlock { +// SetLimit makes bytes available to use. +func (bp *BytesBlock) SetLimit(amount uint64) *BytesBlock { if uint64(*bp) > math.MaxUint64-amount { panic("block's bytes pushed above uint64") } diff --git a/core/state_processor_test.go b/core/state_processor_test.go index 08826ad394..0dbf8eb26f 100644 --- a/core/state_processor_test.go +++ b/core/state_processor_test.go @@ -361,14 +361,6 @@ func GenerateBadBlock(parent *types.Block, engine consensus.Engine, txs types.Tr return types.NewBlock(header, txs, receipts, nil, trie.NewStackTrie(nil)) } -func fillZero(slice []byte, length int) { - for i := 0; i < length && i < len(slice); i++ { - slice[i] = 0x00 - } -} - func getBigData(length int) []byte { - f := make([]byte, length) - fillZero(f, length) - return f + return make([]byte, length) } diff --git a/miner/block.go b/miner/block.go index 82124441ff..d7832500c8 100644 --- a/miner/block.go +++ b/miner/block.go @@ -123,7 +123,7 @@ func prepareBlock(w *worker) (*blockState, error) { header.BaseFee = misc.CalcBaseFee(w.chainConfig, parent.Header(), parentVmRunner) } if w.chainConfig.IsGingerbreadP2(header.Number) { - b.bytesBlock = new(core.BytesBlock).AddBytes(params.MaxTxDataPerBlock) + b.bytesBlock = new(core.BytesBlock).SetLimit(params.MaxTxDataPerBlock) } b.sysCtx = core.NewSysContractCallCtx(header, state.Copy(), w.chain)