Detect backend and fail fast when zcashd lacks required experimental feature#542
Conversation
|
Here's a suggestion for testing. Modifying those zcashd configuration flags on mainnet or even testnet is pretty disruptive, so instead: In a different window, start lightwalletd: (You'll get an error, You can try the various combinations of |
I incorporated the changes you requested, but did not get time to run the tests. I will do it ASAP and share an update here. |
LarryRuane
left a comment
There was a problem hiding this comment.
Thanks for the updates, looks good now, except I still have one question. Yes, please test carefully, even if manual testing, and please report the testing you have done here, thanks! I'm out of town so I won't be able to test until next week on Wednesday probably.
cmd/root.go
Outdated
| case strings.Contains(subver, "/MagicBean:"): | ||
| result, rpcErr := common.RawRequest("getexperimentalfeatures", []json.RawMessage{}) | ||
| if rpcErr != nil { | ||
| common.Log.Infof("zcashd backend detected but getexperimentalfeatures RPC failed: %s", rpcErr.Error()) |
There was a problem hiding this comment.
| common.Log.Infof("zcashd backend detected but getexperimentalfeatures RPC failed: %s", rpcErr.Error()) | |
| common.Log.Fatalf("zcashd backend detected but getexperimentalfeatures RPC failed: %s", rpcErr.Error()) |
Don't you think we should fail here? I'm not sure what is the point of trying to go on?
There was a problem hiding this comment.
Yes, we should fail here. No point proceeding.
Thank you for the pointer Larry. Below are the tests I attempted and pasted are the relevant snippets. Test 1:
|
3e70771 to
cf040a3
Compare
|
@LarryRuane Please review the test runs whenever you get the time and let me know if you have questions. |
LarryRuane
left a comment
There was a problem hiding this comment.
Looks great, one small suggestion, and also please squash down to one commit, thanks.
cmd/root.go
Outdated
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "golang.org/x/exp/slices" |
There was a problem hiding this comment.
My IDE (vscode, aka code) automatically runs an excellent tool called goimports whenever I modify and save a file. It formats the code and organizes imports consistently. I recommend you run this; it will move this import down to its own "section" (you'll see). Can you please make that change?
The entire code base isn't quite clean from this tool's prespective, but close (except for the protobuf generated files, unfortunately), and I'd like to keep it that way.
There was a problem hiding this comment.
As requested -
- ran
goimportsto format the imports - squashed the commits into a single commit
Please let me know if other edits are needed.
9c011b4 to
c679ce1
Compare
github doesn't understand that the requested change has been made
Closes #375
Updated/validated behavior: during startup, lightwalletd detects the backend using
getinfo.subversion.zcashd (/MagicBean:): lightwalletd calls
getexperimentalfeaturesand requires that eitherlightwalletdorinsightexploreris enabled; otherwise startup fails with a clear error.zebrad (/Zebra:): skips the experimental feature check (zebrad does not implement getexperimentalfeatures and supports required RPCs by default).
Test Plan:
go test ./...
Manual – zcashd missing feature:
Expected: startup fails with appropriate error message.
Manual – zcashd with feature enabled:
Expected: startup succeeds.
Manual – zebrad backend:
Expected: startup succeeds