[R4R]-{develop}:Optimize the rollup service #1301
Conversation
Ethanncnm
requested review from
abelliumnt,
wukongcheng,
byteflyfunny and
guoshijiang
mt-batcher/flags/flags.go
Outdated
|
|
||
| MinTimeoutRollupTxnFlag = cli.Uint64Flag{ | ||
| Name: "min-rollup-txn-timeout", | ||
| Usage: "Rollup transaction timeout transactions for mantle da", |
There was a problem hiding this comment.
pls modify these usages, can not understand what they are used for
There was a problem hiding this comment.
The specific criterion for determining whether a Rollup can occur when a timeout happens：the minimum number of accumulated transactions that need to be met.
| rangeLen := end.Uint64() - start.Uint64() | ||
| if rangeLen < d.cfg.MinStateRootElements { | ||
| rollupTxn := end.Uint64() - start.Uint64() | ||
| if rollupTxn < d.cfg.MinStateRootElements && (time.Now().Add(-d.cfg.RollupTimeout).Before(d.lastCommitTime) || rollupTxn < d.cfg.MinTimeoutStateRootElements) { |
There was a problem hiding this comment.
use Sub instead of Add maybe better
There was a problem hiding this comment.
use sub will cost more code change.I suggest just use add.
guoshijiang
changed the title
| @@ -234,6 +236,22 @@ func (d *Driver) GetBatchBlockRange(ctx context.Context) (*big.Int, *big.Int, er | |||
| return start, end, nil | |||
| } | |||
|
|
|||
| func (d *Driver) GetRollupTimeInterval(ctx context.Context, start *big.Int) (time.Duration, error) { | |||
There was a problem hiding this comment.
func (d *Driver) RollupTimeInterval(ctx context.Context) (*big.Int, *big.Int, error) {
log.Debug("RollupTimeInterval start")
pollingInterval := 500 * time.Millisecond
rollupTimout := time.Minute * 15
exit := time.NewTimer(rollupTimout)
ticker := time.NewTicker(pollingInterval)
for {
// normal logic
start, end, err := d.GetBatchBlockRange(d.Ctx)
if err != nil {
return nil, nil, err
}
return start, end, nil
select {
case <-exit.C:
// timeout exit
case <-ticker.C:
}
}
}
You can use this to do it.
There was a problem hiding this comment.
fixed. And add function GetBatchBlockRangeWithTimeout
| @@ -775,7 +782,10 @@ func (d *Driver) RollupMainWorker() { | |||
| for { | |||
| select { | |||
| case <-ticker.C: | |||
| log.Info("MtBatcher eigen da sequencer fetching current block range") | |||
| if d.Cfg.MinTimeoutRollupTxn > d.Cfg.RollUpMinTxn { | |||
| log.Error("MtBatcher MinTimeoutRollupTxn more than RollUpMinTxn error ", "MinTimeoutRollupTxn(%v)>RollUpMinTxn(%v)", d.Cfg.MinTimeoutRollupTxn, d.Cfg.RollUpMinTxn) | |||
There was a problem hiding this comment.
The validation of the log should be placed at the initialization of the service, and if it is unreasonable, then panic. Instead of repeatedly logging the same content in this loop
There was a problem hiding this comment.
Fixed. add function GetBatchBlockRangeWithTimeout.
mt-batcher/mt_batcher.go
Outdated
| } | ||
| if cfg.MinTimeoutRollupTxn > cfg.RollUpMinTxn { | ||
| log.Error("new driver fail", "err", "MinTimeoutRollupTxn(%v)>RollUpMinTxn(%v)", cfg.MinTimeoutRollupTxn, cfg.RollUpMinTxn) | ||
| return nil, err |
There was a problem hiding this comment.
The error information is incorrect, it is recommended to use errors.new() to return an error
Ethanncnm
dismissed stale reviews from guoshijiang, byteflyfunny, and wukongcheng
via
2dc46e9
# Goals of PR Core changes: - modify for Rollup services optimization code. Notes: - associated pr: #1301 Related Issues: - no
Goals of PR
Core changes:
Add a timeout mechanism for mt-batcher. The new mechanism rely on new environment config
RollupTimeout,MinTimeoutRollupTxn.Delete the transaction size check for sequencer driver in
batch-submitter. And change the size check to transaction amount check which rely on new environment configMaxRollupTxnandMinRollupTxn.Delete the constrain of rollup mechanism which requires every rollup action must contains a L2 transaction at least.
Add a timeout transaction constrain for proposer in
batch-submitterwhich rely on new environment configMinTimeoutStateRootElements.Notes:
MaxRollupTxn: Maximum number of transaction from l2geth which is used to submit to rollup.MinRollupTxn: Minimum number of transaction from l2geth which is used to submit to rollup.MinTimeoutStateRootElements: Minimum number of elements required to submit a state root batch.RollupTimeout: Duration we will wait before rollup batch transaction to Mantle Da.MinTimeoutRollupTxn: minimum of timeout Rollup transaction amount for Mantle da.Related Issues:
Link issues here