-
Notifications
You must be signed in to change notification settings - Fork 29
cli: generalise CLIs and introduce CLIs for rollup FP #522
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
a6b8877 to
05e20e9
Compare
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 refactors and generalizes the CLI layer for finality providers and applies the new abstractions to implement a rollup BSN CLI.
- Extract common, keys, incentive, and version subcommands into reusable packages
- Refactor daemon commands into templates and shared utilities
- Add a rollup-specific CLI by wiring the generalized commands
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version/cmd.go | Introduce AddVersionCommands helper and tidy imports |
| types/txresponse.go | Add PrintRespJSON helper for pretty-printed JSON responses |
| finality-provider/cmd/fpd/main.go | Rewrite root command to use generic Add*Commands calls |
| finality-provider/cmd/fpd/daemon/start.go | Separate template and execution for the start subcommand |
| bsn/rollup-finality-provider/cmd/rollup-fpd/main.go | Bootstraps rollup CLI using the shared command packages |
Comments suppressed due to low confidence (2)
finality-provider/cmd/fpd/main.go:40
- [nitpick] New generic CLI groups are registered here but there are no unit tests verifying that the expected subcommands are present. Consider adding tests for
AddCommonCommandsto ensure all shared commands are wired correctly.
commoncmd.AddCommonCommands(cmd)
bsn/rollup-finality-provider/cmd/rollup-fpd/main.go:40
- [nitpick] The rollup CLI now reuses shared command groups but lacks tests to confirm correct registration. Adding tests around
NewRootCmdfor rollup-fpd would improve confidence.
commoncmd.AddCommonCommands(cmd)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Lazar955
left a comment
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.
Overall lgtm, minor comments, let's wait for one more review
bsn/rollup-finality-provider/cmd/rollup-fpd/daemon/create_fp.go
Outdated
Show resolved
Hide resolved
bsn/rollup-finality-provider/cmd/rollup-fpd/daemon/create_fp.go
Outdated
Show resolved
Hide resolved
KonradStaniec
left a comment
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.
nothing blocking from me :) lets just wrap errors when necessary it will help us later with all the problems
bsn/rollup-finality-provider/cmd/rollup-fpd/daemon/create_fp.go
Outdated
Show resolved
Hide resolved
bsn/rollup-finality-provider/cmd/rollup-fpd/daemon/create_fp.go
Outdated
Show resolved
Hide resolved
|
will wait for @0xAleksaOpacic to test out the UPDATE: working properly (except for known issues in fpd) now 🎉 |
Part of #507
This PR generalises the implementation of existing CLIs for the finality provider, and leverage the generalised utilities to implement the CLIs for rollup BSNs. In particular, this PR
finality-provider/cmd/fpd/common/common_cmd.go), shared among any type of FPsfinality-provider/cmd/fpd/common/incentive_cmd.go), shared among any type of FPsfinality-provider/cmd/fpd/common/keys_cmd.go), shared among any type of FPsversion/), shared among any type of FPsfinality-provider/cmd/fpd/daemon), that are specific to each type of FPsfinality-provider/cmd/fpd/daemonsuch that most of the code is reusable for other types of FPsFinalityProviderApp. The refactoring allows having different logics for this part whereas the other parts remain reusable.finality-provider/cmd/fpd/daemonto construct the daemon cmds for rollup FPs.This is a major refactoring to simplify the development of the BSN finality providers.
Some future work in subsequent PRs: