Repository navigation
fix(config,wallet): validate the contract ID checksum; stop dropping wallet disconnects - #1571
Merged
SheyeJDev merged 1 commit intoOct 19, 2026
Conversation
…wallet disconnects Split-Naira#1502 — CONTRACT_ID was checked with /^C[A-Z2-7]{55}$/ and nothing else. That is a format check, not a validity check: a StrKey carries a trailing CRC-16 checksum over its own payload, so a value can satisfy the regex and still be rejected by the network. The failure surfaced late and opaquely — startup succeeded, then the first real call returned 'contract not found'. Adds StrKey.isValidContract alongside the existing regex, keeping the regex so the two failure modes get distinct messages: wrong shape versus well-formed but checksum-mismatched. The second message says what to do (copy it, don't transcribe it) rather than restating the constraint, because a single wrong character is the actual cause and 'must match /^C...$/' does not help anyone diagnose that. Two existing fixtures were well-formed but checksum-invalid, and both call validateEnv(), so they would have failed on this change. Replaced with StrKey.encodeContract(Buffer.alloc(32, 7)) rather than hand-written literals, so the fixture cannot drift from what a real contract ID looks like. Fixtures in database.test.ts and database-pool-exhaustion.test.ts carry the same invalid value but never call validateEnv(), so they are left alone — changing them would be unrelated churn. Split-Naira#1507 — account changes were handled; disconnections were silently dropped. The poll's guard required fresh.connected, so when the user disconnected the condition was false and nothing dispatched. State kept connected: true and the previous address indefinitely. Three consequences: stale balances stayed on screen, actions scoped to the old account stayed reachable, and the reconnect toast in wallet-provider.tsx could never fire because it keys off exactly that flag. lib/wallet.ts registers no wallet event listeners, so this poll is the only thing watching for the change — there was no second path to catch it. Flips the guard to !fresh.connected ||, so a disconnect takes the same RESET path as a switch. Account-switch handling is unchanged. Not run: dependencies not installed and no tests executed, per the task constraint. Verified by reading lib/wallet.ts (no listeners registered) and wallet-provider.tsx (toast keys off wallet.connected).
|
@ummarig Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
LGTM |
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.
Assigned issues
Per the assignment, this PR implements 2 of the 4 assigned issues. The two not
implemented are referenced with
Closesso tracking stays accurate, but they arenot fixed here and a maintainer should close them manually rather than relying
on the merge.
On the two not implemented
receipts/[txHash]/page.tsx,BasisPointsPieChart.tsx,CreateSplitWizardLegacy.tsx,DashboardView.tsx,ManageSplitView.tsx— andneeds a per-locale formatting policy agreed with product. Standardising these in
one pass without that decision risks changing how every timestamp in the product
renders.
main.frontend/scripts/check-i18n-keys.mjschecks locale parity against the baselocale, empty values, required wizard validation keys, and placeholder-argument
matching (
{remaining},{over}). Running it against the current tree showsenandfrat 38 keys each with zero drift in either direction.#1502 — contract ID checksum and format validation
The problem
CONTRACT_IDwas validated with a regex and nothing else:That is a format check, not a validity check. A Stellar StrKey carries a
trailing CRC-16 checksum over its own payload, so a value can satisfy the regex
perfectly and still be rejected by the network. The failure surfaced late and
opaquely: startup succeeded, the process came up healthy, and then the first real
call returned "contract not found".
The fix
Adds
StrKey.isValidContractalongside the existing regex. The regex is keptdeliberately, so the two failure modes produce different messages — wrong shape
versus well-formed-but-checksum-mismatched — because they need different advice.
The checksum message says what to do rather than restating the constraint:
must match /^C[A-Z2-7]{55}$/does not help anyone diagnose a typo. The singlewrong character is the actual cause.
Test fixtures
Two existing fixtures were well-formed but checksum-invalid, and both call
validateEnv(), so they would have failed on this change:backend/src/config/env.test.tsC+ 55As0000, computed5cc8backend/src/__tests__/cors.test.tsCBLASIRZ7C…H2LRBoth are replaced with
StrKey.encodeContract(Buffer.alloc(32, 7))rather thanhand-written literals, so the fixture cannot drift from what a real contract ID
looks like. This is the same call
splits.compatibility.test.ts:16already uses.database.test.tsanddatabase-pool-exhaustion.test.tscarry the same invalidvalue but never call
validateEnv()— they only stubprocess.env. They areleft alone; changing them would be unrelated churn.
#1507 — wallet disconnect state recovery
The problem
Account changes were handled. Disconnects were silently dropped.
When the user disconnects,
fresh.connectedisfalse, so the condition isfalse and nothing dispatches. State keeps
connected: trueand the previousaddress indefinitely.
Three consequences:
wallet-provider.tsxcan never fire — it keys off!walletState.wallet.connected, which is exactly the flag that was neverupdated.
This is not caught by a second listener:
lib/wallet.tsregisters no wallet eventlisteners at all, so this 2s poll is the only thing watching for the change.
The fix
Flips the guard so a disconnect takes the same
RESETpath as a switch:RESETreturnsinitialState(connected: false,address: null), which clearsthe stale balances. Account-switch handling is unchanged.
Files changed
backend/src/config/env.tsbackend/src/config/env.test.tsbackend/src/__tests__/cors.test.tsfrontend/src/hooks/useWallet.tsThe two fixture files are unavoidable: they call
validateEnv(), so leaving themwould mean shipping a change that breaks the existing suite.
Verification
Dependencies are not installed and no tests were executed, per the task
constraint. Verified by inspection:
@stellar/stellar-sdk@14.6.1is a backend dependency andStrKeyis alreadyimported by
backend/src/schemas/user.schemas.ts.validateEnv()were found by grepping everyCONTRACT_IDliteral in
backend/src—database.test.ts,database-pool-exhaustion.test.ts,deploy-config.test.tsandutils/deploy-config.test.tsdo not, and were left untouched.decode and checking each fixture against its own payload.
lib/wallet.tsregisters no event listeners, confirming the poll is the soledetection path.
SIMULATOR_ACCOUNT: "test_account"is validated with.min(1)only, so thecors.test.tsenv block is otherwise unaffected.Closes #1502
Closes #1504
Closes #1505
Closes #1507