Add smooth column in validatorlist #284
Conversation
|
|
||
| export const MAINNET_ORACLE_URL = "https://sp-api.dappnode.io" as const; | ||
|
|
||
| export const TESTNET_ORACLE_URL = "http://65.109.102.216:7300" as const; |
There was a problem hiding this comment.
I'd like to have a domain for the testnet oracle, it is a bit weird to have the IP and port hardcoded here, but since we don't have it it's ok...
There was a problem hiding this comment.
i can ask for a goerli domain, but for now we should either keep it like this or remove prater oracle integration
There was a problem hiding this comment.
Keep it, but let's update it in the future if possible
| const fetchValidatorsData = async (rows: CustomValidatorGetResponse[]) => { | ||
| const newValidatorSubscriptionStatus: SmoothStatusByPubkey = {}; | ||
| // Filter rows to include only those with an index | ||
| const rowsWithIndex = rows.filter(row => row.index); |
There was a problem hiding this comment.
Do you set a specific icon for those validators that have not received a deposit yet? It could be great to have this information given to the user
There was a problem hiding this comment.
Tooltip like: "No index found for this validator. Are your clients synced? Have you done a deposit? "
There was a problem hiding this comment.
If 0 index are found --> consensus client probably offline. Oracle call returns error, so a red "?" is rendered with error message when hovering.
https://github.com/dappnode/StakingBrain/blob/marc/smooth-status-integration/packages/ui/src/components/ValidatorList/SmoothStatus.tsx#L65
If only some index could not be found --> their subscriptionStatus is not fetched (does not exist) so they are stuck in "Loading..." I could improve this but its not that bad neither https://github.com/dappnode/StakingBrain/blob/marc/smooth-status-integration/packages/ui/src/components/ValidatorList/SmoothStatus.tsx#L74
|
|
||
| // Healthy checks: check that the response is valid and that the response is ok | ||
| if (!isValidOracleResponse(data)) { | ||
| throw new Error( |
There was a problem hiding this comment.
Maybe you could define a specific error for this? Something like OracleApiError? (just a suggestion)
There was a problem hiding this comment.
I dont think its worth it for now
|
|
||
| // find the validator in the batch and mark it as not subscribed | ||
| const validator = batch.find( | ||
| (validator) => validator.index === notFoundValiToString |
There was a problem hiding this comment.
Maybe you could avoid creating a variable here, you do not use anywhere else (notFoundValidator.toString()). It's also ok to create it if the code looks cleaner/simpler
There was a problem hiding this comment.
reafactored this part
|
|
||
| // Only proceed if there is at least one row with an index | ||
| // new validatos will not have an index yet | ||
| if (rowsWithIndex.length > 0) { |
There was a problem hiding this comment.
You could remove this and include inside the try-catch:
if (rowsWithIndex.length < 1 ) throw ... (error message)
I suggest this because you already handle setting the oracle error on the catch. This way you could avoid this duplication:
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| renderCell: (rowData: { row: any; }) => { | ||
| // only render smooth status if tag is "solo" | ||
| if (rowData.row.tag === "solo") { |
There was a problem hiding this comment.
Needs to be checked, but probably you can set Smooth for any tag that has an editable fee recipient, not only solo
There was a problem hiding this comment.
for now, lets keep oracle status only to "solo" tag validators
| /> | ||
| ); | ||
| } else { | ||
| return <span>-</span>; |
There was a problem hiding this comment.
You could try to find an emoji like 🚫 or something similar that lets the user know it is not available for that protocol
There was a problem hiding this comment.
I have this emoji as "banned" status. I want the column to not have noise when validator is not "solo", so went for the "-"
|
|
||
| const feeRecipient = rowData.row.feeRecipient; | ||
| const withdrawalFormat = rowData.row.withdrawalCredentials.format; | ||
| const mevSpAddress = mevSpFeeRecipient |
| } | ||
| // subscriptionStatus is null when oracleCall hasnt finished yet | ||
| if (!subscriptionStatus) { | ||
| return <span>Loading...</span>; |
There was a problem hiding this comment.
I would return a loading spinner
There was a problem hiding this comment.
done! looks way better
|
|
||
| // unhealthy subscription. Wrong fee recipient | ||
| } else if ( | ||
| (subscriptionStatus.toLowerCase() === "active" || |
There was a problem hiding this comment.
You might need to add extra parenthesis here:
(cond1 OR cond2 OR cond3) AND cond4
There was a problem hiding this comment.
there already is right?
else if (
(subscriptionStatus.toLowerCase() === "active" ||
subscriptionStatus.toLowerCase() === "yellowcard" ||
subscriptionStatus.toLowerCase() === "redcard") &&
rowData.row.feeRecipient !== mevSpAddress
)
an extra parentesis encapsulates all 3 "healthy" possible oracle states. If a validator is in any of these and has bad fee recipient, he is in danger of being banned
| @@ -61,6 +69,106 @@ export default function KeystoresDataGrid({ | |||
| openDashboardTab(); | |||
| }, [validatorSummaryURL]); | |||
|
|
|||
| useState<CustomValidatorUpdateRequest>(); | |||
| const [validatorsSubscriptionMap, setValidatorsData] = | |||
There was a problem hiding this comment.
Following best practices, the useState should have the same naming in the const and the setter: const [validatorsData, setValidatorsData]
| @@ -61,6 +69,106 @@ export default function KeystoresDataGrid({ | |||
| openDashboardTab(); | |||
| }, [validatorSummaryURL]); | |||
|
|
|||
| useState<CustomValidatorUpdateRequest>(); | |||
| const [validatorsSubscriptionMap, setValidatorsData] = | |||
| useState<SmoothStatusByPubkey>({}); | |||
There was a problem hiding this comment.
I would consider as "dangerous" setting a initial value an empty object, it might lead to future issues since it will be considered as a non-nullish value
There was a problem hiding this comment.
you're right. changed it to be initialized as null, not in danger of being considered non-nullish anymore.
|
|
||
| // Check that Smooth API returns an expected response format | ||
| function isValidOracleResponse(response: SmoothValidatorByIndexApiResponse): boolean { | ||
| return ( |
There was a problem hiding this comment.
I might be out of context, why are we expecting those values as null? could you add a comment pls?
There was a problem hiding this comment.
We expect them to either exist or to be null.
The oracle call recieves the validator's indeces loaded in brain and returns two arrays, found_validators and not_found_validators. A validator that never interacted with oracle will be inside not_found_validator. A validator that at some point has already interacted with the oracle will be inside found_validators
For example, if a user loads web3signer brain for the first time and has never subscribed to smooth, we expect found_validators to be null, since its validators have not interactuated with the oracle yet and it is not tracking them.
| const validator = batch.find( | ||
| (validator) => validator.index === notFoundValiToString | ||
| ); | ||
| if (validator) { |
There was a problem hiding this comment.
should be taken any action if validator not found?
There was a problem hiding this comment.
i have refactored this part, all "not found" validators will have the unsubscribed status now. It wasnt clear before
|
|
||
| // Call the Oracle API to get the subscription status of the validators in the batch by their index | ||
| const response = await fetch( | ||
| `${apiUrl}/memory/validatorsbyindex/${batch |
There was a problem hiding this comment.
Are you querying all the validators in the smooth? not see the index arg or I might need to have a look to the smooth api to fully understand
There was a problem hiding this comment.
im querying only the validators loaded in brain with index available (notice that new validators have to wait 16-24h to be indexed by the chain)
const oracleApiResponse = await fetch(
`${apiUrl}/memory/validatorsbyindex/${batch
.map((row) => row.index)
.join(",")}`
);
| } | ||
| } else { | ||
| setOracleCallError( | ||
| "Skipping Oracle subscription status fetch, no validator index could be fetch. Is your consensus client synced?" |
There was a problem hiding this comment.
It might be implemented in a separated PR, but, can we know in advanced if is synced or not? this way we would display useful data instead of an error with a "maybe"
There was a problem hiding this comment.
True! Didnt, think about it, oracle has an endpoint that tells you if it is synced. I have implemented it in code now, just before trying to call subscription status (if not sync, subscription status call is skipped)
| @@ -74,7 +182,7 @@ export default function KeystoresDataGrid({ | |||
| field: "feeRecipient", | |||
| headerName: "Fee Recipient", | |||
| description: | |||
| "Fee Recipient is a feature that lets you specify a priority fee recipient address on your validator client instance and beacon node", | |||
| "Address to which the rewards generated from proposing a block are sent", | |||
There was a problem hiding this comment.
Much better description!
This PR Adds a new "Smooth" coulmn in
KeystoresDataGridcomponent.This new column is only rendered in networks where Smooth exists (mainnet or prater). It shows the Smooth subscription status of each validator (only if their
tag == "solo").The rendering logic is done in a new file
SmoothStatus.tsxhttps://github.com/dappnode/StakingBrain/blob/marc/smooth-status-integration/packages/ui/src/components/ValidatorList/SmoothStatus.tsx#L50The oracle call is a method inside
KeystoresDataGrid.tsx: https://github.com/dappnode/StakingBrain/blob/marc/smooth-status-integration/packages/ui/src/components/ValidatorList/KeystoresDataGrid.tsx#L91