exp/services/recoverysigner: empty allowed accounts cause application crash

[leighmcculloch]

When starting the recoverysigner without allowed accounts, the application crashes.

The allowed accounts feature added in 15de5ff is intended to be an optional feature, but it appears it is required by the code because the code isn't handling its absence. The CLI options parameters don't have the parameter defined as required, so there is no helpful error message if a user doesn't provide it.

When starting the application without the parameter specified, I see the following error as if I have provided a malformed invalid value for the option.

FATA[2021-06-04T21:59:55.747Z] Error: parsing allowed source accounts: strkey is 0 bytes long; minimum valid length is 5  pid=17606

I think this should be fixed by keeping the parameter as optional and making it so that if no value is provided the previous behavior is retained.

[leighmcculloch]

I think the bug exists in the following code. strings.Split always returns at least one value, which will be an empty string if an empty string was given to the function.

go/exp/services/recoverysigner/internal/serve/serve.go

Lines 147 to 154 in 991ed55

	allowedSourceAccounts := []*keypair.FromAddress{}
	for _, addressStr := range strings.Split(opts.AllowedSourceAccounts, ",") {
	accountAddress, err := keypair.ParseAddress(addressStr)
	if err != nil {
	return handlerDeps{}, errors.Wrap(err, "parsing allowed source accounts")
	}
	allowedSourceAccounts = append(allowedSourceAccounts, accountAddress)
	}

[mollykarcher]

Closing all recoverysigner issues