fix: manage pending relay connection requests #159

Merged
eugene merged 6 commits from reduce-relay-connection-attempts into staging 2024-08-21 08:06:39 +00:00
8 changed files with 107 additions and 151 deletions
Showing only changes of commit f9fcfb1c9e - Show all commits

View File

@ -15,15 +15,16 @@ import {
findRelayListAndUpdateCache, findRelayListAndUpdateCache,
findRelayListInCache, findRelayListInCache,
getDefaultRelaySet, getDefaultRelaySet,
getMostPopularRelays,
getUserRelaySet, getUserRelaySet,
isOlderThanOneWeek, isOlderThanOneDay,
unixNow unixNow
} from '../utils' } from '../utils'
import { DEFAULT_LOOK_UP_RELAY_LIST } from '../utils/const'
export class MetadataController extends EventEmitter { export class MetadataController extends EventEmitter {
private nostrController: NostrController private nostrController: NostrController
private specialMetadataRelay = 'wss://purplepag.es' private specialMetadataRelay = 'wss://purplepag.es'
private pendingFetches = new Map<string, Promise<Event | null>>() // Track pending fetches
constructor() { constructor() {
super() super()
@ -42,70 +43,55 @@ export class MetadataController extends EventEmitter {
hexKey: string, hexKey: string,
currentEvent: Event | null currentEvent: Event | null
): Promise<Event | null> { ): Promise<Event | null> {
// Define the event filter to only include metadata events authored by the given key // Return the ongoing fetch promise if one exists for the same hexKey
const eventFilter: Filter = { if (this.pendingFetches.has(hexKey)) {
kinds: [kinds.Metadata], // Only metadata events return this.pendingFetches.get(hexKey)!
authors: [hexKey] // Authored by the specified key
} }
// Try to get the metadata event from a special relay (wss://purplepag.es) // Define the event filter to only include metadata events authored by the given key
const metadataEvent = await relayController const eventFilter: Filter = {
.fetchEvent(eventFilter, [this.specialMetadataRelay]) kinds: [kinds.Metadata],
authors: [hexKey]
}
const fetchPromise = relayController
.fetchEvent(eventFilter, DEFAULT_LOOK_UP_RELAY_LIST)
.catch((err) => { .catch((err) => {
console.error(err) // Log any errors console.error(err)
return null // Return null if an error occurs return null
})
.finally(() => {
this.pendingFetches.delete(hexKey)
}) })
// If a valid metadata event is found from the special relay this.pendingFetches.set(hexKey, fetchPromise)
const metadataEvent = await fetchPromise
if ( if (
metadataEvent && metadataEvent &&
validateEvent(metadataEvent) && // Validate the event validateEvent(metadataEvent) &&
verifyEvent(metadataEvent) // Verify the event's authenticity verifyEvent(metadataEvent)
) { ) {
// If there's no current event or the new metadata event is more recent
if ( if (
!currentEvent || !currentEvent ||
metadataEvent.created_at >= currentEvent.created_at metadataEvent.created_at >= currentEvent.created_at
) { ) {
// Handle the new metadata event
this.handleNewMetadataEvent(metadataEvent) this.handleNewMetadataEvent(metadataEvent)
} }
return metadataEvent return metadataEvent
} }
// If no valid metadata event is found from the special relay, get the most popular relays // todo/implement: if no valid metadata event is found in DEFAULT_LOOK_UP_RELAY_LIST
const mostPopularRelays = await getMostPopularRelays() // try to query user relay list
// Query the most popular relays for metadata events // if current event is null we should cache empty metadata event for provided hexKey
if (!currentEvent) {
const events = await relayController const emptyMetadata = this.getEmptyMetadataEvent(hexKey)
.fetchEvents(eventFilter, mostPopularRelays) this.handleNewMetadataEvent(emptyMetadata as VerifiedEvent)
.catch((err) => {
console.error(err) // Log any errors
return null // Return null if an error occurs
})
// If events are found from the popular relays
if (events && events.length) {
events.sort((a, b) => b.created_at - a.created_at) // Sort events by creation date (descending)
// Iterate through the events
for (const event of events) {
// If the event is valid, authentic, and more recent than the current event
if (
validateEvent(event) &&
verifyEvent(event) &&
(!currentEvent || event.created_at > currentEvent.created_at)
) {
// Handle the new metadata event
this.handleNewMetadataEvent(event)
return event
}
}
} }
return currentEvent // Return the current event if no newer event is found return currentEvent
} }
/** /**
@ -131,7 +117,7 @@ export class MetadataController extends EventEmitter {
// If cached metadata is found, check its validity // If cached metadata is found, check its validity
if (cachedMetadataEvent) { if (cachedMetadataEvent) {
// Check if the cached metadata is older than one week // Check if the cached metadata is older than one week
if (isOlderThanOneWeek(cachedMetadataEvent.cachedAt)) { if (isOlderThanOneDay(cachedMetadataEvent.cachedAt)) {
s marked this conversation as resolved
Review

There is a discrepancy between the function name and the comments around it. Should it be older than one week or one day?

There is a discrepancy between the function name and the comments around it. Should it be older than one week or one day?
Review

fixed, Updated comment

fixed, Updated comment
// If older than one week, find the metadata from relays in background // If older than one week, find the metadata from relays in background
this.checkForMoreRecentMetadata(hexKey, cachedMetadataEvent.event) this.checkForMoreRecentMetadata(hexKey, cachedMetadataEvent.event)
@ -161,11 +147,7 @@ export class MetadataController extends EventEmitter {
public findRelayListMetadata = async (hexKey: string): Promise<RelaySet> => { public findRelayListMetadata = async (hexKey: string): Promise<RelaySet> => {
const relayEvent = const relayEvent =
(await findRelayListInCache(hexKey)) || (await findRelayListInCache(hexKey)) ||
(await findRelayListAndUpdateCache( (await findRelayListAndUpdateCache(DEFAULT_LOOK_UP_RELAY_LIST, hexKey))
[this.specialMetadataRelay],
hexKey
)) ||
(await findRelayListAndUpdateCache(await getMostPopularRelays(), hexKey))
return relayEvent ? getUserRelaySet(relayEvent.tags) : getDefaultRelaySet() return relayEvent ? getUserRelaySet(relayEvent.tags) : getDefaultRelaySet()
} }
@ -216,13 +198,13 @@ export class MetadataController extends EventEmitter {
public validate = (event: Event) => validateEvent(event) && verifyEvent(event) public validate = (event: Event) => validateEvent(event) && verifyEvent(event)
public getEmptyMetadataEvent = (): Event => { public getEmptyMetadataEvent = (pubkey?: string): Event => {
return { return {
content: '', content: '',
created_at: new Date().valueOf(), created_at: new Date().valueOf(),
id: '', id: '',
kind: 0, kind: 0,
pubkey: '', pubkey: pubkey || '',
sig: '', sig: '',
tags: [] tags: []
} }

View File

@ -7,6 +7,7 @@ import { SIGIT_RELAY } from '../utils/const'
*/ */
export class RelayController { export class RelayController {
private static instance: RelayController private static instance: RelayController
private pendingConnections = new Map<string, Promise<Relay | null>>() // Track pending connections
public connectedRelays = new Map<string, Relay>() public connectedRelays = new Map<string, Relay>()
private constructor() {} private constructor() {}
@ -35,23 +36,26 @@ export class RelayController {
* @returns A promise that resolves to the connected relay object if successful, or `null` if the connection fails. * @returns A promise that resolves to the connected relay object if successful, or `null` if the connection fails.
*/ */
public connectRelay = async (relayUrl: string): Promise<Relay | null> => { public connectRelay = async (relayUrl: string): Promise<Relay | null> => {
// Check if a relay with the same URL is already connected
const normalizedWebSocketURL = normalizeWebSocketURL(relayUrl) const normalizedWebSocketURL = normalizeWebSocketURL(relayUrl)
const relay = this.connectedRelays.get(normalizedWebSocketURL) const relay = this.connectedRelays.get(normalizedWebSocketURL)
if (relay) { if (relay) {
// If a relay is found in connectedRelay map and is connected, just return it
if (relay.connected) return relay if (relay.connected) return relay
// If relay is found in connectedRelay map but not connected, // If relay is found in connectedRelay map but not connected,
// remove it from map and call connectRelay method again // remove it from map and call connectRelay method again
this.connectedRelays.delete(relayUrl) this.connectedRelays.delete(relayUrl)
return this.connectRelay(relayUrl) return this.connectRelay(relayUrl)
} }
// Attempt to connect to the relay using the provided URL // Check if there's already a pending connection for this relay URL
const newRelay = await Relay.connect(relayUrl) if (this.pendingConnections.has(relayUrl)) {
// Return the existing promise to avoid making another connection
return this.pendingConnections.get(relayUrl)!
}
// Create a new connection promise and store it in pendingConnections
const connectionPromise = Relay.connect(relayUrl)
.then((relay) => { .then((relay) => {
if (relay.connected) { if (relay.connected) {
// Add the newly connected relay to the connected relays map // Add the newly connected relay to the connected relays map
@ -70,8 +74,13 @@ export class RelayController {
// Return null to indicate connection failure // Return null to indicate connection failure
return null return null
}) })
.finally(() => {
// Remove the connection from pendingConnections once it settles
this.pendingConnections.delete(relayUrl)
})
return newRelay this.pendingConnections.set(relayUrl, connectionPromise)
return connectionPromise
} }
/** /**
@ -86,8 +95,13 @@ export class RelayController {
filter: Filter, filter: Filter,
relayUrls: string[] = [] relayUrls: string[] = []
): Promise<Event[]> => { ): Promise<Event[]> => {
// Add app relay to relays array and connect to all specified relays if (!relayUrls.includes(SIGIT_RELAY)) {
const relayPromises = [...relayUrls, SIGIT_RELAY].map((relayUrl) => // Add app relay to relays array if not exists already
relayUrls.push(SIGIT_RELAY)
}
// connect to all specified relays
const relayPromises = relayUrls.map((relayUrl) =>
this.connectRelay(relayUrl) this.connectRelay(relayUrl)
) )
@ -201,11 +215,15 @@ export class RelayController {
relayUrls: string[] = [], relayUrls: string[] = [],
eventHandler: (event: Event) => void eventHandler: (event: Event) => void
) => { ) => {
// Add app relay to relays array and connect to all specified relays if (!relayUrls.includes(SIGIT_RELAY)) {
// Add app relay to relays array if not exists already
relayUrls.push(SIGIT_RELAY)
}
const relayPromises = [...relayUrls, SIGIT_RELAY].map((relayUrl) => // connect to all specified relays
this.connectRelay(relayUrl) const relayPromises = relayUrls.map((relayUrl) => {
) return this.connectRelay(relayUrl)
})
// Use Promise.allSettled to wait for all promises to settle // Use Promise.allSettled to wait for all promises to settle
const results = await Promise.allSettled(relayPromises) const results = await Promise.allSettled(relayPromises)
@ -258,11 +276,16 @@ export class RelayController {
event: Event, event: Event,
relayUrls: string[] = [] relayUrls: string[] = []
): Promise<string[]> => { ): Promise<string[]> => {
// Add app relay to relays array and connect to all specified relays if (!relayUrls.includes(SIGIT_RELAY)) {
// Add app relay to relays array if not exists already
relayUrls.push(SIGIT_RELAY)
}
const relayPromises = [...relayUrls, SIGIT_RELAY].map((relayUrl) => // connect to all specified relays
this.connectRelay(relayUrl) const relayPromises = relayUrls.map((relayUrl) => {
) console.log('being called from publish events')
return this.connectRelay(relayUrl)
})
// Use Promise.allSettled to wait for all promises to settle // Use Promise.allSettled to wait for all promises to settle
const results = await Promise.allSettled(relayPromises) const results = await Promise.allSettled(relayPromises)

View File

@ -15,7 +15,6 @@ export const SET_USER_ROBOT_IMAGE = 'SET_USER_ROBOT_IMAGE'
export const SET_RELAY_MAP = 'SET_RELAY_MAP' export const SET_RELAY_MAP = 'SET_RELAY_MAP'
export const SET_RELAY_INFO = 'SET_RELAY_INFO' export const SET_RELAY_INFO = 'SET_RELAY_INFO'
export const SET_RELAY_MAP_UPDATED = 'SET_RELAY_MAP_UPDATED' export const SET_RELAY_MAP_UPDATED = 'SET_RELAY_MAP_UPDATED'
export const SET_MOST_POPULAR_RELAYS = 'SET_MOST_POPULAR_RELAYS'
export const UPDATE_USER_APP_DATA = 'UPDATE_USER_APP_DATA' export const UPDATE_USER_APP_DATA = 'UPDATE_USER_APP_DATA'
export const UPDATE_PROCESSED_GIFT_WRAPS = 'UPDATE_PROCESSED_GIFT_WRAPS' export const UPDATE_PROCESSED_GIFT_WRAPS = 'UPDATE_PROCESSED_GIFT_WRAPS'

View File

@ -1,7 +1,6 @@
import * as ActionTypes from '../actionTypes' import * as ActionTypes from '../actionTypes'
import { import {
SetRelayMapAction, SetRelayMapAction,
SetMostPopularRelaysAction,
SetRelayInfoAction, SetRelayInfoAction,
SetRelayMapUpdatedAction SetRelayMapUpdatedAction
} from './types' } from './types'
@ -19,13 +18,6 @@ export const setRelayInfoAction = (
payload payload
}) })
export const setMostPopularRelaysAction = (
payload: string[]
): SetMostPopularRelaysAction => ({
type: ActionTypes.SET_MOST_POPULAR_RELAYS,
payload
})
export const setRelayMapUpdatedAction = (): SetRelayMapUpdatedAction => ({ export const setRelayMapUpdatedAction = (): SetRelayMapUpdatedAction => ({
type: ActionTypes.SET_RELAY_MAP_UPDATED type: ActionTypes.SET_RELAY_MAP_UPDATED
}) })

View File

@ -4,7 +4,6 @@ import { RelaysDispatchTypes, RelaysState } from './types'
const initialState: RelaysState = { const initialState: RelaysState = {
map: undefined, map: undefined,
mapUpdated: undefined, mapUpdated: undefined,
mostPopular: undefined,
info: undefined info: undefined
} }
@ -25,9 +24,6 @@ const reducer = (
info: { ...state.info, ...action.payload } info: { ...state.info, ...action.payload }
} }
case ActionTypes.SET_MOST_POPULAR_RELAYS:
return { ...state, mostPopular: [...action.payload] }
case ActionTypes.RESTORE_STATE: case ActionTypes.RESTORE_STATE:
return action.payload.relays return action.payload.relays

View File

@ -5,7 +5,6 @@ import { RelayMap, RelayInfoObject } from '../../types'
export type RelaysState = { export type RelaysState = {
map?: RelayMap map?: RelayMap
mapUpdated?: number mapUpdated?: number
mostPopular?: string[]
info?: RelayInfoObject info?: RelayInfoObject
} }
@ -14,11 +13,6 @@ export interface SetRelayMapAction {
payload: RelayMap payload: RelayMap
} }
export interface SetMostPopularRelaysAction {
type: typeof ActionTypes.SET_MOST_POPULAR_RELAYS
payload: string[]
}
export interface SetRelayInfoAction { export interface SetRelayInfoAction {
type: typeof ActionTypes.SET_RELAY_INFO type: typeof ActionTypes.SET_RELAY_INFO
payload: RelayInfoObject payload: RelayInfoObject
@ -32,5 +26,4 @@ export type RelaysDispatchTypes =
| SetRelayMapAction | SetRelayMapAction
| SetRelayInfoAction | SetRelayInfoAction
| SetRelayMapUpdatedAction | SetRelayMapUpdatedAction
| SetMostPopularRelaysAction
| RestoreState | RestoreState

View File

@ -11,7 +11,18 @@ export const DEFLATE = 'DEFLATE'
/** /**
* Number of milliseconds in one week. * Number of milliseconds in one week.
* Calc based on: 7 * 24 * 60 * 60 * 1000
*/ */
export const ONE_WEEK_IN_MS: number = 604800000 export const ONE_WEEK_IN_MS = 7 * 24 * 60 * 60 * 1000
s marked this conversation as resolved
Review

Why do you need to calculate this constant instead of providing a defined value?

Why do you need to calculate this constant instead of providing a defined value?
Review

I guess this avoids any uncertainty about how the number was calculated, or if it was calculated correctly. It's a one time operation right?

I guess this avoids any uncertainty about how the number was calculated, or if it was calculated correctly. It's a one time operation right?
Review

yes, its one time.

yes, its one time.
export const SIGIT_RELAY: string = 'wss://relay.sigit.io'
/**
* Number of milliseconds in one day.
*/
export const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000
export const SIGIT_RELAY = 'wss://relay.sigit.io'
export const DEFAULT_LOOK_UP_RELAY_LIST = [
SIGIT_RELAY,
'wss://user.kindpag.es',
'wss://purplepag.es'
]

View File

@ -1,13 +1,14 @@
import axios from 'axios'
import { Event, Filter, kinds, UnsignedEvent } from 'nostr-tools' import { Event, Filter, kinds, UnsignedEvent } from 'nostr-tools'
import { RelayList } from 'nostr-tools/kinds' import { RelayList } from 'nostr-tools/kinds'
import { getRelayInfo, unixNow } from '.' import { getRelayInfo, unixNow } from '.'
import { NostrController, relayController } from '../controllers' import { NostrController, relayController } from '../controllers'
import { localCache } from '../services' import { localCache } from '../services'
import { setMostPopularRelaysAction } from '../store/actions' import { RelayMap, RelaySet } from '../types'
import store from '../store/store' import {
import { RelayMap, RelayReadStats, RelaySet, RelayStats } from '../types' DEFAULT_LOOK_UP_RELAY_LIST,
import { ONE_WEEK_IN_MS, SIGIT_RELAY } from './const' ONE_WEEK_IN_MS,
SIGIT_RELAY
} from './const'
const READ_MARKER = 'read' const READ_MARKER = 'read'
const WRITE_MARKER = 'write' const WRITE_MARKER = 'write'
@ -29,6 +30,7 @@ const findRelayListAndUpdateCache = async (
authors: [hexKey] authors: [hexKey]
} }
console.count('findRelayListAndUpdateCache')
const event = await relayController.fetchEvent(eventFilter, lookUpRelays) const event = await relayController.fetchEvent(eventFilter, lookUpRelays)
if (event) { if (event) {
await localCache.addUserRelayListMetadata(event) await localCache.addUserRelayListMetadata(event)
@ -90,6 +92,10 @@ const isOlderThanOneWeek = (cachedAt: number) => {
return Date.now() - cachedAt < ONE_WEEK_IN_MS return Date.now() - cachedAt < ONE_WEEK_IN_MS
} }
const isOlderThanOneDay = (cachedAt: number) => {
return Date.now() - cachedAt < ONE_WEEK_IN_MS
}
const isRelayTag = (tag: string[]): boolean => tag[0] === 'r' const isRelayTag = (tag: string[]): boolean => tag[0] === 'r'
const toRelaySet = (obj: RelaySet, tag: string[]): RelaySet => { const toRelaySet = (obj: RelaySet, tag: string[]): RelaySet => {
@ -110,51 +116,6 @@ const toRelaySet = (obj: RelaySet, tag: string[]): RelaySet => {
return obj return obj
} }
/**
* Provides most popular relays.
* @param numberOfTopRelays - number representing how many most popular relays to provide
* @returns - promise that resolves into an array of most popular relays
*/
const getMostPopularRelays = async (
numberOfTopRelays: number = 30
): Promise<string[]> => {
const mostPopularRelaysState = store.getState().relays?.mostPopular
// return most popular relays from app state if present
if (mostPopularRelaysState) return mostPopularRelaysState
// relays in env
const { VITE_MOST_POPULAR_RELAYS } = import.meta.env
const hardcodedPopularRelays = (VITE_MOST_POPULAR_RELAYS || '').split(' ')
const url = `https://stats.nostr.band/stats_api?method=stats`
const response = await axios.get<RelayStats>(url).catch(() => undefined)
if (!response) {
return hardcodedPopularRelays //return hardcoded relay list
}
const data = response.data
if (!data) {
return hardcodedPopularRelays //return hardcoded relay list
}
const apiTopRelays = data.relay_stats.user_picks.read_relays
.slice(0, numberOfTopRelays)
.map((relay: RelayReadStats) => relay.d)
if (!apiTopRelays.length) {
return Promise.reject(`Couldn't fetch popular relays.`)
}
if (store.getState().auth?.loggedIn) {
store.dispatch(setMostPopularRelaysAction(apiTopRelays))
}
return apiTopRelays
}
/** /**
* Provides relay map. * Provides relay map.
* @param npub - user's npub * @param npub - user's npub
@ -163,16 +124,15 @@ const getMostPopularRelays = async (
const getRelayMap = async ( const getRelayMap = async (
npub: string npub: string
): Promise<{ map: RelayMap; mapUpdated?: number }> => { ): Promise<{ map: RelayMap; mapUpdated?: number }> => {
const mostPopularRelays = await getMostPopularRelays()
// More info about this kind of event available https://github.com/nostr-protocol/nips/blob/master/65.md // More info about this kind of event available https://github.com/nostr-protocol/nips/blob/master/65.md
const eventFilter: Filter = { const eventFilter: Filter = {
kinds: [kinds.RelayList], kinds: [kinds.RelayList],
authors: [npub] authors: [npub]
} }
console.count('getRelayMap')
const event = await relayController const event = await relayController
.fetchEvent(eventFilter, mostPopularRelays) .fetchEvent(eventFilter, DEFAULT_LOOK_UP_RELAY_LIST)
.catch((err) => { .catch((err) => {
return Promise.reject(err) return Promise.reject(err)
}) })
@ -196,6 +156,7 @@ const getRelayMap = async (
}) })
Object.keys(relaysMap).forEach((relayUrl) => { Object.keys(relaysMap).forEach((relayUrl) => {
console.log('being called from getRelayMap')
s marked this conversation as resolved Outdated

Should this console log be removed?

Should this console log be removed?
Outdated
Review

removed

removed
relayController.connectRelay(relayUrl) relayController.connectRelay(relayUrl)
}) })
@ -255,9 +216,8 @@ const publishRelayMap = async (
// If relay map is empty, use most popular relay URIs // If relay map is empty, use most popular relay URIs
if (!relaysToPublish.length) { if (!relaysToPublish.length) {
relaysToPublish = await getMostPopularRelays() relaysToPublish = DEFAULT_LOOK_UP_RELAY_LIST
s marked this conversation as resolved Outdated

Should DEFAULT_LOOK_UP_RELAY_LIST be copied rather than re-assigned?

Should `DEFAULT_LOOK_UP_RELAY_LIST` be copied rather than re-assigned?
Outdated
Review

Its fine here because we're not updating the relaysToPublish array after this assignment in this function (which would have caused side effects on DEFAULT_LOOK_UP_RELAY_LIST).
Further I'm handling the case where I need to append the array by copying the array in the relay controller.

Its fine here because we're not updating the relaysToPublish array after this assignment in this function (which would have caused side effects on DEFAULT_LOOK_UP_RELAY_LIST). Further I'm handling the case where I need to append the array by copying the array in the relay controller.
} }
const publishResult = await relayController.publish( const publishResult = await relayController.publish(
signedEvent, signedEvent,
relaysToPublish relaysToPublish
@ -277,9 +237,9 @@ export {
findRelayListInCache, findRelayListInCache,
getDefaultRelayMap, getDefaultRelayMap,
getDefaultRelaySet, getDefaultRelaySet,
getMostPopularRelays,
getRelayMap, getRelayMap,
publishRelayMap,
getUserRelaySet, getUserRelaySet,
isOlderThanOneWeek isOlderThanOneDay,
isOlderThanOneWeek,
publishRelayMap
} }