-
Notifications
You must be signed in to change notification settings - Fork 281
feat: handle l1 rpc error #854
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
base: l1sload
Are you sure you want to change the base?
Changes from all commits
a306ca2
aac7d47
d3adddf
8f10e58
712fd3b
11ad398
c28fe4b
edeb6d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |||||
"encoding/binary" | ||||||
"errors" | ||||||
"math/big" | ||||||
"time" | ||||||
|
||||||
"github.com/scroll-tech/go-ethereum/common" | ||||||
"github.com/scroll-tech/go-ethereum/common/math" | ||||||
|
@@ -145,7 +146,7 @@ func PrecompiledContractsDescartes(cfg Config) map[common.Address]PrecompiledCon | |||||
common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{}, | ||||||
common.BytesToAddress([]byte{9}): &blake2FDisabled{}, | ||||||
// TODO final contract address to be decided | ||||||
common.BytesToAddress([]byte{1, 1}): &l1sload{l1Client: cfg.L1Client}, | ||||||
common.BytesToAddress([]byte{1, 1}): &l1sload{l1Client: cfg.L1Client, callerType: cfg.CallerType}, | ||||||
} | ||||||
} | ||||||
|
||||||
|
@@ -1164,7 +1165,8 @@ func (c *bls12381MapG2) Run(state StateDB, input []byte) ([]byte, error) { | |||||
|
||||||
// L1SLoad precompiled | ||||||
type l1sload struct { | ||||||
l1Client L1Client | ||||||
l1Client L1Client | ||||||
callerType CallerType | ||||||
} | ||||||
|
||||||
// RequiredGas returns the gas required to execute the pre-compiled contract. | ||||||
|
@@ -1179,6 +1181,9 @@ func (c *l1sload) RequiredGas(input []byte) uint64 { | |||||
} | ||||||
|
||||||
func (c *l1sload) Run(state StateDB, input []byte) ([]byte, error) { | ||||||
log.Info("l1sload", "input", input) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it intentional to set |
||||||
const l1ClientMaxRetries = 3 | ||||||
|
||||||
if c.l1Client == nil { | ||||||
log.Error("No L1Client in the l1sload") | ||||||
return nil, ErrNoL1Client | ||||||
|
@@ -1198,10 +1203,29 @@ func (c *l1sload) Run(state StateDB, input []byte) ([]byte, error) { | |||||
keys[i] = common.BytesToHash(input[20+32*i : 52+32*i]) | ||||||
} | ||||||
|
||||||
res, err := c.l1Client.StoragesAt(context.Background(), address, keys, block) | ||||||
if err != nil { | ||||||
return nil, &ErrL1RPCError{err: err} | ||||||
// if caller type is non-worker then we can retry request multiple times and return err, the tx will be reinserted in tx poll | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and use |
||||||
// otherwise, we should retry requests forever | ||||||
if c.callerType == CallerTypeNonWorker { | ||||||
for { | ||||||
res, err := c.l1Client.StoragesAt(context.Background(), address, keys, block) | ||||||
if err == nil { | ||||||
return res, nil | ||||||
} | ||||||
// wait before retrying | ||||||
time.Sleep(100 * time.Millisecond) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we do exponential backoff? |
||||||
log.Warn("L1 client request error", "err", err) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} else { | ||||||
var innerErr error | ||||||
for i := 0; i < l1ClientMaxRetries; i++ { | ||||||
res, err := c.l1Client.StoragesAt(context.Background(), address, keys, block) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this call have a built-in timeout mechanism? Or is it possible that it would hang indefinitely? |
||||||
if err != nil { | ||||||
innerErr = err | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about printing the number of |
||||||
continue | ||||||
} else { | ||||||
return res, nil | ||||||
} | ||||||
} | ||||||
return nil, &ErrL1RPCError{err: innerErr} | ||||||
} | ||||||
|
||||||
return res, nil | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ import ( | |
"github.com/scroll-tech/go-ethereum/core/rawdb" | ||
"github.com/scroll-tech/go-ethereum/core/state" | ||
"github.com/scroll-tech/go-ethereum/core/types" | ||
"github.com/scroll-tech/go-ethereum/core/vm" | ||
"github.com/scroll-tech/go-ethereum/event" | ||
"github.com/scroll-tech/go-ethereum/log" | ||
"github.com/scroll-tech/go-ethereum/metrics" | ||
|
@@ -846,7 +847,7 @@ func (w *worker) makeCurrent(parent *types.Block, header *types.Header) error { | |
// don't commit the state during tracing for circuit capacity checker, otherwise we cannot revert. | ||
// and even if we don't commit the state, the `refund` value will still be correct, as explained in `CommitTransaction` | ||
commitStateAfterApply := false | ||
traceEnv, err := tracing.CreateTraceEnv(w.chainConfig, w.chain, w.engine, w.eth.ChainDb(), state, w.chain.GetVMConfig().L1Client, parent, | ||
traceEnv, err := tracing.CreateTraceEnv(w.chainConfig, w.chain, w.engine, w.eth.ChainDb(), state, w.chain.GetVMConfig().L1Client, vm.CallerTypeWorker, parent, | ||
// new block with a placeholder tx, for traceEnv's ExecutionResults length & TxStorageTraces length | ||
types.NewBlockWithHeader(header).WithBody([]*types.Transaction{types.NewTx(&types.LegacyTx{})}, nil), | ||
commitStateAfterApply) | ||
|
@@ -1006,9 +1007,13 @@ func (w *worker) commitTransaction(tx *types.Transaction, coinbase common.Addres | |
// create new snapshot for `core.ApplyTransaction` | ||
snap := w.current.state.Snapshot() | ||
|
||
// todo: apply this changes to new worker when merged with upstream | ||
// make a copy of vm config and change caller type to worker | ||
var vmConf vm.Config = *w.chain.GetVMConfig() | ||
vmConf.CallerType = vm.CallerTypeWorker | ||
var receipt *types.Receipt | ||
common.WithTimer(l2CommitTxApplyTimer, func() { | ||
receipt, err = core.ApplyTransaction(w.chainConfig, w.chain, &coinbase, w.current.gasPool, w.current.state, w.current.header, tx, &w.current.header.GasUsed, *w.chain.GetVMConfig()) | ||
receipt, err = core.ApplyTransaction(w.chainConfig, w.chain, &coinbase, w.current.gasPool, w.current.state, w.current.header, tx, &w.current.header.GasUsed, vmConf) | ||
}) | ||
if err != nil { | ||
w.current.state.RevertToSnapshot(snap) | ||
|
@@ -1134,6 +1139,7 @@ loop: | |
w.current.state.SetTxContext(tx.Hash(), w.current.tcount) | ||
|
||
logs, traces, err := w.commitTransaction(tx, coinbase) | ||
var errL1 *vm.ErrL1RPCError | ||
switch { | ||
case errors.Is(err, core.ErrGasLimitReached) && tx.IsL1MessageTx(): | ||
// If this block already contains some L1 messages, | ||
|
@@ -1168,6 +1174,12 @@ loop: | |
log.Trace("Skipping account with hight nonce", "sender", from, "nonce", tx.Nonce()) | ||
txs.Pop() | ||
|
||
case errors.As(err, &errL1): | ||
Thegaram marked this conversation as resolved.
Show resolved
Hide resolved
NazariiDenha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Skip the current transaction failed on L1Sload precompile with L1RpcError without shifting in the next from the account, this tx will be left in txpool and retried in future block | ||
log.Trace("Skipping transaction failed on L1Sload precompile with L1RpcError", "sender", from) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log error |
||
atomic.AddInt32(&w.newTxs, int32(1)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? |
||
txs.Pop() | ||
|
||
case errors.Is(err, nil): | ||
NazariiDenha marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Everything ok, collect the logs and shift in the next transaction from the same account | ||
coalescedLogs = append(coalescedLogs, logs...) | ||
|
Uh oh!
There was an error while loading. Please reload this page.