Skip to content

Commit

Permalink
address findings of code review
Browse files Browse the repository at this point in the history
  • Loading branch information
manuelsc committed Oct 24, 2023
1 parent 106e7a9 commit 128440d
Show file tree
Hide file tree
Showing 14 changed files with 90 additions and 89 deletions.
6 changes: 3 additions & 3 deletions src/app/components/dashboard/dashboard.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -851,10 +851,10 @@
</ion-label>
</ion-item>

<ion-item lines="none" *ngIf="(unit.pref.Cons.value != 'ETHER' && unit.pref.Cons.value != 'FINNEY') || classReference.currencyPipe.Cons != null">
<ion-label class="stat-title"> {{ unit.pref.Cons.getCurrencyName() }} Price </ion-label>
<ion-item lines="none" *ngIf="unit.pref.Cons.value != unit.getNetworkDefaultCurrency('cons') || classReference.currencyPipe.Cons != null">
<ion-label class="stat-title"> {{ unit.getNetworkDefaultUnit('cons').display }} Price </ion-label>
<ion-label class="value" (click)="unit.switchCurrencyPipe()">
{{ 1 | mcurrency : 'ETHER' : unit.pref.Cons }}
{{ unit.lastPrice.Cons | mcurrency : unit.getFiatCurrency('cons') : unit.getFiatCurrency('cons') }}
</ion-label>
</ion-item>

Expand Down
25 changes: 15 additions & 10 deletions src/app/components/dashboard/dashboard.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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)})`
}
}

Expand Down Expand Up @@ -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 += `<b><span style="color:${tooltip.chart.hoverPoints[i].color}">●</span> ${
Expand All @@ -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 += `<b>Total: ${getValueString(total, 'cons')}</b>`
}

Expand Down
5 changes: 4 additions & 1 deletion src/app/components/machinechart/machinechart.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -26,6 +27,8 @@ export class MachinechartComponent implements OnInit {
chartError = false
specificError: string = null

constructor(private api: ApiService) {}

doClick() {
this.clickAction()
}
Expand Down Expand Up @@ -58,7 +61,7 @@ export class MachinechartComponent implements OnInit {
}

ngOnInit() {
highChartOptions(Highstock)
highChartOptions(Highstock, this.api.getHostName())
this.id = makeid(6)
}

Expand Down
2 changes: 1 addition & 1 deletion src/app/pages/notifications/notifications.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
2 changes: 1 addition & 1 deletion src/app/pages/subscribe/subscribe.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 4 additions & 4 deletions src/app/services/api.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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<unknown>): string {
Expand Down
24 changes: 22 additions & 2 deletions src/app/services/unitconv.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

/*
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 9 additions & 4 deletions src/app/tab-preferences/notification-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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.'
)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/app/tab-preferences/tab-preferences.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
7 changes: 2 additions & 5 deletions src/app/utils/BlockUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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<BigNumber> {
const proposer = block.posConsensus.proposerIndex
Expand Down
27 changes: 18 additions & 9 deletions src/app/utils/CacheModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,23 @@ export class CacheModule {
private cache: Map<string, CachedData> = new Map()
private hotOnly: Map<string, CachedData> = 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()
Expand Down Expand Up @@ -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()
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/app/utils/HighchartOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
* // along with Beaconchain Dashboard. If not, see <http://www.gnu.org/licenses/>.
*/

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)',
Expand Down
4 changes: 2 additions & 2 deletions src/app/utils/MerchantUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
Loading

0 comments on commit 128440d

Please sign in to comment.