fix: resolve XBullAdapter listener leak, computeMovingAverage, cert pinning, TOML version check - #896
Merged
Merged
Conversation
…it#780, Stellar-split#779 Stellar-split#840 — XBullAdapter: drop previous onAccountChange registration before installing a new one in setupAccountChangeListener(). Mirrors LobstrAdapter. Repeated connect() calls now leave at most one live listener; disconnect() leaves zero; a post-disconnect account-change event cannot write back into an adapter the app has torn down. Stellar-split#781 — Add computeMovingAverage(samples, windowSize) to src/fees/trend.ts. Returns a Simple Moving Average series of the same length as the input, padding the first windowSize-1 entries with NaN. Throws RangeError when windowSize < 1. Pure function with no side effects. Stellar-split#780 — AnchorVerifier gains an optional pinnedCertFingerprints option (Record<domain, SHA-256 fingerprint>). When a fingerprint is configured for a domain, the TLS certificate is verified before the TOML is fetched or trusted. A mismatch throws CertificatePinningError naming the domain. _fetchCertFingerprint is injectable for unit-test isolation. Stellar-split#779 — StellarTomlParser exports SUPPORTED_TOML_VERSIONS ([2.0, 2.1]) and validates the VERSION field immediately after successful TOML parsing. An unsupported version throws UnsupportedTomlVersionError naming the encountered version. Parsing a TOML without a VERSION field is accepted unchanged. VERSION check runs before caching so a rejected parse never pollutes the cache. CertificatePinningError and UnsupportedTomlVersionError are added to src/errors.ts following the existing StellarSplitError pattern. All new symbols are exported from src/index.ts. 129 tests pass.
|
@iam-mercy 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! 🚀 |
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
Resolves four Wave issues in a single PR. Each fix is scoped to its target file and does not touch unrelated code.
#840 — XBullAdapter leaks account-change listeners on reconnect
Issue:
setupAccountChangeListener()overwrotethis.unsubscribewithout calling the previous one. After threeconnect()calls, three listeners were live;disconnect()removed only the last one. A post-disconnect account-change event then wrote an address back into an adapter the app believed torn down.Fix: Drop the previous registration before installing a new one — mirroring
LobstrAdapter.setupAccountChangeListener().disconnect()needed no change; with at most one live listener its existingunsubscribecall is sufficient.Files:
src/wallets/adapters/XBullAdapter.ts,test/xbullAdapter.test.ts#781 — Add
computeMovingAveragetofees/trend.tsIssue:
fees/trend.tsexposed no derived statistics. Callers needed a smoothed signal for fee forecasting.Fix: Exported a pure
computeMovingAverage(samples, windowSize)function that returns an SMA series of the same length, padding the firstwindowSize - 1entries withNaN. ThrowsRangeErrorwhenwindowSize < 1.Files:
src/fees/trend.ts,test/feeTrend.test.ts#780 — Certificate pinning for anchor HTTPS endpoints in
AnchorVerifierIssue:
AnchorVerifiervalidated anchor metadata but did not verify TLS certificates, leaving it open to compromised DNS or rogue CAs.Fix: Added optional
pinnedCertFingerprints?: Record<string, string>toAnchorVerifierOptions. When a domain appears in the map, the SHA-256 fingerprint of its TLS certificate is retrieved and compared before the TOML is fetched. A mismatch throwsCertificatePinningError(added toerrors.ts) naming the domain. An injectable_fetchCertFingerprintoption is provided for test isolation.Files:
src/anchors/AnchorVerifier.ts,src/errors.ts,test/anchorPinningAndVersion.test.ts#779 — Validate TOML schema version in
StellarTomlParserIssue:
StellarTomlParserparsedstellar.tomlfiles without checkingVERSION, so a breaking schema revision would be processed silently.Fix: Exported
SUPPORTED_TOML_VERSIONS: readonly number[] = [2.0, 2.1]. After successful TOML parse (and before caching),VERSIONis read; if present and not inSUPPORTED_TOML_VERSIONS, anUnsupportedTomlVersionError(added toerrors.ts) is thrown naming the encountered version. Files without aVERSIONfield are accepted unchanged.Files:
src/anchors/StellarTomlParser.ts,src/errors.ts,test/anchorPinningAndVersion.test.tsTesting
All changes are covered by unit tests:
test/xbullAdapter.test.ts— 7 tests covering single connect, repeated connect, disconnect listener counts, post-disconnect event immunity, andonAccountChangehandler lifecycle.test/feeTrend.test.ts— 12 tests covering correct SMA values, NaN padding, edge cases (empty, single-element, windowSize > length), RangeError on invalid windowSize, and purity.test/anchorPinningAndVersion.test.ts— 23 tests coveringSUPPORTED_TOML_VERSIONSshape, supported/unsupported/missing VERSION, cache behaviour on version error, fingerprint match/mismatch, case-insensitive comparison, timeout forwarding,CertificatePinningErrorandUnsupportedTomlVersionErrorshape.129 tests pass. Existing
test/anchorVerifier.test.ts(19 tests) continues to pass unchanged.closes #840
closes #781
closes #780
closes #779