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

Local Head Tracking ; Bug Fixes #11991

Closed
wants to merge 10 commits into from
Closed

Conversation

axelKingsley
Copy link
Contributor

@axelKingsley axelKingsley commented Sep 18, 2024

Tracking Ticket: #11983

This PR builds on the Emitter Contract PR because it enables testing: #11956

Local Heads

This PR features the changes required to track the Local Heads of a chain. There were existing locations to integrate into, but nothing written already to update the Head Storage when new blocks were processed.

HeadPointerProcessor

Head Pointer Processors are a new kind of HeadProcessor. They do the following:

  • Use the provided block number to call into the Storage, getting the last index for the given block
  • Create and apply an update to the Heads Storage using an assigned Safety level

Head Pointer Processors are now created with a Chain Monitor, and added to the list of processors invoked during new subscribed ethereum events.

Other Changes

To support this feature, I had to make some bug fixes along the way. this could change considerably with changes @protolambda is making to the Iterator:
https://github.com/ethereum-optimism/optimism/pull/11989/files

Storage instead of LogStorage for ChainMonitor

Chain monitors previously only passed a LogStorage, which meant that they could not access or update the Heads Storage. I expanded the interface they use, and implemented any functions required.

Exposing LastEntryIdx

When updating cross-heads, it's possible to reach an EOF, meaning you've read to the end of the database and never found another Executing Message. When this happens, the x-head would optimistically be the end of the database. So, I exposed LastEntryIndex to appropriate Interfaces from ChainsDB down to LogsDB.

Fixing some DB Access Logic

Two changes are made to support correct finding of indexes:

  • MAYBE TEMPORARY: When searching for LastLogInBlock, if it scans and finds future blocks, but never found the target block, it returns that first index which passed the target. This is to support cases where there are no entries for a block, and the presence of a future-block implies its existence.
  • TEMPORARY: ClosestBlockIterator is currently suppressing EOF errors from SearchCheckpoint. This is so that a zero-value iterator ends up betting used by LastLogInBlock instead of a nil one when EOF happens.

Current Results

With these changes, all 6 head types (Local, Cross).(Unsafe, Safe, Finalized) are staying updated in what appears to be a sane way.

Hacks

  • The Search Checkpoint distance is set to 4 at the moment so I can see more realistic behavior
  • A special fmt.Println is in place so I can see the Heads Storage during runtime

Test

  • New unit tests for Head Pointer Processor

Other Known Issues

Despite these changes, it seems the lookup still encounters issues:

  • OP Nodes are complaining failed to check Block 0xffdde99def42f5a0a83822f469f064e814788d197aa78d5c3647769b0a59db84:1 (chain 900201): unrecognized safety level: "cross-finalized"
    OP Supervisor is complaining failed to check Block 0x8a4212b7cc14eafcc1b6d7404f112ef3b71553dc27783a850bc8bb0e0d9b77f3:37 (chain 900200): failed to scan block: block 37 not found in chain 900200

I think both of these issues get fixed up considerably when every block has a guaranteed entry in the database.

@axelKingsley axelKingsley changed the title Bug Fixes Draft: Supervisor DB Bug Fixes Sep 18, 2024
@axelKingsley axelKingsley changed the title Draft: Supervisor DB Bug Fixes Local Head Tracking ; Bug Fixes Sep 19, 2024
Base automatically changed from supervisor/SuperSystemContracts to develop September 20, 2024 03:41
// create a mock storage which stubs out lastLogInBlock
store := &MockStorage{
lastLogInBlock: 1234,
lastLogInBlockErr: fmt.Errorf("error"),
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.

@axelKingsley
Copy link
Contributor Author

this code was superseded by: #12154

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