Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: renterd recommendations states and crashing #620

Merged
merged 1 commit into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fast-colts-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

Fixed an issue where the recommendations bar was overlapping notifications.
5 changes: 5 additions & 0 deletions .changeset/old-candles-remember.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

Fixed an issue where clearing some configuration fields would crash the app due to how recommendations were being calculated.
5 changes: 5 additions & 0 deletions .changeset/proud-dancers-train.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'renterd': patch
---

The configuration recommendations now instruct the user to first fill all fields. Closes https://github.com/SiaFoundation/renterd/issues/1214
81 changes: 49 additions & 32 deletions apps/renterd/components/Config/Recommendations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
CaretDown16,
Information20,
CheckmarkFilled20,
PendingFilled20,
} from '@siafoundation/react-icons'
import { useApp } from '../../contexts/app'
import { routes } from '../../config/routes'
Expand All @@ -33,6 +34,7 @@ export function Recommendations() {
const {
hostMargin50,
hostTarget50,
hasDataToEvaluate,
needsRecommendations,
foundRecommendation,
recommendations,
Expand Down Expand Up @@ -113,6 +115,27 @@ export function Recommendations() {
</div>
)

if (!hasDataToEvaluate) {
return (
<Layout
maximized={maximized}
setMaximized={setMaximized}
maximizeControls={false}
title={
<>
<Text color="contrast">
<PendingFilled20 />
</Text>
<Text size="16" weight="medium">
The system will review your configuration once all fields are
filled
</Text>
</>
}
/>
)
}

if (!needsRecommendations) {
return (
<Layout
Expand Down Expand Up @@ -239,43 +262,37 @@ function Layout({
setMaximized: (maximized: boolean) => void
maximizeControls: boolean
title: React.ReactNode
tip: React.ReactNode
tip?: React.ReactNode
}) {
const el = (
<div
className={cx(
'flex justify-between items-center px-3 py-1.5',
maximized && children
? 'border-b border-gray-200 dark:border-graydark-300'
: '',
maximizeControls ? 'cursor-pointer' : ''
)}
onClick={() => {
if (maximizeControls) {
setMaximized(!maximized)
}
}}
>
<div className={cx('flex gap-2 items-center')}>{title}</div>
{maximizeControls && (
<Button variant="ghost" onClick={() => setMaximized(!maximized)}>
{maximized ? <Subtract24 /> : <Add24 />}
</Button>
)}
</div>
)
return (
<div className="relative">
<div className="z-20 absolute top-0 left-1/2 -translate-x-1/2 flex justify-center">
<div className="z-10 absolute top-0 left-1/2 -translate-x-1/2 flex justify-center">
<div className="w-[600px] flex flex-col max-h-[600px] bg-gray-50 dark:bg-graydark-50 border-b border-x border-gray-300 dark:border-graydark-400 rounded-b">
<ScrollArea>
<HoverCard
trigger={
<div
className={cx(
'flex justify-between items-center px-3 py-1.5',
maximized && children
? 'border-b border-gray-200 dark:border-graydark-300'
: '',
maximizeControls ? 'cursor-pointer' : ''
)}
onClick={() => {
if (maximizeControls) {
setMaximized(!maximized)
}
}}
>
<div className={cx('flex gap-2 items-center')}>{title}</div>
{maximizeControls && (
<Button
variant="ghost"
onClick={() => setMaximized(!maximized)}
>
{maximized ? <Subtract24 /> : <Add24 />}
</Button>
)}
</div>
}
>
{tip}
</HoverCard>
{tip ? <HoverCard trigger={el}>{tip}</HoverCard> : el}
{children}
</ScrollArea>
</div>
Expand Down
79 changes: 71 additions & 8 deletions apps/renterd/contexts/config/useAutopilotEvaluations.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,31 @@ export function useAutopilotEvaluations({
}) {
const values = form.watch()
const renterdState = useBusState()
const payloads = useMemo(() => {

const hasDataToEvaluate = useMemo(() => {
if (!checkIfAllResourcesLoaded(resources)) {
return
return false
}
if (!form.formState.isValid) {
return
return false
}
if (!renterdState.data) {
return false
}
return true
}, [form, resources, renterdState])

// We need to pass valid settings data into transformUp to get the payloads.
// The form can be invalid so we need to merge in valid data to make sure
// numbers are not undefined in the transformUp calculations that assume
// all data is valid and present.
const currentValuesWithZeroDefaults = useMemo(
() => mergeIfDefined(valuesZeroDefaults, values),
[values]
)

const payloads = useMemo(() => {
if (!hasDataToEvaluate) {
return
}

Expand All @@ -46,17 +63,17 @@ export function useAutopilotEvaluations({
isAutopilotEnabled,
showAdvanced,
estimatedSpendingPerMonth,
values,
values: currentValuesWithZeroDefaults,
})
return payloads
}, [
form,
values,
currentValuesWithZeroDefaults,
resources,
renterdState,
isAutopilotEnabled,
showAdvanced,
estimatedSpendingPerMonth,
hasDataToEvaluate,
])

const userContractCountTarget = payloads?.autopilot.contracts.amount || 0
Expand Down Expand Up @@ -187,15 +204,20 @@ export function useAutopilotEvaluations({
const rec = getRecommendationItem({
key: remoteToLocalFields[key],
recommendationDown,
values,
values: currentValuesWithZeroDefaults,
})
if (rec) {
recs.push(rec)
}
}
})
return recs
}, [recommendedGougingSettings, resources, payloads, values])
}, [
recommendedGougingSettings,
resources,
payloads,
currentValuesWithZeroDefaults,
])
const foundRecommendation = !!recommendations.length

return {
Expand All @@ -206,6 +228,7 @@ export function useAutopilotEvaluations({
usableHostsCurrent,
userContractCountTarget,
usableHostsAfterRecommendation,
hasDataToEvaluate,
needsRecommendations,
foundRecommendation,
recommendations,
Expand Down Expand Up @@ -283,3 +306,43 @@ const remoteToLocalFields: Record<keyof GougingSettings, keyof Fields> = {
minMaxEphemeralAccountBalance: 'minMaxEphemeralAccountBalance',
migrationSurchargeMultiplier: 'migrationSurchargeMultiplier',
}

export const valuesZeroDefaults: SettingsData = {
autopilotContractSet: '',
amountHosts: new BigNumber(0),
allowanceMonth: new BigNumber(0),
periodWeeks: new BigNumber(0),
renewWindowWeeks: new BigNumber(0),
downloadTBMonth: new BigNumber(0),
uploadTBMonth: new BigNumber(0),
storageTB: new BigNumber(0),
prune: false,
allowRedundantIPs: false,
maxDowntimeHours: new BigNumber(0),
minRecentScanFailures: new BigNumber(0),
minProtocolVersion: '',
defaultContractSet: '',
uploadPackingEnabled: true,
maxRpcPriceMillion: new BigNumber(0),
maxStoragePriceTBMonth: new BigNumber(0),
maxContractPrice: new BigNumber(0),
maxDownloadPriceTB: new BigNumber(0),
maxUploadPriceTB: new BigNumber(0),
hostBlockHeightLeeway: new BigNumber(0),
minPriceTableValidityMinutes: new BigNumber(0),
minAccountExpiryDays: new BigNumber(0),
minMaxEphemeralAccountBalance: new BigNumber(0),
migrationSurchargeMultiplier: new BigNumber(0),
minShards: new BigNumber(0),
totalShards: new BigNumber(0),
}

export function mergeIfDefined(defaults: SettingsData, values: SettingsData) {
const merged: SettingsData = { ...defaults }
Object.keys(values).forEach((key) => {
if (values[key] !== undefined) {
merged[key] = values[key]
}
})
return merged
}
9 changes: 8 additions & 1 deletion apps/renterd/contexts/config/useForm.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useMemo } from 'react'
import { useEffect, useMemo } from 'react'
import { defaultValues, getAdvancedDefaults } from './types'
import { getRedundancyMultiplier } from './utils'
import { useForm as useHookForm } from 'react-hook-form'
Expand Down Expand Up @@ -46,6 +46,13 @@ export function useForm({ resources }: { resources: Resources }) {
}
)

// Trigger input validation on showAdvanced change because many field validation
// objects switch from required to not required.
useEffect(() => {
form.trigger()
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [showAdvanced])

const estimates = useEstimates({
isAutopilotEnabled,
redundancyMultiplier,
Expand Down
Loading