Fix login funding: add debug logging and quote paths#2262
Merged
0pcom merged 31 commits intoskycoin:developfrom Mar 28, 2026
Merged
Fix login funding: add debug logging and quote paths#22620pcom merged 31 commits intoskycoin:developfrom
0pcom merged 31 commits intoskycoin:developfrom
Conversation
The createRawTransaction command was failing silently. Now logs the full command being run and captures stderr (2>&1) for error diagnostics. Also quotes all paths and includes output in error messages.
- Add CheckXpubDepth to reject chain-level xpub (depth 4) with clear error pointing users to walletKeyExport --path=0 for account xpub - Add depth check + error page in login UI explaining the BIP44 hierarchy - Add warning in `skywire cli reward` when setting a depth 4 xpub - Add -W flag to LoginChainCmd so it uses the correct working directory - Document xpub key setup in reward command help with BIP44 path diagram - Document working directory requirement and login flow in loginchain help Root cause: Skycoin web wallet shows external chain xpub (depth 4, path 0/0) but login system needs account-level xpub (depth 3, path 0) to derive change chain addresses for verification.
Three leak sources fixed: 1. sudph dial(): close dialConn (pfilter) on KCP connection failure or hole punch write failure — each leaked KCP session spawns 3-4 goroutines (updater, ReadFrom, keepalive, sendLoop) 2. sudph dialWithTimeout(): add 1s pause between retries to prevent tight loop creating many leaked KCP sessions on repeated failure 3. All transport Dial() methods (stcp, stcpr, sudph): close the raw connection if initTransport fails (handshake error, port exhaustion) — previously the connection was abandoned with its goroutines
When handshake fails after kcp.NewConn succeeds, the KCP connection was abandoned with its goroutines (updater, ReadFrom, sendLoop). Also close arConn on subsequent failures (LocalAddresses, Marshal, Write).
The /api/v1/transactions endpoint returns inputs as UxID strings, not objects with owner fields. With verbose=1, inputs include the owner address. Also add fallback for non-verbose responses — if the tx was queried by the user's address and has an output to the genesis address, it's the return transaction.
Display the full node-info.json survey for each visor associated with the logged-in user's reward address.
Visor PKs on the account page are now links to /account/survey/<pk>. The endpoint checks the session cookie and verifies the requested PK is in the user's visor list before serving the survey JSON. Users can only access surveys for visors where their reward address is set.
…ogin After logout, the previous verification transaction was still on the chain so re-login succeeded without a new transaction. Now each txid is marked as consumed after successful verification and skipped on subsequent login attempts.
The reward calculator used coincipher.DecodeBase58Address which only validates skycoin addresses, causing visors with xpub reward addresses to be marked ineligible. Now uses rewardconfig.ValidateRewardAddress which accepts both skycoin addresses and xpub keys.
Scans hist/*_rewardtxn0.csv files for reward payments matching the user's address. For xpub keys, derives the first 20 external chain addresses (m/account'/0/0..19) to match against, since the reward transaction CSV contains derived addresses, not the xpub itself. Also adds DeriveExternalAddressFromXpub utility for external chain address derivation (BIP44 m/account'/0/index).
… calendar 1. Transport logs: show full transport ID instead of truncated 8 chars 2. Transport graph: hide Data Flow Animation and Show Routes checkboxes (only meaningful when hosted by the visor, not the reward server) 3. Calendar: use cal --color without lolcat to preserve current day highlight (lolcat overwrites the reverse video ANSI codes)
…ent, tree listing 1. Stats page: add current date to TPD Network Summary heading 2. Bandwidth history: fetch from TPD /metrics instead of hist/ files 3. Visor bandwidth: fetch from TPD /metrics, show top 20 visors stacked chart 4. Version history chart: fix x-axis labels using absolute positioning to align with bar positions (was using flex which drifted) 5. Log collection tree: replace 'skywire cli log st' with direct directory listing showing visor PKs and survey status (no CSV filenames)
…g styles The /skycoin-rewards page had no HTML document structure (no html/head/body tags), causing navigation to render differently from /stats. Now uses the same dark theme styling as other pages.
The consumedTxIDs map reset on server restart, allowing old transactions to pass verification. Now only accepts transactions in blocks created after the challenge was issued (pending.CreatedAt), which survives restarts since the challenge is re-created each time.
Prevent attack where someone enters another user's xpub, creates a challenge, and waits for the real user to verify on a different machine. Now only one active challenge per login address is allowed — creating a new challenge invalidates any existing ones for that address. Combined with the block timestamp check (only accepts transactions after challenge creation), this ensures: 1. Old transactions can't satisfy new challenges (timestamp check) 2. Attacker can't piggyback on victim's transaction (single challenge)
Each login challenge generates a fresh random skycoin address as the verification target. The user sends coins to this unique address instead of the static genesis address. Security improvements: - No replay: unique address has never received coins before - No piggybacking: different challenges have different target addresses - No timestamp checks needed: balance > 0 means challenge satisfied - No challenge invalidation needed: challenges are fully independent Verification simplified to a balance check on the unique address.
Each challenge address is derived deterministically from the genesis wallet seed at an incrementing index. Index 0 is the genesis address (used for the blockchain), so challenges start at index 1. All verify addresses belong to the same wallet, making them auditable and recoverable. Falls back to random keypair if the seed can't be loaded.
Challenges expire after 10 minutes, sessions after 24 hours. Without cleanup, expired entries accumulated in memory indefinitely. Now a background goroutine sweeps every 5 minutes.
1. Remove duplicate HTML wrapper from htmlRewardPageTemplate — the base template (htmlMainPageTemplate) already provides the document structure and navigation. The reward template just provides content. 2. Restore lolcat for calendar colors. 3. Improve verify page wording with step-by-step instructions.
1. Show full YYYY-MM-DD on first label and when the year changes 2. Show MM-DD for same-year labels 3. Skip the last label if it would overlap the previous one (within half the label interval)
All items are charted completely. The maxSlices parameter is ignored.
Replace the old navlinks (raw inline <a> tags) with a proper <nav> element matching the base template structure. Includes inline CSS for flex layout, dropdown menus, and responsive behavior so pages not using the base template still render the nav correctly. Adds the 'community' dropdown (telegram links) that was only in the base template. All pages now have identical navigation.
Previously, generateAndCacheCountryStats spawned 'skywire svc ip' for each unique IP address (40+ concurrent processes). Each subprocess loaded the embedded ~30MB GeoIP database, did one lookup, and exited. Now opens the embedded database once via geoip2.OpenBytes and performs all lookups in-process. Eliminates the CPU spike from dozens of concurrent process spawns. Exports EmbeddedGeoIP() and LookupIP() from the geoip commands package so the reward server can use the same database directly.
…nd borders - Removed duplicate <table> opening tag - Added cell padding (4px 12px) for spacing - Right-aligned numeric columns with monospace font - Added row borders for visual separation - Cleaner header labels
Lint fixes: - Use fmt.Fprintf instead of Write([]byte(fmt.Sprintf(...))) (staticcheck QF1012) - Remove unused checkTransactionToAddress (replaced by balance check) - Remove unused hours return from checkBalanceOnLoginChain (unparam) - Remove unused loginPeerTOMLTemplate const Table fix: - Simplified Pool 1/Pool 2 headers to align with data columns
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
1 participant
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
createRawTransactioncommand being run for debugging2>&1) so error messages aren't swallowed byscript.ExecContext
createRawTransactionwas failing withexit status 1but no error details were visible becausescript.Execonly captures stdout.