fix(deploy,wallet,doctor): derive sequencer URL from [localnet] port#145
Closed
wozos wants to merge 1 commit into
Closed
fix(deploy,wallet,doctor): derive sequencer URL from [localnet] port#145wozos wants to merge 1 commit into
wozos wants to merge 1 commit into
Conversation
logos-co#40 covered doctor's port-check label hardcoding 127.0.0.1:3040. Three sites still embedded the same default after that fix: 1. deploy.rs DEFAULT_SEQUENCER_ADDR fallback ignored [localnet] port. 2. wallet_support::is_connectivity_failure substring-matched the literal addresses, which both ignored non-default localnet ports (the configured URL embeds whatever port is in scaffold.toml) and misclassified functional failures that echoed the URL in their error context (RPC rejection, signature mismatch, malformed payload) as transient connectivity errors. 3. doctor's wallet network-config check substring-matched the literal addresses against the raw wallet_config.json text instead of parsing the JSON, brittle to whitespace/formatting and blind to non-default ports. The new behavior: - WalletRuntimeContext.sequencer_addr is now resolved once in load_wallet_runtime (String, not Option<String>). Missing wallet_config.json#sequencer_addr falls back to http://127.0.0.1:<localnet.port>. deploy and wallet topup just use wallet.sequencer_addr directly. - is_connectivity_failure keeps only transport-error tokens; address literals were dropped. - doctor's wallet network-config check parses the JSON with serde_json, reads sequencer_addr, and PASSes when it points at http(s)://(127.0.0.1|localhost):<localnet.port>. Missing field PASSes because the runtime fallback produces the local URL. Anything else WARNs with a remediation that names the expected URL for this project's localnet port. Tests added (15 new): - wallet_support: is_connectivity_failure positive coverage for the retained transport-error tokens; negative coverage for bare-URL and RPC-rejection shapes that no longer trigger; default sequencer URL derivation on 3040 and 14321. - doctor: check_wallet_network_config covering matching/mismatched ports, localhost alias, missing field, remote host, malformed JSON, missing file, non-string sequencer_addr, and the sequencer_addr_targets_local_port URL matcher (positive and negative). Closes logos-co#118
8500476 to
e8ed638
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
logos-scaffoldusers on non-default localnet ports ([localnet] port = 14321etc.) had three sites still hardcoding the defaulthttp://127.0.0.1:3040:deploy's sequencer fallback,is_connectivity_failure's substring needles, anddoctor's wallet network-config check. The first strandeddeployagainst the wrong RPC whenwallet_config.jsonomittedsequencer_addr; the second silently downgraded genuine functional failures (RPC rejection, signature mismatch) to transient connectivity warnings whenever the wallet echoed the URL in its error output; the third blindly substring-matched127.0.0.1:3040against the raw config text, so any non-default port produced a falseWarnand a brittle whitespace match could swing the verdict either way. #40 established the port-config-respect invariant; this PR finishes propagating it.Solution
WalletRuntimeContext.sequencer_addris now resolved once inload_wallet_runtime:wallet_config.json#sequencer_addrwhen present, otherwisehttp://127.0.0.1:<[localnet] port>(default 3040 if[localnet]is omitted, matching the existingLocalnetConfig::default). Field changes fromOption<String>toStringso call sites indeploy.rsandwallet.rsno longer carry their own copies of the fallback URL — both just clonewallet.sequencer_addr.DEFAULT_SEQUENCER_ADDRconstant indeploy.rsis gone; the"http://127.0.0.1:3040"literal inwallet.rsis gone.is_connectivity_failure(src/commands/wallet_support.rs:226) drops the"127.0.0.1:3040"/"localhost:3040"needles. The remaining transport-error tokens (connection refused,tcp connect error,network is unreachable, etc.) cover the actual transport failure modes; the address literals were noise that caused two distinct false-positive shapes (non-default port loses the heuristic; functional failure echoing the URL gets misclassified).doctor's wallet-network-config check is nowcheck_wallet_network_configinsrc/commands/doctor.rs. It parseswallet_config.jsonwithserde_json, extractssequencer_addr, andPasses iff the URL host is127.0.0.1orlocalhostand the port equalsproject.config.localnet.port. Missingsequencer_addris also aPassbecause the runtime falls back to the local URL; missing file / parse failure / non-stringsequencer_addr/ remote-hostWarnwith remediations that name the project's expected URL (not a hardcoded 3040). The URL-shape predicate (sequencer_addr_targets_local_port) is its own function for unit testability.check_wallet_network_configbranch (matching/mismatched port, localhost alias, missing field, missing file, malformed JSON, non-string value, remote host) plus the URL matcher.cargo buildandcargo test --all-targetsboth pass on the worktree (250 lib tests, 135 integration tests).cargo fmtclean. No newcargo clippywarnings on the touched files; pre-existing warnings inbasecamp.rsare unchanged.Notes
fix(wallet,deploy): default sequencer URL from scaffold localnet port), which addresses only point (1). This PR is broader: it also covers (2) and (3), and centralizes resolution insideload_wallet_runtimerather than introducing a per-caller helper. Whichever lands first, the other should rebase against the resolved approach. The trade-off considered: leavingsequencer_addrasOption<String>with adefault_sequencer_http_url_for_projecthelper (PR fix(wallet,deploy): default sequencer URL from scaffold localnet port #84's shape). Rejected because callers were already cloning, the field is always populated in practice, and the issue (Hardcoded port-3040 sites beyond #40 (deploy.rs DEFAULT_SEQUENCER_ADDR + 2 substring-match callsites) #118) explicitly preferred deriving it once inload_wallet_runtime.is_localnet_connectivity_failureindoctor.rs— different function, different concern. Both can land independently; the only contact is conceptual (both stop misusing the URL literal)."127.0.0.1:3040"still appears incheck_port_warn("sequencer port 3040", "127.0.0.1:3040", ...)(src/commands/doctor.rs:174). That callsite is what bug: doctor uses hardcoded port 3040 instead of configured localnet port #40 / PR fix(doctor): use configured localnet port instead of hardcoded 3040 #41 is rewriting — out of scope here.Closes #118
Run ID: 20260512-012013
Authored by: scaffold-wozos-issue-impl recurring task (claude-code worker)