Skip to content
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

Suggestion: Remove Hardcoded URLs / URIs #176

Open
calebpower opened this issue Oct 8, 2021 · 1 comment
Open

Suggestion: Remove Hardcoded URLs / URIs #176

calebpower opened this issue Oct 8, 2021 · 1 comment

Comments

@calebpower
Copy link

Good afternoon.

It appears that blzd relies on the location of the blzcli-managed wallet to obtain nft keys. Precedence holds that the user (sysadmin) should be able to change the path of the blzd and blzcli home directories, allowing for curium to be essentially portable. Unfortunately, it seems that a few items are hardcoded in curium that prevent ease of use for a custom configuration. For example:

Line 80 in app/app.go holds the following.

func getCLIDir() string {
	if devel.IsDevelopment() && !isFirstNode() {
		return os.ExpandEnv("$HOME/$BLZCLI")
	}
	return os.ExpandEnv("$HOME/.blzcli")
}

This seems to require that blzcli wallets be housed under the service user's home directory. Symlinks do theoretically solve this issue, but such patches decrease maintainability.

Another example is found at lines 295 and 308 of keeper/keeper.go, respectively:

	status, err := httpGet(fmt.Sprintf("http://localhost:%d/status", k.rpcPort))

and

	info, err := httpGet(fmt.Sprintf("http://localhost:%d/net_info", k.rpcPort))

wherein, blzd expects that the RPC endpoint always be bound to the localhost. It is not correct to assume that the RPC will be bound to only either the local interface or an external interface-- for example, a sysadmin could choose to bind the RPC to an internal VPN interface to allow for internal management and monitoring. Such use necessitates additional manipulation of a firewall to prevent public access.

I'm no expert on the curium codebase, so my preference would be to leave changes to someone who knows the codebase more (as I'm sure there are other instances, and I wouldn't want to leave any out). However, given that practically everything else regarding blzcli/blzd entrypoints and locations is otherwise configurable, I recommend that this too be configurable.

Thanks!

@scottburch
Copy link
Collaborator

@calebpower Thanks for pointing to this. I agree, this code needs to be updated. There are a few shortcuts that we took to save some time while we were doing cleanup and this is one of them. Thank you for adding this, so we make sure we take care of it. I can not say at this point when it will be resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants