new env var SOLVER_EXPLORER_URL#9
Conversation
There was a problem hiding this comment.
Review: #9 — SOLVER_EXPLORER_URL env var
Verdict: APPROVE ✅
Clean, minimal change. Adds an optional SOLVER_EXPLORER_URL env var that gets passed through to arksdk.WithExplorerURL() during wallet init. Reviewed all 5 changed files against the go-sdk InitOption contract.
What's good
- Correctly optional:
ExplorerURLdefaults to empty string, andservice.go:209-211only appends theWithExplorerURLoption when non-empty. This avoids triggering the go-sdk's"explorer url cannot be empty"validation error (go-sdk/init_opts.go:23). - Proper go-sdk integration:
WithExplorerURLandInitOptionare confirmed exported ingo-sdkat the pinned version (v0.9.2-0.20260518112312-588477f9d618). The variadicInit(...opts)signature matches. - Tests cover both paths:
config_test.goassertsExplorerURLis empty by default and correctly overridden via env var. Good. - Config layer is clean: follows the existing viper pattern exactly — add var, add struct field, read via
viper.GetString(). - Not protocol-critical: this only affects which block explorer the go-sdk wallet talks to for chain queries. No VTXO, signing, forfeit, or round lifecycle code is touched.
One bug in docker-compose
test/docker-compose.yml:70 — SOLVER_EXPLORER_URL=http://chopsticks:3000 is set on the arkd container, not the solver. The arkd image doesn't read SOLVER_* env vars — this env var is dead weight here. It should be set on whatever runs the solver process during tests (or in a solver service block if one gets added to this compose file). Not a blocker since tests likely set this env var separately, but worth fixing so the compose file isn't misleading.
No cross-repo breakage
- No public API, proto, or type changes — this is purely internal config plumbing.
- go-sdk already exports
WithExplorerURL; no SDK changes needed. - README table updated correctly.
LGTM. Ship it, fix the docker-compose nit when convenient.
|
included into 3b934ac |
allows passing explorer url as env var for arksdk Init
closes #8