-
Notifications
You must be signed in to change notification settings - Fork 13
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
refactor: move network validation to GetFullNodeAPIV1Curio #345
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revert this file to its original form. There is still unused code there. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,9 @@ import ( | |
|
||
"github.com/filecoin-project/lotus/lib/ulimit" | ||
"github.com/filecoin-project/lotus/metrics" | ||
"github.com/filecoin-project/lotus/api/v0api" | ||
"github.com/filecoin-project/lotus/build" | ||
"github.com/filecoin-project/lotus/cli/lcli" | ||
) | ||
|
||
type stackTracer interface { | ||
|
@@ -135,7 +138,13 @@ var runCmd = &cli.Command{ | |
return xerrors.Errorf("starting market RPCs: %w", err) | ||
} | ||
|
||
err = rpc.ListenAndServe(ctx, dependencies, shutdownChan) // Monitor for shutdown. | ||
api, closer, err := lcli.GetFullNodeAPI(cctx) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I though I already clarified that we don't need this in run in the original issue |
||
if err != nil { | ||
return err | ||
} | ||
defer closer() | ||
|
||
err = rpc.ListenAndServe(ctx, dependencies, shutdownChan) | ||
if err != nil { | ||
return err | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |
lapi "github.com/filecoin-project/lotus/api" | ||
"github.com/filecoin-project/lotus/chain/types" | ||
cliutil "github.com/filecoin-project/lotus/cli/util" | ||
"github.com/filecoin-project/lotus/build" | ||
) | ||
|
||
var clog = logging.Logger("curio/chain") | ||
|
@@ -30,20 +31,20 @@ func GetFullNodeAPIV1Curio(ctx *cli.Context, ainfoCfg []string) (api.Chain, json | |
if tn, ok := ctx.App.Metadata["testnode-full"]; ok { | ||
return tn.(api.Chain), func() {}, nil | ||
} | ||
|
||
if len(ainfoCfg) == 0 { | ||
LexLuthr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return nil, nil, xerrors.Errorf("could not get API info: none configured. \nConsider getting base.toml with './curio config get base >/tmp/base.toml' \nthen adding \n[APIs] \n ChainApiInfo = [\" result_from lotus auth api-info --perm=admin \"]\n and updating it with './curio config set /tmp/base.toml'") | ||
} | ||
|
||
var httpHeads []httpHead | ||
version := "v1" | ||
{ | ||
if len(ainfoCfg) == 0 { | ||
return nil, nil, xerrors.Errorf("could not get API info: none configured. \nConsider getting base.toml with './curio config get base >/tmp/base.toml' \nthen adding \n[APIs] \n ChainApiInfo = [\" result_from lotus auth api-info --perm=admin \"]\n and updating it with './curio config set /tmp/base.toml'") | ||
} | ||
for _, i := range ainfoCfg { | ||
ainfo := cliutil.ParseApiInfo(i) | ||
addr, err := ainfo.DialArgs(version) | ||
if err != nil { | ||
return nil, nil, xerrors.Errorf("could not get DialArgs: %w", err) | ||
} | ||
httpHeads = append(httpHeads, httpHead{addr: addr, header: ainfo.AuthHeader()}) | ||
for _, i := range ainfoCfg { | ||
ainfo := cliutil.ParseApiInfo(i) | ||
addr, err := ainfo.DialArgs(version) | ||
if err != nil { | ||
return nil, nil, xerrors.Errorf("could not get DialArgs: %w", err) | ||
} | ||
httpHeads = append(httpHeads, httpHead{addr: addr, header: ainfo.AuthHeader()}) | ||
} | ||
|
||
if cliutil.IsVeryVerbose { | ||
|
@@ -53,18 +54,36 @@ func GetFullNodeAPIV1Curio(ctx *cli.Context, ainfoCfg []string) (api.Chain, json | |
var fullNodes []api.Chain | ||
var closers []jsonrpc.ClientCloser | ||
|
||
// Check network compatibility for each node | ||
for _, head := range httpHeads { | ||
v1api, closer, err := newChainNodeRPCV1(ctx.Context, head.addr, head.header) | ||
if err != nil { | ||
clog.Warnf("Not able to establish connection to node with addr: %s, Reason: %s", head.addr, err.Error()) | ||
continue | ||
} | ||
|
||
// Validate network match | ||
networkName, err := v1api.StateNetworkName(ctx.Context) | ||
if err != nil { | ||
clog.Warnf("Failed to get network name from node %s: %s", head.addr, err.Error()) | ||
closer() | ||
continue | ||
} | ||
|
||
// Compare with binary's network using BuildTypeString() | ||
if string(networkName) != build.BuildTypeString() { | ||
clog.Warnf("Network mismatch for node %s: binary built for %s but node is on %s", | ||
head.addr, build.BuildTypeString(), networkName) | ||
closer() | ||
continue | ||
} | ||
|
||
fullNodes = append(fullNodes, v1api) | ||
closers = append(closers, closer) | ||
} | ||
|
||
if len(fullNodes) == 0 { | ||
return nil, nil, xerrors.Errorf("failed to establish connection with all nodes") | ||
return nil, nil, xerrors.Errorf("no compatible nodes found - all nodes had network mismatches or connection errors") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to change this message |
||
} | ||
|
||
finalCloser := func() { | ||
|
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.
We don't need this function or above constants