Skip to content

fabricx: UpdatePublicParams/InstallPublicParams panic on nil issuer client or CallView error (one variant crashes the process from a background goroutine) #2046

Description

@adecaro

Summary

Three related issues in the fabricx factory's public-params setup path make UpdatePublicParams/InstallPublicParams crash instead of returning an error on ordinary, recoverable failures — including one variant that crashes the whole process asynchronously from a background goroutine.

Where

integration/nwo/token/fabricx/factory.go:91,101,116:

func (b *Backend) UpdatePublicParams(tms *tokentopology.TMS, ppRaw []byte) {
    _, err := b.ClientProvider.Client("issuer").CallView("SetupPublicParams", ...)  // no nil check
    if err != nil {
        panic("failed updating pps: " + err.Error())                              // factory.go:116
    }
}
  • UpdatePublicParams calls b.ClientProvider.Client("issuer").CallView(...) with no nil check on the client — unlike InstallPublicParams, which retries up to 60 times waiting for the client to become non-nil before proceeding.
  • Both functions panic("failed updating pps: " + err.Error()) on any CallView error, rather than returning it.
  • InstallPublicParams's retry loop runs in a bare go func(){...}() with no recover(). A CallView failure there panics on a background goroutine nobody is watching — the caller gets no error at all, and the entire process (test runner, orchestrator, whatever process called it) crashes asynchronously, potentially long after InstallPublicParams itself returned.

Impact

Calling UpdatePublicParams before the issuer FSC node has finished starting (a normal race during network bring-up) panics on a nil-interface method call. A transient RPC failure during SetupPublicParams — network blip, issuer temporarily overloaded — turns an ordinary, recoverable error into an unrecoverable process crash, and in the InstallPublicParams case, one that surfaces asynchronously on a goroutine with no error channel back to the caller.

Reproduction

Reproduced locally with unit tests (not yet committed). The synchronous panics are confirmed directly with require.Panics. The background-goroutine crash required a subprocess harness, since an unrecovered goroutine panic terminates the whole process, including a test binary:

cmd := exec.CommandContext(ctx, os.Args[0],
    "-test.run=^TestInstallPublicParams_CallViewErrorCrashesProcessInBackground$", "-test.v")
cmd.Env = append(os.Environ(), installPublicParamsCrashEnv+"=1")
out, runErr := cmd.CombinedOutput()

require.Error(t, runErr, "background goroutine panic should crash the subprocess")
require.Contains(t, string(out), "panic: failed updating pps")

Happy to include these regression tests in the fix PR.

Suggested fix

  • Add a nil check on the issuer client in UpdatePublicParams, mirroring InstallPublicParams's retry-until-ready pattern.
  • Return the CallView error instead of panicking in both functions.
  • Add recover() inside the background goroutine in InstallPublicParams, surfacing the failure through an error channel or similar instead of letting it crash the process.

Severity

HIGH

Metadata

Metadata

Assignees

Type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions