Skip to content

Commit

Permalink
fix: account for tx subcircuit confidence factor (#1069)
Browse files Browse the repository at this point in the history
* fix: account for tx subcircuit confidence factor (#1065)

* fix types

* fix `TestValidateTxBlockSize`

---------

Co-authored-by: Ömer Faruk Irmak <[email protected]>
  • Loading branch information
0xmountaintop and omerfirmak authored Oct 14, 2024
1 parent 3d88e87 commit 20f8ade
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 8 deletions.
3 changes: 2 additions & 1 deletion core/block_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"time"

"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/consensus"
"github.com/scroll-tech/go-ethereum/core/rawdb"
"github.com/scroll-tech/go-ethereum/core/state"
Expand Down Expand Up @@ -77,7 +78,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
return consensus.ErrInvalidTxCount
}
// Check if block payload size is smaller than the max size
if !v.config.Scroll.IsValidBlockSize(block.PayloadSize()) {
if !v.config.Scroll.IsValidBlockSize(common.StorageSize(block.PayloadSize())) {
return ErrInvalidBlockPayloadSize
}

Expand Down
2 changes: 1 addition & 1 deletion core/txpool/legacypool/legacypool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2740,7 +2740,7 @@ func TestValidateTxBlockSize(t *testing.T) {
account := crypto.PubkeyToAddress(key.PublicKey)
testAddBalance(pool, account, big.NewInt(1000000000000000000))

validTx := pricedDataTransaction(1, 2100000, big.NewInt(1), key, uint64(*pool.chainconfig.Scroll.MaxTxPayloadBytesPerBlock)-128)
validTx := pricedDataTransaction(1, 2100000, big.NewInt(1), key, uint64(float64(*pool.chainconfig.Scroll.MaxTxPayloadBytesPerBlock)*float64(0.9)))
oversizedTx := pricedDataTransaction(2, 2100000, big.NewInt(1), key, uint64(*pool.chainconfig.Scroll.MaxTxPayloadBytesPerBlock))

tests := []struct {
Expand Down
2 changes: 1 addition & 1 deletion core/txpool/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func ValidateTransaction(tx *types.Transaction, head *types.Header, signer types
return fmt.Errorf("%w: transaction size %v, limit %v", ErrOversizedData, tx.Size(), opts.MaxSize)
}
// Reject transactions that cannot fit into a block even as a single transaction
if !opts.Config.Scroll.IsValidBlockSize(tx.Size()) {
if !opts.Config.Scroll.IsValidBlockSizeForMining(common.StorageSize(tx.Size())) {
return ErrOversizedData
}
// Ensure only transactions that have been enabled are accepted
Expand Down
2 changes: 1 addition & 1 deletion miner/scroll_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ func (w *worker) processTxn(tx *types.Transaction) (bool, error) {
return false, ErrUnexpectedL1MessageIndex
}

if !tx.IsL1MessageTx() && !w.chain.Config().Scroll.IsValidBlockSize(uint64(w.current.blockSize)+tx.Size()) {
if !tx.IsL1MessageTx() && !w.chain.Config().Scroll.IsValidBlockSizeForMining(w.current.blockSize+common.StorageSize(tx.Size())) {
// can't fit this txn in this block, silently ignore and continue looking for more txns
return false, errors.New("tx too big")
}
Expand Down
2 changes: 1 addition & 1 deletion miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ loop:
)
break
}
if !tx.IsL1MessageTx() && !w.chainConfig.Scroll.IsValidBlockSize(env.blockSize+tx.Size()) {
if !tx.IsL1MessageTx() && !w.chainConfig.Scroll.IsValidBlockSizeForMining(common.StorageSize(env.blockSize+tx.Size())) {
log.Trace("Block size limit reached", "have", env.blockSize, "want", w.chainConfig.Scroll.MaxTxPayloadBytesPerBlock, "tx", tx.Size())
txs.Pop() // skip transactions from this account
continue
Expand Down
9 changes: 7 additions & 2 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,13 @@ func (s ScrollConfig) IsValidTxCount(count int) bool {
}

// IsValidBlockSize returns whether the given block's transaction payload size is below the limit.
func (s ScrollConfig) IsValidBlockSize(size uint64) bool {
return s.MaxTxPayloadBytesPerBlock == nil || size <= uint64(*s.MaxTxPayloadBytesPerBlock)
func (s ScrollConfig) IsValidBlockSize(size common.StorageSize) bool {
return s.MaxTxPayloadBytesPerBlock == nil || size <= common.StorageSize(*s.MaxTxPayloadBytesPerBlock)
}

// IsValidBlockSizeForMining is similar to IsValidBlockSize, but it accounts for the confidence factor in Rust CCC
func (s ScrollConfig) IsValidBlockSizeForMining(size common.StorageSize) bool {
return s.IsValidBlockSize(size * (1.0 / 0.95))
}

func (s ScrollConfig) String() string {
Expand Down
3 changes: 2 additions & 1 deletion rollup/pipeline/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"
"unsafe"

"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/core"
"github.com/scroll-tech/go-ethereum/core/state"
"github.com/scroll-tech/go-ethereum/core/txpool"
Expand Down Expand Up @@ -306,7 +307,7 @@ func (p *Pipeline) traceAndApplyStage(txsIn <-chan *txpool.LazyTransaction) (<-c
continue
}

if !tx.IsL1MessageTx() && !p.chain.Config().Scroll.IsValidBlockSize(p.blockSize+tx.Size()) {
if !tx.IsL1MessageTx() && !p.chain.Config().Scroll.IsValidBlockSizeForMining(common.StorageSize(p.blockSize+tx.Size())) {
// can't fit this txn in this block, silently ignore and continue looking for more txns
sendCancellable(resCh, nil, p.ctx.Done())
continue
Expand Down

0 comments on commit 20f8ade

Please sign in to comment.