From 128440d8f4ca1457eda8860699ca763728cb4b77 Mon Sep 17 00:00:00 2001 From: Manuel <5877862+manuelsc@users.noreply.github.com> Date: Tue, 24 Oct 2023 15:15:27 +0200 Subject: [PATCH] address findings of code review --- .../dashboard/dashboard.component.html | 6 +-- .../dashboard/dashboard.component.ts | 25 ++++++---- .../machinechart/machinechart.component.ts | 5 +- .../pages/notifications/notifications.page.ts | 2 +- src/app/pages/subscribe/subscribe.page.ts | 2 +- src/app/services/api.service.ts | 8 +-- src/app/services/unitconv.service.ts | 24 ++++++++- src/app/tab-preferences/notification-base.ts | 13 +++-- .../tab-preferences/tab-preferences.page.ts | 3 +- src/app/utils/BlockUtils.ts | 7 +-- src/app/utils/CacheModule.ts | 27 ++++++---- src/app/utils/HighchartOptions.ts | 4 +- src/app/utils/MerchantUtils.ts | 4 +- src/app/utils/ValidatorUtils.ts | 49 +++---------------- 14 files changed, 90 insertions(+), 89 deletions(-) diff --git a/src/app/components/dashboard/dashboard.component.html b/src/app/components/dashboard/dashboard.component.html index 0e99af21..6fe7af03 100644 --- a/src/app/components/dashboard/dashboard.component.html +++ b/src/app/components/dashboard/dashboard.component.html @@ -851,10 +851,10 @@ - - {{ unit.pref.Cons.getCurrencyName() }} Price + + {{ unit.getNetworkDefaultUnit('cons').display }} Price - {{ 1 | mcurrency : 'ETHER' : unit.pref.Cons }} + {{ unit.lastPrice.Cons | mcurrency : unit.getFiatCurrency('cons') : unit.getFiatCurrency('cons') }} diff --git a/src/app/components/dashboard/dashboard.component.ts b/src/app/components/dashboard/dashboard.component.ts index 30b1557a..d64fdf3e 100644 --- a/src/app/components/dashboard/dashboard.component.ts +++ b/src/app/components/dashboard/dashboard.component.ts @@ -336,8 +336,8 @@ export class DashboardComponent implements OnInit { //this.doneLoading = false this.storage.getBooleanSetting('rank_percent_mode', false).then((result) => (this.rankPercentMode = result)) this.storage.getItem('rpl_pdisplay_mode').then((result) => (this.rplState = result ? result : 'rpl')) - highChartOptions(HighCharts) - highChartOptions(Highstock) + highChartOptions(HighCharts, this.api.getHostName()) + highChartOptions(Highstock, this.api.getHostName()) this.merchant.getCurrentPlanMaxValidator().then((result) => { this.currentPackageMaxValidators = result }) @@ -661,14 +661,14 @@ export class DashboardComponent implements OnInit { const network = this.api.getNetwork() const getValueString = (value: BigNumber, type: RewardType): string => { - let text = `${value.toFixed(5)} ETH` + let text = `${value.toFixed(5)} ` + this.unit.getNetworkDefaultUnit(type).display if (type == 'cons') { - if (this.unit.isDefaultCurrency(this.unit.pref.Cons) && network.key == 'main') { - text += ` (${this.unit.convertToPref(value, 'ETHER', type)})` + if (!this.unit.isDefaultCurrency(this.unit.pref.Cons)) { + text += ` (${this.unit.convertToPref(value, this.unit.getNetworkDefaultCurrency(type), type)})` } } else if (type == 'exec') { - if (this.unit.isDefaultCurrency(this.unit.pref.Exec) && network.key == 'main') { - text += ` (${this.unit.convertToPref(value, 'ETHER', type)})` + if (!this.unit.isDefaultCurrency(this.unit.pref.Exec)) { + text += ` (${this.unit.convertToPref(value, this.unit.getNetworkDefaultCurrency(type), type)})` } } @@ -720,11 +720,16 @@ export class DashboardComponent implements OnInit { let text = this.getChartToolTipCaption(tooltip.chart.hoverPoints[0].x, network.genesisTs, tooltip.chart.hoverPoints[0].dataGroup.length) // income - let consAndExecSameCurrency = 0 + let lastCurrency = null + let consAndExecSameCurrency = true let total = new BigNumber(0) for (let i = 0; i < tooltip.chart.hoverPoints.length; i++) { const type = tooltip.chart.hoverPoints[i].series.name == 'Execution' ? 'exec' : 'cons' - consAndExecSameCurrency += type == 'exec' ? 1 : -1 + const currency = this.unit.getNetworkDefaultCurrency(type) + if (lastCurrency != null && lastCurrency != currency) { + consAndExecSameCurrency = false + } + lastCurrency = currency const value = new BigNumber(tooltip.chart.hoverPoints[i].y) text += `● ${ @@ -735,7 +740,7 @@ export class DashboardComponent implements OnInit { // add total if hovered point contains rewards for both EL and CL // only if both exec and cons currencies are the same - if (tooltip.chart.hoverPoints.length > 1 && Math.abs(consAndExecSameCurrency) == 2) { + if (tooltip.chart.hoverPoints.length > 1 && consAndExecSameCurrency) { text += `Total: ${getValueString(total, 'cons')}` } diff --git a/src/app/components/machinechart/machinechart.component.ts b/src/app/components/machinechart/machinechart.component.ts index 387f103d..2374c8fa 100644 --- a/src/app/components/machinechart/machinechart.component.ts +++ b/src/app/components/machinechart/machinechart.component.ts @@ -3,6 +3,7 @@ import { highChartOptions } from 'src/app/utils/HighchartOptions' import * as HighCharts from 'highcharts' import * as Highstock from 'highcharts/highstock' import { MachineChartData } from 'src/app/controllers/MachineController' +import { ApiService } from 'src/app/services/api.service' @Component({ selector: 'app-machinechart', @@ -26,6 +27,8 @@ export class MachinechartComponent implements OnInit { chartError = false specificError: string = null + constructor(private api: ApiService) {} + doClick() { this.clickAction() } @@ -58,7 +61,7 @@ export class MachinechartComponent implements OnInit { } ngOnInit() { - highChartOptions(Highstock) + highChartOptions(Highstock, this.api.getHostName()) this.id = makeid(6) } diff --git a/src/app/pages/notifications/notifications.page.ts b/src/app/pages/notifications/notifications.page.ts index 726d0400..b0329c7a 100644 --- a/src/app/pages/notifications/notifications.page.ts +++ b/src/app/pages/notifications/notifications.page.ts @@ -84,7 +84,7 @@ export class NotificationsPage extends NotificationBase implements OnInit { }) this.storage.getAuthUser().then((result) => (this.authUser = result)) - this.network = this.api.getNetworkName() + this.network = this.api.capitalize(this.api.getNetworkName()) this.merchantUtils.hasCustomizableNotifications().then((result) => { this.canCustomizeThresholds = result }) diff --git a/src/app/pages/subscribe/subscribe.page.ts b/src/app/pages/subscribe/subscribe.page.ts index 8fc5426b..9490e426 100644 --- a/src/app/pages/subscribe/subscribe.page.ts +++ b/src/app/pages/subscribe/subscribe.page.ts @@ -101,7 +101,7 @@ export class SubscribePage implements OnInit { async purchaseIntern() { let loggedIn = await this.storage.isLoggedIn() if (!loggedIn) { - this.alertService.confirmDialog('Login', 'You need to login to your beaconcha.in account first. Continue?', 'Login', () => { + this.alertService.confirmDialog('Login', 'You need to login to your ' + this.api.getHostName() + ' account first. Continue?', 'Login', () => { this.oauth.login().then(async () => { loggedIn = await this.storage.isLoggedIn() if (loggedIn) this.continuePurchaseIntern() diff --git a/src/app/services/api.service.ts b/src/app/services/api.service.ts index 8beb6b3c..b09bc3a7 100644 --- a/src/app/services/api.service.ts +++ b/src/app/services/api.service.ts @@ -48,7 +48,7 @@ export class ApiService extends CacheModule { private lastCacheInvalidate = 0 constructor(private storage: StorageService) { - super('api', 6 * 60 * 1000, storage, false) + super('api', 6 * 60 * 1000, storage, 1000, false) this.storage.getBooleanSetting('migrated_4_4_0', false).then((migrated) => { if (!migrated) { this.clearHardCache() @@ -179,13 +179,13 @@ export class ApiService extends CacheModule { this.awaitingResponses[resource].release() } - isNotMainnet(): boolean { + isNotEthereumMainnet(): boolean { const test = this.networkConfig.net != '' return test } - isMainnet(): boolean { - return !this.isNotMainnet() + isEthereumMainnet(): boolean { + return !this.isNotEthereumMainnet() } private getCacheKey(request: APIRequest): string { diff --git a/src/app/services/unitconv.service.ts b/src/app/services/unitconv.service.ts index 753e6e6d..192fbd9e 100644 --- a/src/app/services/unitconv.service.ts +++ b/src/app/services/unitconv.service.ts @@ -40,7 +40,7 @@ export class UnitconvService { Exec: { value: 'ETHER', type: 'exec', unit: Unit.ETHER } as Currency, RPL: { value: 'ETHER', type: 'rpl', unit: Unit.RPL } as Currency, } - private lastPrice: LastPrice + public lastPrice: LastPrice private rplETHPrice: BigNumber = new BigNumber(1) /* @@ -151,6 +151,10 @@ export class UnitconvService { return currency.value == this.getNetworkDefaultCurrency(currency.type) } + public getNetworkDefaultUnit(type: RewardType): Unit { + return MAPPING.get(this.getNetworkDefaultCurrency(type)) + } + public getNetworkDefaultCurrency(type: RewardType | Currency): string { if (typeof type == 'object') { type = type.type @@ -168,6 +172,22 @@ export class UnitconvService { return 'ETHER' } + public getFiatCurrency(type: RewardType) { + if (type == 'cons') { + if (this.isDefaultCurrency(this.pref.Cons)) { + return UnitconvService.currencyPipe.Cons + } else { + return this.pref.Cons.value + } + } else if (type == 'exec') { + if (this.isDefaultCurrency(this.pref.Exec)) { + return UnitconvService.currencyPipe.Exec + } else { + return this.pref.Exec.value + } + } + } + private async getLastStoredPrice(currency: Currency) { const lastUpdatedConsPrice = (await this.storage.getObject(this.getLastPriceKey(currency))) as LastPrice if (lastUpdatedConsPrice && lastUpdatedConsPrice.lastPrice) { @@ -256,7 +276,7 @@ export class UnitconvService { * (cons and exec could have different conversions so they cant use the same reference to unit) */ public convertNonFiat(value: BigNumber | number | string, from: string, to: string, displayable = true) { - if (MAPPING.get(to).coinbaseSpot != null && to != 'ETH' && to != 'ETHER') { + if (MAPPING.get(to).coinbaseSpot != null && to != 'ETH' && to != 'ETHER' && from != to) { console.warn('convertNonFiat does not support fiat currencies. Use convert instead', value.toString(), from, to, displayable) } return this.convertBase(value, from, MAPPING.get(to), displayable) diff --git a/src/app/tab-preferences/notification-base.ts b/src/app/tab-preferences/notification-base.ts index f71da124..c92c3de8 100644 --- a/src/app/tab-preferences/notification-base.ts +++ b/src/app/tab-preferences/notification-base.ts @@ -158,7 +158,7 @@ export class NotificationBase implements OnInit { // locking toggle so we dont execute onChange when setting initial values const preferences = await this.storage.loadPreferencesToggles(net) - if (this.api.isNotMainnet()) { + if (this.api.isNotEthereumMainnet()) { this.notify = preferences this.notifyInitialized = true this.disableToggleLock() @@ -188,7 +188,9 @@ export class NotificationBase implements OnInit { if (!(await this.isSupportedOnAndroid())) { this.alerts.showInfo( 'Play Service', - 'Your device can not receive push notifications. Please note that notifications do not work without Google Play Services. As an alternative you can configure webhook notifications on the beaconcha.in website, otherwise changing these settings will have no effect.' + 'Your device can not receive push notifications. Please note that notifications do not work without Google Play Services. As an alternative you can configure webhook notifications on the ' + + this.api.getHostName() + + ' website, otherwise changing these settings will have no effect.' ) } @@ -208,7 +210,10 @@ export class NotificationBase implements OnInit { const net = this.api.networkConfig.net this.storage.setBooleanSetting(net + SETTING_NOTIFY, this.notify) this.settingsChanged = true - if (this.api.isMainnet()) { + // TODO: instead of using this.api.isEthereumMainnet(), check each app supported network and if + // one single network has notifications enabled, also sync general als true. If all are disabled, + // set general notify as false + if (this.api.isEthereumMainnet() || this.notify == true) { this.sync.changeGeneralNotify(this.notify) } @@ -298,7 +303,7 @@ export class NotificationBase implements OnInit { this.sync.changeClient(clientKey, clientKey) } else { this.sync.changeClient(clientKey, 'null') - this.api.deleteAllCacheKeyContains(clientKey) + this.api.deleteAllHardStorageCacheKeyContains(clientKey) } } diff --git a/src/app/tab-preferences/tab-preferences.page.ts b/src/app/tab-preferences/tab-preferences.page.ts index b3355650..5feb7dd5 100644 --- a/src/app/tab-preferences/tab-preferences.page.ts +++ b/src/app/tab-preferences/tab-preferences.page.ts @@ -243,7 +243,7 @@ export class Tab3Page { if (this.changeCurrencyLocked) return this.changeCurrencyLocked = true - await this.api.deleteAllCacheKeyContains('coinbase') + await this.api.deleteAllHardStorageCacheKeyContains('coinbase') this.overrideDisplayCurrency = this.unit.pref await this.unit.changeCurrency(this.currentFiatCurrency) @@ -462,7 +462,6 @@ export async function changeNetwork( const newConfig = findConfigForKey(network) await storage.clearCache() //await api.clearNetworkCache() - await validatorUtils.clearCache() await storage.setNetworkPreferences(newConfig) await api.initialize() diff --git a/src/app/utils/BlockUtils.ts b/src/app/utils/BlockUtils.ts index e577e73d..312fbad4 100644 --- a/src/app/utils/BlockUtils.ts +++ b/src/app/utils/BlockUtils.ts @@ -21,7 +21,6 @@ import { ApiService } from '../services/api.service' import { Injectable } from '@angular/core' import { BlockProducedByRequest, BlockResponse, DashboardRequest } from '../requests/requests' -import { CacheModule } from './CacheModule' import BigNumber from 'bignumber.js' import { ValidatorUtils } from './ValidatorUtils' @@ -33,10 +32,8 @@ const MONTH = 60 * 60 * 24 * 30 @Injectable({ providedIn: 'root', }) -export class BlockUtils extends CacheModule { - constructor(public api: ApiService, public validatorUtils: ValidatorUtils) { - super() - } +export class BlockUtils { + constructor(public api: ApiService, public validatorUtils: ValidatorUtils) {} async getBlockRewardWithShare(block: BlockResponse): Promise { const proposer = block.posConsensus.proposerIndex diff --git a/src/app/utils/CacheModule.ts b/src/app/utils/CacheModule.ts index 4a97e17d..f46e6f7d 100644 --- a/src/app/utils/CacheModule.ts +++ b/src/app/utils/CacheModule.ts @@ -37,14 +37,23 @@ export class CacheModule { private cache: Map = new Map() private hotOnly: Map = new Map() - constructor(keyPrefix = '', staleTime = 6 * 60 * 1000, hardStorage: StorageService = null, callInit: boolean = true) { + private hardStorageSizeLimit: number + + constructor( + keyPrefix = '', + staleTime = 6 * 60 * 1000, + hardStorage: StorageService = null, + hardStorageSizeLimit: number = 1000, + callInit: boolean = true + ) { this.keyPrefix = keyPrefix this.staleTime = staleTime this.hardStorage = hardStorage + this.hardStorageSizeLimit = hardStorageSizeLimit if (callInit) this.init() } - async init() { + protected async init() { if (this.hardStorage) { this.hardStorage.setObject('cachemodule_' + this.keyPrefix, null, false) await this.initHardCache() @@ -76,7 +85,7 @@ export class CacheModule { kiloBytes = Math.round((size * 100) / 1024) / 100 } console.log('[CacheModule] initialized with ', kiloBytes == null ? '(unknown size)' : '(' + kiloBytes + ' KiB)', this.cache) - if (kiloBytes && kiloBytes > 1000) { + if (kiloBytes && kiloBytes > this.hardStorageSizeLimit) { console.warn('[CacheModule] storage cap exceeded (1 MB), clearing cache') await this.clearHardCache() } @@ -94,7 +103,7 @@ export class CacheModule { return storeHard ? this.cache : this.hotOnly } - public async deleteAllCacheKeyContains(search: string) { + public async deleteAllHardStorageCacheKeyContains(search: string) { if (!this.hardStorage) { return } @@ -137,21 +146,21 @@ export class CacheModule { } } - async clearCache() { + public async clearCache() { await this.clearHardCache() this.hotOnly.clear() } - async clearNetworkCache() { + public async clearNetworkCache() { if (this.hardStorage) { const network = await this.hardStorage.getNetworkPreferences() - this.deleteAllCacheKeyContains(network.key == 'main' ? '//beaconcha.in' : '//' + network.key) + this.deleteAllHardStorageCacheKeyContains(network.key == 'main' ? '//beaconcha.in' : '//' + network.key) } this.hotOnly.clear() } - async clearHardCache() { + public async clearHardCache() { if (this.hardStorage) { await this.hardStorage.setObject('cachemodule2_' + this.keyPrefix, null, false) this.setLastHardCacheWrite() @@ -207,7 +216,7 @@ export class CacheModule { this.cache[this.getKey(key)] = null } - invalidateAllCache() { + public invalidateAllCache() { this.cache = new Map() if (this.hardStorage) { this.hardStorage.setObject('cachemodule2_' + this.keyPrefix, null, false) diff --git a/src/app/utils/HighchartOptions.ts b/src/app/utils/HighchartOptions.ts index d3a9cd65..7482d61e 100644 --- a/src/app/utils/HighchartOptions.ts +++ b/src/app/utils/HighchartOptions.ts @@ -18,14 +18,14 @@ * // along with Beaconchain Dashboard. If not, see . */ -export function highChartOptions(where) { +export function highChartOptions(where, hostName) { where.setOptions({ time: { useUTC: false, }, credits: { enabled: true, - href: 'https://beaconcha.in', + href: 'https://' + hostName, text: '', style: { color: 'var(--body-color)', diff --git a/src/app/utils/MerchantUtils.ts b/src/app/utils/MerchantUtils.ts index 05a42ed4..fd8f4876 100644 --- a/src/app/utils/MerchantUtils.ts +++ b/src/app/utils/MerchantUtils.ts @@ -386,7 +386,7 @@ export class MerchantUtils { const currentProduct = this.findProduct(currentPlan) if (currentProduct == null) return 100 - const notMainnet = this.api.isNotMainnet() + const notMainnet = this.api.isNotEthereumMainnet() if (notMainnet) return currentProduct.maxTestnetValidators return currentProduct.maxValidators } @@ -395,7 +395,7 @@ export class MerchantUtils { const currentProduct = this.findProduct(MAX_PRODUCT) if (currentProduct == null) return 100 - const notMainnet = this.api.isNotMainnet() + const notMainnet = this.api.isNotEthereumMainnet() if (notMainnet) return currentProduct.maxTestnetValidators return currentProduct.maxValidators } diff --git a/src/app/utils/ValidatorUtils.ts b/src/app/utils/ValidatorUtils.ts index 1422185d..7dcae3e9 100644 --- a/src/app/utils/ValidatorUtils.ts +++ b/src/app/utils/ValidatorUtils.ts @@ -38,7 +38,6 @@ import { ETH1ValidatorResponse, SyncCommitteesStatisticsResponse, } from '../requests/requests' -import { CacheModule } from './CacheModule' import { MerchantUtils } from './MerchantUtils' import BigNumber from 'bignumber.js' import { UnitconvService } from '../services/unitconv.service' @@ -52,12 +51,6 @@ const KEYPREFIX = 'validators_' export const LAST_TIME_ADDED_KEY = 'last_time_added' export const LAST_TIME_REMOVED_KEY = 'last_time_removed' -const cachePerformanceKeyBare = 'performance' -const cacheAttestationKeyBare = 'attestationperformance' -const epochCachedKeyBare = 'epochcached' -const allMyKeyBare = 'allmy' -const cacheValidatorsKeyBare = 'validators_' - export enum ValidatorState { ACTIVE, OFFLINE, @@ -90,7 +83,7 @@ export interface Validator { @Injectable({ providedIn: 'root', }) -export class ValidatorUtils extends CacheModule { +export class ValidatorUtils { private listeners: (() => void)[] = [] private currentEpoch: EpochResponse @@ -103,9 +96,7 @@ export class ValidatorUtils extends CacheModule { private storage: StorageService, private merchantUtils: MerchantUtils, private unitConversion: UnitconvService - ) { - super('vu_') // initialize cache module with vu prefix - } + ) {} notifyListeners() { this.listeners.forEach((callback) => callback()) @@ -266,13 +257,6 @@ export class ValidatorUtils extends CacheModule { const validatorString = getValidatorQueryString([...local.values()], 2000, (await this.merchantUtils.getCurrentPlanMaxValidator()) - 1) - // TODO:: - /* const cached = await this.getMultipleCached(allMyKeyBare, validatorString.split(",")) - if (cached != null) { - console.log("return my validators from cache") - return cached - }*/ - const remoteUpdatesPromise = this.getDashboardDataValidators(SAVED, validatorString).catch((err) => { console.warn('error getAllMyValidators getDashboardDataValidators', err) return [] @@ -290,8 +274,6 @@ export class ValidatorUtils extends CacheModule { const result = [...local.values()] - this.cacheMultiple(allMyKeyBare, result) - return result } @@ -375,12 +357,10 @@ export class ValidatorUtils extends CacheModule { console.warn('error getDashboardDataValidators', response, result) return [] } - if (result.currentEpoch && result.currentEpoch.length > 0) { - this.currentEpoch = result.currentEpoch[0] - } - if (result.olderEpoch && result.olderEpoch.length > 0) { - this.olderEpoch = result.olderEpoch[0] - } + + this.currentEpoch = result.currentEpoch[0] + this.olderEpoch = result.olderEpoch[0] + if (result.rocketpool_network_stats && result.rocketpool_network_stats.length > 0) { this.rocketpoolStats = result.rocketpool_network_stats[0] } @@ -520,22 +500,6 @@ export class ValidatorUtils extends CacheModule { return Promise.reject(new Error('Response is invalid')) } - getCachedAttestationKey() { - return cacheAttestationKeyBare + this.api.getNetworkName() - } - - getCachedPerformanceKey() { - return cachePerformanceKeyBare + this.api.getNetworkName() - } - - getCachedValidatorKey() { - return cacheValidatorsKeyBare + this.api.getNetworkName() - } - - getCachedEpochKey() { - return epochCachedKeyBare + this.api.getNetworkName() - } - // single async convertToValidatorModelAndSaveValidatorLocal(synced: boolean, validator: ValidatorResponse) { await this.convertToValidatorModelsAndSaveLocal(synced, [validator]) @@ -546,7 +510,6 @@ export class ValidatorUtils extends CacheModule { await this.saveValidatorsLocal(this.convertToValidatorModel({ synced, storage: SAVED, validatorResponse: validator })) if (!synced) { await this.storage.setObject(LAST_TIME_ADDED_KEY, { timestamp: Date.now() } as StoredTimestamp) - await this.clearCache() } }