refactor: drop WALLET_PROVIDER_USER_URL and derive the URL default from the port - #7
Merged
Merged
Conversation
…om the port WALLET_PROVIDER_USER_URL had no reader and named a human-facing wallet page this service does not serve, so it and the userUrl field on the provider descriptor and connect result are gone. WALLET_PROVIDER_URL keeps overriding verbatim; only its fallback changes, from a fixed http://localhost:3010 to the port the process actually binds, so moving WALLET_SERVICE_PORT no longer leaves the descriptor advertising 3010. Refs #2
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.
Summary
Closes #2
WALLET_PROVIDER_USER_URLhad no reader and named a human-facing wallet page this service does not serve.WALLET_PROVIDER_URLfell back to a fixedhttp://localhost:3010, so moving the bind port left the advertised URL pointing at a port nothing listens on.Changes
WALLET_PROVIDER_USER_URLis gone, withuserUrlon the provider descriptor and theconnectresultWALLET_PROVIDER_URLfallback is nowhttp://localhost:plus the port actually boundWALLET_PROVIDER_URLis still used verbatim, port or no portDeviations
.env.examplecarries no explanatory comment onWALLET_PROVIDER_URL, dropped by decisionWALLET_PROVIDER_*row inarchitecture.mdstays accurateAcceptance criteria
WALLET_PROVIDER_URLdefaults tohttp://localhost:${WALLET_SERVICE_PORT}WALLET_PROVIDER_URLoverrides the default, with or without a portWALLET_PROVIDER_USER_URLis removed fromsrc/config.tsuserUrlis removed frombuildProviderand from theconnectresult.env.exampledropsWALLET_PROVIDER_USER_URLand commentsWALLET_PROVIDER_URLas a deployment overrideTest plan
Automated tests
pnpm installpnpm test, expect 91 passing and 0 failingpnpm run typecheck,pnpm knipandpnpm run build, expect no output from anyManual verification
cp .env.example .env, fillCANTON_BACKEND_TOKENWALLET_PROVIDER_URLline from.envand setWALLET_SERVICE_PORT=4000pnpm run devcurl -s localhost:4000/rpc -H 'content-type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"status"}'provider.urlto readhttp://localhost:4000, and nouserUrlin the responseBreaking changes
userUrlno longer appears in thestatusprovider descriptor or theconnectresult. It is optional in the vendored OpenRPC spec, so no consumer contract required it. No migration.Checklist
Screenshots
None.