-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add network readiness #210
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds network readiness functionality to the builder-playground, allowing users to verify when networks are ready to accept transactions by checking that blocks are being produced. The implementation includes a /readyz HTTP endpoint and a CLI command to wait for network readiness.
Key changes:
- Introduces
NetworkReadyCheckerinterface for recipes to implement network-level readiness checks - Adds
/readyzHTTP endpoint that returns network readiness status - Implements
wait-readyCLI command to poll the readiness endpoint until the network is ready
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| playground/watchers.go | Adds helper functions to check if chains are producing blocks and wait for first block |
| playground/manifest.go | Defines NetworkReadyChecker interface for network readiness checks |
| playground/readyz.go | Implements HTTP server for /readyz endpoint |
| playground/recipe_l1.go | Implements NetworkReadyChecker interface for L1 recipe |
| playground/recipe_opstack.go | Implements NetworkReadyChecker interface for OP stack recipe with L1/L2 checks |
| playground/recipe_buildernet.go | Implements NetworkReadyChecker interface by delegating to L1 recipe |
| playground/cmd/wait_ready.go | Adds CLI command to poll readiness endpoint |
| main.go | Integrates readyz server and network readiness wait into main execution flow |
| README.md | Documents the new network readiness features |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem: on Docker on MacOS causes the container to fail
|
How is this different from the watchdog functionality? A component can implement a ready check which can be use to determine whether the whole recipe is ready or not. (here) |
Watchdog functionality doesn't have a readiness check as I see, it's more about continuously monitor health of a particular service. But we need one time readiness indicator (basically that the network started producing blocks) in order to delay next CI steps until the block production is started. type ServiceReady interface {
Ready(instance *instance) error
}is one another thing I was looking to implement this feature, but problem it that it is readiness for a specific service, not recipe and generally it makes more sense to me to make it recipe based, so it will be more flexible. |
|
My bad, it was not the watchdog but the service ReadyCheck. Internally this is used to create dependencies between services. You can extend this concept easily and a recipe is not ready until all the ready checks are ok.
Do you have any use case in mind for this? I cannot think of any in which it is not enough to validate the readiness of each service and not the recipe as a whole. |
|
The only problem I can think of can be ordering. ex: for I think it won't be critical, bc in the end we need all of readiness checks to pass anyway. I think we can make service level check work for now and redo it later to recipe level if some edge case arises |
This can be achieved with the DependsOnHealthy function. |
6399618 to
30e63d5
Compare
|
@ferranbt I implemented ready check as part of |
| "--chain", "/data/genesis.json", | ||
| "--datadir", "/data_reth", | ||
| "--color", "never", | ||
| "--ipcpath", "/data_reth/reth.ipc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not remember why right now, but I think it was important to have this. Sorry for not documenting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, let's leave this then, will revert
LGTM the service ready part. Medium term I think it would be nice to figure out how to do those checks as part of the docker-compose ready checks. |
This reverts commit d0d3401.
|
@metachris @ferranbt anything else to take into account here before we can merge it? |
Add Network Readiness Checking
Problem
After all services start and Docker healthchecks pass, the network may not yet be producing blocks. This makes it inconvenient for users who need to wait before deploying contracts or running tests, as they have to manually check when the network is actually ready for transactions.
Solution
Implemented network-level readiness checking that ensures the blockchain is producing blocks, not just that services are responsive.
Closes #204