Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfixes: Prevent unnecessary tx copying, WorkObject-Header view nil checks #1750

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions core/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package core

import (
"fmt"
"math/big"

"github.com/dominant-strategies/go-quai/common"
Expand Down Expand Up @@ -54,7 +55,7 @@ type ChainContext interface {
}

// NewEVMBlockContext creates a new context for use in the EVM.
func NewEVMBlockContext(header *types.WorkObject, chain ChainContext, author *common.Address) vm.BlockContext {
func NewEVMBlockContext(header *types.WorkObject, chain ChainContext, author *common.Address) (vm.BlockContext, error) {
var (
beneficiary common.Address
baseFee *big.Int
Expand All @@ -76,13 +77,18 @@ func NewEVMBlockContext(header *types.WorkObject, chain ChainContext, author *co
if parent != nil {
timestamp = parent.Time()
} else {
log.Global.Fatal("Parent is not in the db, panic", "headerHash", header.Hash(), "parentHash", header.ParentHash(chain.NodeCtx()), "number", header.Number(chain.NodeCtx()).Uint64())
log.Global.Error("Parent is not in the db, cannot append", "headerHash", header.Hash(), "parentHash", header.ParentHash(chain.NodeCtx()), "number", header.Number(chain.NodeCtx()).Uint64())
return vm.BlockContext{}, fmt.Errorf("parent header not found for block %s, parent %s", header.Hash(), header.ParentHash(chain.NodeCtx()))
}
}

// Prime terminus determines which location is eligible to except the etx
primeTerminus := header.PrimeTerminus()
primeTerminusHeader := chain.GetHeaderByHash(primeTerminus)
if primeTerminusHeader == nil {
log.Global.Error("Prime terminus header not found", "headerHash", header.Hash(), "primeTerminus", primeTerminus)
return vm.BlockContext{}, ErrSubNotSyncedToDom
}
etxEligibleSlices := primeTerminusHeader.EtxEligibleSlices()

return vm.BlockContext{
Expand All @@ -97,7 +103,7 @@ func NewEVMBlockContext(header *types.WorkObject, chain ChainContext, author *co
GasLimit: header.GasLimit(),
CheckIfEtxEligible: chain.CheckIfEtxIsEligible,
EtxEligibleSlices: etxEligibleSlices,
}
}, nil
}

// NewEVMTxContext creates a new transaction context for a single transaction.
Expand Down
18 changes: 16 additions & 2 deletions core/rawdb/accessors_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,12 @@ func HasHeader(db ethdb.Reader, hash common.Hash, number uint64) bool {

// ReadHeader retrieves the block header corresponding to the hash.
func ReadHeader(db ethdb.Reader, hash common.Hash, number uint64) *types.WorkObject {
return ReadWorkObjectHeaderOnly(db, hash, types.BlockObject)
wo := ReadWorkObjectHeaderOnly(db, hash, types.BlockObject)
if wo == nil || wo.Body() == nil || wo.Header() == nil {
// Try backup function
return ReadWorkObject(db, hash, types.BlockObject)
}
return wo
}

// WriteHeader stores a block header into the database and also stores the hash-
Expand Down Expand Up @@ -675,6 +680,7 @@ func ReadWorkObjectBody(db ethdb.Reader, hash common.Hash) *types.WorkObjectBody
err := proto.Unmarshal(data, protoWorkObjectBody)
if err != nil {
db.Logger().WithField("err", err).Fatal("Failed to proto Unmarshal work object body")
return nil
}
workObjectBody := new(types.WorkObjectBody)
err = workObjectBody.ProtoDecode(protoWorkObjectBody, db.Location())
Expand All @@ -698,9 +704,17 @@ func ReadWorkObjectBodyHeaderOnly(db ethdb.Reader, hash common.Hash) *types.Work
err := proto.Unmarshal(data, protoWorkObjectBody)
if err != nil {
log.Global.WithField("err", err).Fatal("Failed to proto Unmarshal work object body")
return nil
}
workObjectBody := new(types.WorkObjectBody)
workObjectBody.ProtoDecodeHeader(protoWorkObjectBody, db.Location())
err = workObjectBody.ProtoDecodeHeader(protoWorkObjectBody, db.Location())
if workObjectBody.Header() == nil || err != nil {
log.Global.WithFields(log.Fields{
"hash": hash,
"err": err,
}).Error("workObjectBody Header is nil")
return nil
}
return workObjectBody
}

Expand Down
15 changes: 12 additions & 3 deletions core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,10 @@ func (p *StateProcessor) Process(block *types.WorkObject, etxSet *types.EtxSet)
}
p.hc.pool.SendersMu.RUnlock()
timeSenders = time.Since(startTimeSenders)
blockContext := NewEVMBlockContext(header, p.hc, nil)
blockContext, err := NewEVMBlockContext(header, p.hc, nil)
if err != nil {
return nil, nil, nil, nil, 0, err
}
vmenv := vm.NewEVM(blockContext, vm.TxContext{}, statedb, p.config, p.vmConfig)
time3 := common.PrettyDuration(time.Since(start))

Expand Down Expand Up @@ -933,7 +936,10 @@ func ApplyTransaction(config *params.ChainConfig, parent *types.WorkObject, bc C
msg.SetData([]byte{}) // data is not used in conversion
}
// Create a new context to be used in the EVM environment
blockContext := NewEVMBlockContext(header, bc, author)
blockContext, err := NewEVMBlockContext(header, bc, author)
if err != nil {
return nil, err
}
vmenv := vm.NewEVM(blockContext, vm.TxContext{}, statedb, config, cfg)
if tx.Type() == types.ExternalTxType {
prevZeroBal := prepareApplyETX(statedb, msg.Value(), config.Location)
Expand Down Expand Up @@ -1222,7 +1228,10 @@ func (p *StateProcessor) StateAtTransaction(block *types.WorkObject, txIndex int
// Assemble the transaction call message and return if the requested offset
msg, _ := tx.AsMessage(signer, block.BaseFee())
txContext := NewEVMTxContext(msg)
context := NewEVMBlockContext(block, p.hc, nil)
context, err := NewEVMBlockContext(block, p.hc, nil)
if err != nil {
return nil, vm.BlockContext{}, nil, err
}
if idx == txIndex {
return msg, context, statedb, nil
}
Expand Down
10 changes: 7 additions & 3 deletions core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ func (tx *Transaction) SetInner(inner TxData) {
tx.setDecoded(inner.copy(), 0)
}

func (tx *Transaction) SetInnerNoCopy(inner TxData) {
tx.setDecoded(inner, 0)
}

// TxData is the underlying data of a transaction.
//
// This is implemented by QuaiTx, ExternalTx, InternalToExternal, and QiTx.
Expand Down Expand Up @@ -288,7 +292,7 @@ func (tx *Transaction) ProtoDecode(protoTx *ProtoTransaction, location common.Lo
nonce := BlockNonce(uint64ToByteArr(*protoTx.WorkNonce))
quaiTx.WorkNonce = &nonce
}
tx.SetInner(&quaiTx)
tx.SetInnerNoCopy(&quaiTx)

case 1:
if protoTx.Gas == nil {
Expand Down Expand Up @@ -326,7 +330,7 @@ func (tx *Transaction) ProtoDecode(protoTx *ProtoTransaction, location common.Lo
etx.ETXIndex = uint16(protoTx.GetEtxIndex())
etx.Sender = common.BytesToAddress(protoTx.GetEtxSender(), location)

tx.SetInner(&etx)
tx.SetInnerNoCopy(&etx)

case 2:
if protoTx.TxIns == nil {
Expand Down Expand Up @@ -379,7 +383,7 @@ func (tx *Transaction) ProtoDecode(protoTx *ProtoTransaction, location common.Lo
nonce := BlockNonce(uint64ToByteArr(*protoTx.WorkNonce))
qiTx.WorkNonce = &nonce
}
tx.SetInner(&qiTx)
tx.SetInnerNoCopy(&qiTx)

default:
return errors.New("invalid transaction type")
Expand Down
5 changes: 4 additions & 1 deletion quai/api_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ func (b *QuaiAPIBackend) GetEVM(ctx context.Context, msg core.Message, state *st
vmConfig = b.quai.core.GetVMConfig()
}
txContext := core.NewEVMTxContext(msg)
context := core.NewEVMBlockContext(header, b.quai.Core(), nil)
context, err := core.NewEVMBlockContext(header, b.quai.Core(), nil)
if err != nil {
return nil, vmError, err
}
return vm.NewEVM(context, txContext, state, b.quai.core.Config(), *vmConfig), vmError, nil
}

Expand Down
5 changes: 4 additions & 1 deletion quai/tracers/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,10 @@ func (b *testBackend) StateAtTransaction(ctx context.Context, block *types.Block
for idx, tx := range block.Transactions() {
msg, _ := tx.AsMessage(signer, block.BaseFee())
txContext := core.NewEVMTxContext(msg)
context := core.NewEVMBlockContext(block.Header(), b.chain, nil)
context, err := core.NewEVMBlockContext(block.Header(), b.chain, nil)
if err != nil {
return nil, vm.BlockContext{}, nil, fmt.Errorf("failed to create block context: %v", err)
}
if idx == txIndex {
return msg, context, statedb, nil
}
Expand Down
Loading