optimism icon indicating copy to clipboard operation
optimism copied to clipboard

Data transport layer syncing with local L1 - reliability of discovering StartingL1BlockNumber - .queryFilter topic

Open CAPtheorem opened this issue 3 years ago • 5 comments

Describe the bug When booting, the DTL sometimes struggles to determine the correct StartingL1BlockNumber, due to problematic query filtering of this.state.contracts.Lib_AddressManager.queryFilter.

To Reproduce Steps to reproduce the behavior:

  1. Configure the DTL like this:
DATA_TRANSPORT_LAYER__SYNC_FROM_L1: 'true'
DATA_TRANSPORT_LAYER__SYNC_FROM_L2: 'false'
  1. Spin up the system: docker-compose up --renew-anon-volumes
  2. DTL quits with Unable to find appropriate L1 starting block number

Expected behavior Regardless of L1 data origin (local L1, Infura,...), the DTL, when configured to sync from L1, should reliably determine the starting L1 block based on the deployment of the AddressManager

Analysis The DTL is looking for topic 0x188466739ff00cc68bfb2367d23ae4b855264264fe1624caa8884399af23454c aka AddressSet(), which may or may not be present/available to this.state.contracts.Lib_AddressManager.queryFilter. A more reliable topic to filter for might be 0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0 aka ownershipTransferred, which should be a good indicator of the first L1 block? In testing, filtering for ownershipTransferred rather than AddressSet has reduced Unable to find appropriate L1 starting block number errors.

Why does this matter Anything that depends on L1 data, including the Sequencer and Verifier, need the DTL to be able to discover the L1 starting block for correct subsequent function.

CAPtheorem avatar Jun 19 '21 01:06 CAPtheorem

Yeah this is a good observation, thank you for the report! I forgot that Lib_AddressManager is now Ownable, which means that OwnershipTransferred gets emitted as soon as it's created. Would you be interested in creating a PR that searches for OwnershipTransferred instead of AddressSet? Since it's backwards compatible it can be a PR directly into develop.

smartcontracts avatar Jun 19 '21 18:06 smartcontracts

A more compute heavy but decoupled from implemention way to do this would be a binary search for the code. Then changes to the address manger wouldn't ever break discovery of the starting L1 blocknumber

On Sat, Jun 19, 2021, 11:35 AM smartcontracts @.***> wrote:

Yeah this is a good observation, thank you for the report! I forgot that Lib_AddressManager is now Ownable, which means that OwnershipTransferred gets emitted as soon as it's created. Would you be interested in creating a PR that searches for OwnershipTransferred instead of AddressSet? Since it's backwards compatible it can be a PR directly into develop.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ethereum-optimism/optimism/issues/1126#issuecomment-864447286, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSR4ATPSGMAV5EI6SHZDKTTTTPQ3ANCNFSM466RTAFA .

tynes avatar Jun 19 '21 18:06 tynes

Yeah this is a good observation, thank you for the report! I forgot that Lib_AddressManager is now Ownable, which means that OwnershipTransferred gets emitted as soon as it's created. Would you be interested in creating a PR that searches for OwnershipTransferred instead of AddressSet? Since it's backwards compatible it can be a PR directly into develop.

Yes, on it, 24 hours, will PR.

CAPtheorem avatar Jun 19 '21 19:06 CAPtheorem

A more compute heavy but decoupled from implemention way to do this would be a binary search for the code. Then changes to the address manger wouldn't ever break discovery of the starting L1 blocknumber

Problem with this approach is that many infra providers don't allow archival searches like this for free tiers. One alternative is to have all contracts extend a standard contract that either emits an event when created or stores the creation block number in the constructor.

smartcontracts avatar Jun 20 '21 03:06 smartcontracts

#2126

yzx0409 avatar May 11 '22 09:05 yzx0409

Hi @CAPtheorem please close the issue, as the changes seem to have been merged but the issue is still open!!!

shubhamshd avatar May 09 '23 12:05 shubhamshd