Skip to content

Conversation

@protolambda
Copy link
Contributor

@protolambda protolambda commented Oct 9, 2024

Changes from Axel, rebased on develop (from #12213 ):

  • Wire in APIs for new input flow
  • Better logging for Iterator State Needs
  • Remove Chain Monitor ; Trigger Chain Processor on Local Unsafe Update
  • BlockRef
  • uncomment skipped guard

Work in progress by Proto:

  • op-supervisor: generic entry type
  • op-supervisor: progresss on generic entry db, and derived-from DB

Description

Draft PR, to track changes.

Plan to land the changes:

  • Split out the changes to entrydb, including the naming / usages tweaks in the event logs index.
  • Split out the introduction of fromda
  • A PR that integrates the fromda type of index, to handle the safety index functionality

Tests

Work in progress

Additional context

Metadata

axelKingsley and others added 2 commits October 9, 2024 13:30
Changes from Axel:
- Wire in APIs for new input flow
- Better logging for Iterator State Needs
- Remove Chain Monitor ; Trigger Chain Processor on Local Unsafe Update
- BlockRef
- uncomment skipped guard

Work in progress by Proto:
- op-supervisor: generic entry type
- experimental
- op-supervisor: progresss on generic entry db, and derived-from DB
Comment on lines 97 to 101
if err != nil {
return common.Hash{}, err
}
// TODO(#11693): need to get the actual block hash for this log entry for reorg detection
return common.Hash{}, nil
Copy link
Contributor

@semgrep-app semgrep-app bot Oct 9, 2024

Choose a reason for hiding this comment

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

superfluous nil err check before return

Ignore this finding from err-nil-check.

Comment on lines 97 to 100
if err != nil {
return err
}
return nil
Copy link
Contributor

@semgrep-app semgrep-app bot Oct 9, 2024

Choose a reason for hiding this comment

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

superfluous nil err check before return

Ignore this finding from err-nil-check.

Comment on lines +98 to +102
if err != nil {
return eth.BlockID{}, err
}
// TODO fix this for cross-safe to work
return eth.BlockID{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

superfluous nil err check before return

Ignore this finding from err-nil-check.

finalizedL1 := db.finalizedL1
derived, err := db.LastDerivedFrom(chainID, finalizedL1.ID())
if err != nil {
return eth.BlockID{}, fmt.Errorf("could not find what was last derived from the finalized L1 block")
Copy link
Contributor

Choose a reason for hiding this comment

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

No fmt.Errorf invocations without fmt arguments allowed

Ignore this finding from golang_fmt_errorf_no_params.

// The L2 blockhash ensures we are still checking the same log content.
crossDerivedFrom, err := s.deps.CrossDerivedFrom(chainID, includedIn, execMsg.LogIdx)
if err != nil {
// TODO this can be ErrFuture when we don't know the cross-derivation link of the L2 block yet.
Copy link
Contributor

@semgrep-app semgrep-app bot Oct 9, 2024

Choose a reason for hiding this comment

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

TODO in error handling code

Ignore this finding from err-todo.

// does indeed exist in an L2 block.
includedIn, err := s.deps.Check(chainID, execMsg.BlockNum, execMsg.LogIdx, execMsg.Hash)
if err != nil {
// TODO this can be an ErrFuture where we just don't have the data for a definite answer,
Copy link
Contributor

@semgrep-app semgrep-app bot Oct 9, 2024

Choose a reason for hiding this comment

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

TODO in error handling code

Ignore this finding from err-todo.

@protolambda
Copy link
Contributor Author

Closing in favor of #12427

And will open a separate PR for the derived-from DB, and another separate PR for the cross-safe update process.

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