curium icon indicating copy to clipboard operation
curium copied to clipboard

Suggestion: Remove Hardcoded URLs / URIs

Open calebpower opened this issue 3 years ago • 1 comments

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!

calebpower avatar Oct 08 '21 20:10 calebpower

@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.

scottburch avatar Oct 08 '21 20:10 scottburch