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

batcher: batchSubmitter.checkExpectedProgress #12430

Draft
wants to merge 2 commits into
base: gk/batcher-node-progress
Choose a base branch
from

Conversation

geoknee
Copy link
Contributor

@geoknee geoknee commented Oct 11, 2024

towards #12124

@geoknee geoknee changed the title implement batchSubmitter.checkExpectedProgress batcher: batchSubmitter.checkExpectedProgress Oct 11, 2024
@geoknee geoknee added this to the Holocene: Derivation milestone Oct 11, 2024
Copy link
Contributor

semgrep-app bot commented Oct 11, 2024

Semgrep found 1 err-todo finding:

  • op-program/client/l2/engineapi/l2_engine_api.go

TODO in error handling code

Ignore this finding from err-todo.

op-batcher/batcher/driver.go Outdated Show resolved Hide resolved
// TODO extract this into a config variable
// TODO should we also wait numConfirmations (this is a txmgr config variable, not in scope of the batch submitter)

BUFFER := uint64(1)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought - given how this is used, I wonder whether naming itPROCESSING_WINDOW would make it more intuitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for choosing a better name. What do you think about just using VERIFIER_L1_CONFS, which is the flag for the verifier?

VerifierL1Confs = &cli.Uint64Flag{
Name: "verifier.l1-confs",
Usage: "Number of L1 blocks to keep distance from the L1 head before deriving L2 data from. Reorgs are supported, but may be slow to perform.",
EnvVars: prefixEnvVars("VERIFIER_L1_CONFS"),
Value: 0,
Category: L1RPCCategory,
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, that sounds great!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

semgrep-app bot commented Oct 18, 2024

Semgrep found 3 sol-style-require-reason findings:

require() must include a reason string

Ignore this finding from sol-style-require-reason.

Semgrep found 6 sol-style-input-arg-fmt findings:

Inputs to functions must be prepended with an underscore (_)

Ignore this finding from sol-style-input-arg-fmt.

Semgrep found 4 golang_fmt_errorf_no_params findings:

No fmt.Errorf invocations without fmt arguments allowed

Ignore this finding from golang_fmt_errorf_no_params.

Semgrep found 1 math-random-used finding:

  • op-supervisor/supervisor/backend/source/head_monitor_test.go

Do not use math/rand. Use crypto/rand instead.

Ignore this finding from math-random-used.

Semgrep found 1 marshal-json-pointer-receiver finding:

  • op-supervisor/supervisor/backend/db/heads/types.go

MarshalJSON with a pointer receiver has surprising results: golang/go#22967

Ignore this finding from marshal-json-pointer-receiver.

Semgrep found 1 err-nil-check finding:

superfluous nil err check before return

Ignore this finding from err-nil-check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants