Skip to content

Comments

Remove redundant declaration of address names#1695

Merged
smartcontracts merged 2 commits intoregenesis/0.5.0from
maurelian/eng-1645-remove-redundant-declaration-of-address
Nov 9, 2021
Merged

Remove redundant declaration of address names#1695
smartcontracts merged 2 commits intoregenesis/0.5.0from
maurelian/eng-1645-remove-redundant-declaration-of-address

Conversation

@maurelian
Copy link
Contributor

Description
The strings used to resolve system contracts in the Address Manager must be correct, but they are declared independently in many places.

This PR defines an exported object to act as a dictionary of correct names.

@changeset-bot
Copy link

changeset-bot bot commented Nov 4, 2021

🦋 Changeset detected

Latest commit: 6f0fcf2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@eth-optimism/contracts Patch
@eth-optimism/batch-submitter Patch
@eth-optimism/data-transport-layer Patch
@eth-optimism/message-relayer Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2021

Codecov Report

Merging #1695 (f1af0af) into regenesis/0.5.0 (8282054) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           regenesis/0.5.0    #1695   +/-   ##
================================================
  Coverage            72.64%   72.64%           
================================================
  Files                   69       69           
  Lines                 2274     2274           
  Branches               337      337           
================================================
  Hits                  1652     1652           
  Misses                 622      622           
Flag Coverage Δ
batch-submitter 61.56% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.72% <ø> (ø)
data-transport-layer 38.23% <ø> (ø)
message-relayer 83.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8282054...f1af0af. Read the comment docs.

Copy link
Contributor Author

@maurelian maurelian left a comment

Choose a reason for hiding this comment

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

I've found some places I've missed applying the object member access, so this is not quite ready, but appreciate the quick reviews coming in. :)

@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch from d03e032 to cfc8b9b Compare November 5, 2021 11:10
@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch from 88312e2 to 4835794 Compare November 5, 2021 12:07
@maurelian maurelian marked this pull request as draft November 5, 2021 13:59
@maurelian maurelian marked this pull request as ready for review November 5, 2021 14:13
@github-actions github-actions bot added the A-cannon Area: cannon label Nov 5, 2021
@smartcontracts
Copy link
Contributor

Looking good, I'd rebase into one commit and should be good to go after approval

@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch from 2a0349b to 96bf7b4 Compare November 5, 2021 23:13
@github-actions github-actions bot removed the A-cannon Area: cannon label Nov 5, 2021
@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch 2 times, most recently from be42c24 to 64b53f4 Compare November 5, 2021 23:22
@github-actions github-actions bot added the A-ops Area: ops label Nov 7, 2021
Copy link
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

Also note that there are other places in this repository where these names are being used. However, that could make a good first issue or two so I'd recommend not updating those values in this PR.

@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch 3 times, most recently from 0bc574d to 07c5cb5 Compare November 8, 2021 20:36
@github-actions github-actions bot removed the A-ops Area: ops label Nov 8, 2021
@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch from ab6cace to f1af0af Compare November 9, 2021 02:01
Copy link
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

LGTM, nice cleanup.

This commit adds an addressNames object, which should be used both for
names in the address manager, and naming deployment artifacts.

fix(contracts): Remove unused queue container test code

feat(contracts): Split up managed and unmanaged names

refactor(contracts): Move address names into a single nested object

refactor(contracts): Make address keys similar to values

chore: add changeset

docs(contracts): Add a docstring to deployAndPostDeploy

refactor(contracts): Remove unnecessary contract name

If the Address Manager's name is the same as the contract artifact
we don't need specify the contract name argument.
@maurelian maurelian force-pushed the maurelian/eng-1645-remove-redundant-declaration-of-address branch from f1af0af to 6f0fcf2 Compare November 9, 2021 21:44
@smartcontracts smartcontracts merged commit 1cd32e1 into regenesis/0.5.0 Nov 9, 2021
@smartcontracts smartcontracts deleted the maurelian/eng-1645-remove-redundant-declaration-of-address branch November 9, 2021 22:45
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Adds a path for finalizing L2 blocks, based off of the L1 block they
were derived from. To accomplish this:
1. The derivation pipeline now attaches the L1 block that the L2 block
was derived from, within `OpAttributesWithParent`.
2. The DA watcher actor now watches a stream of finalized L1 blocks,
polled every 1m.
3. The engine actor now holds onto a map of `l1_block_number =>
highest_l2_block_in_epoch`, saturated when it receives attributes from
the derivation actor.
- When a new finalized L1 block is observed by the engine actor, it will
find the highest L2 block whose batch data is contained within the
finalized L1 chain, and finalize it (if it knows of any.)

This approach is different than suggested by the tickets, but results in
a more slim outcome that takes advantage of existing actors.

### Periphery changes

- Some refactoring of the `OpAttributesWithParent` type.
- The sync start algorithm now stops safe block traversal at the
finalized head, to ensure it never assigns a safe block past it.
- The `L1WatcherRpc` now uses a custom `BlockStream`, since alloy's
`watch_blocks` stream doesn't dedup nor allow for observing anything
other than head block updates.

### Result

Finalized blocks are correctly streaming in 😄 

<img width="1228" alt="Screenshot 2025-05-24 at 5 09 01 PM"
src="https://github.com/user-attachments/assets/609184a5-35b8-4b04-a124-eeece8ef53e4"
/>

### Meta

closes #1693 
closes #1694 
closes #1695 
closes #1696 
closes #1698
theochap pushed a commit that referenced this pull request Jan 14, 2026
## Overview

Adds a path for finalizing L2 blocks, based off of the L1 block they
were derived from. To accomplish this:
1. The derivation pipeline now attaches the L1 block that the L2 block
was derived from, within `OpAttributesWithParent`.
2. The DA watcher actor now watches a stream of finalized L1 blocks,
polled every 1m.
3. The engine actor now holds onto a map of `l1_block_number =>
highest_l2_block_in_epoch`, saturated when it receives attributes from
the derivation actor.
- When a new finalized L1 block is observed by the engine actor, it will
find the highest L2 block whose batch data is contained within the
finalized L1 chain, and finalize it (if it knows of any.)

This approach is different than suggested by the tickets, but results in
a more slim outcome that takes advantage of existing actors.

### Periphery changes

- Some refactoring of the `OpAttributesWithParent` type.
- The sync start algorithm now stops safe block traversal at the
finalized head, to ensure it never assigns a safe block past it.
- The `L1WatcherRpc` now uses a custom `BlockStream`, since alloy's
`watch_blocks` stream doesn't dedup nor allow for observing anything
other than head block updates.

### Result

Finalized blocks are correctly streaming in 😄 

<img width="1228" alt="Screenshot 2025-05-24 at 5 09 01 PM"
src="https://github.com/user-attachments/assets/609184a5-35b8-4b04-a124-eeece8ef53e4"
/>

### Meta

closes #1693 
closes #1694 
closes #1695 
closes #1696 
closes #1698
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.

4 participants