ethereum / eth-portal

A collection of utilities related to Ethereum's Portal Network
MIT License
6 stars 6 forks source link

Support --latest-backfill cli arg #47

Closed njgheorghita closed 1 year ago

njgheorghita commented 1 year ago

What was wrong?

During sync we've been discussing initial goals for data availability on the portal network. A first "target" we settled upon was measuring the availability of the most recent X number of blocks. Starting with something like 1000 for the value of X and then gradually increasing this number. In terms of "dockerizing" the bridge, it would be nice to have a cli option that will backfill the "latest" X number of blocks and then exit. In which case the deployment script will restart the bridge and re-run the backfill, with the latest head.

Issue #

How was it fixed?

I'm not sure if this is the most aesthetically pleasing option. I'm definitely open to alternatives. I played around with supporting some other implementations...

In the end, it was simpler to just introduce a new flag rather than try and get clever with the existing arguments.

To-Do

Cute Animal Picture

image

carver commented 1 year ago

So my first question when reading this was: when would this CLI arg be used outside of this narrow short-term goal? I hesitate to add CLI cruft for something that will be around for less than a year.

I'd be curious to hear your answers. If there is nothing, then I might want to mark it as deprecated right away or something, so we know not to rely on it. Otherwise, we might use the other use case as a guide for how to design it.

One use case I can think of is if we have a bridge outage and we need to backfill from now to when the outage started. In that case, the most salient number to refer to might be the number of hours, like --backfill-hours 200. On the other hand, the other argument that lets you specify specifics makes you specify blocks. So I'm good with following that standard.

Maybe just to avoid the name clash with latest, something like --patch-recent?

njgheorghita commented 1 year ago

when would this CLI arg be used outside of this narrow short-term goal? ... One use case I can think of is if we have a bridge outage and we need to backfill from now to when the outage started.

@carver Seems like a reasonable use case to me. I can't think of any others off the top of my head. But, I wouldn't be quick to say that the stated goal here is "narrow & short-term". I could see us running a bridge node in "patch-recent" mode indefinitely, to complement a separate node running in "latest" mode.

I didn't mark the flag deprecated here, but I can make that change if you'd still like to see it.

(BTW, please remember to request review next time, so it shows up in my notifications)

Ahh, my bad, I'll make sure to do that going forward. Could you add me as a collaborator to the repo? I'm currently not able to request reviews (besides pinging you manually), and don't have merge privileges. But this is ready for merge if you want to 🚢

carver commented 1 year ago

Ok, I still have an intuition that this will be cruft or have a cleaner design going forward, but I don't have anything clear enough to suggest and don't want to block it any longer.

Also, I'll work on moving it into /ethereum and getting the right folks on as maintainers.