Integrate Smooth Fee recipient changes #283
Conversation
mateumiralles
commented
Jan 9, 2024
mateumiralles
commented
- Adding stepper on editing FR from selected validators when "Set Dappnode MEV Smoothing Pool Fee Recipient" is checked in ValidatorsList
…lidators is different from SP Address
…om selected validators
…at different to ETH1
| @@ -23,3 +23,23 @@ export interface TagSelectOption { | |||
| value: Tag; | |||
| label: string; | |||
| } | |||
|
|
|||
| export const alertTypes = [ | |||
There was a problem hiding this comment.
are all of these alerts related to Smooth? Maybe we could rename this to smoothAlertTypes
There was a problem hiding this comment.
Not all of them, (successAlert, errorAlert, onlyEditableFeesAlert, feeAlreadySetToAllAlert) are related to the process of changing the FR. Those can appear without checking the join Smooth switch. Would be better to split it in two? (generalAlertTypes and smoothAlertTypes)
| const handleSubscriptionClick = async () => { | ||
| try { | ||
| await updateValidators(); | ||
| window.open("https://smooth.dappnode.io/", "_blank"); |
There was a problem hiding this comment.
it should open https://smooth-goerli.dappnode.io/ if in testnet (prater). These hardcoded URLs could be inside params.ts
| }; | ||
|
|
||
| const handleUnsubscriptionClick = async () => { | ||
| window.open("https://smooth.dappnode.io/", "_blank"); |
There was a problem hiding this comment.
it should open https://smooth-goerli.dappnode.io/ if in testnet (prater). These hardcoded URLs could be inside params.ts
| const [errorMessage, setErrorMessage] = useState(""); | ||
| const [successMessage, setSuccessMessage] = useState(""); | ||
| const [loading, setLoading] = useState(false); | ||
| const [isMevSpAddressSelected, setIsMevSpAddressSelected] = useState(false); | ||
| const [activeStep, setActiveStep] = React.useState(0); | ||
| const [isUnsubUndestood, setIsUnsubUndestood] = useState(false); |
There was a problem hiding this comment.
Understood misspelled here
| const [isMevSpAddressSelected, setIsMevSpAddressSelected] = useState(false); | ||
| const [activeStep, setActiveStep] = React.useState(0); | ||
| const [isUnsubUndestood, setIsUnsubUndestood] = useState(false); | ||
| const [NonEcdsaValidatorsData, setNonEcdsaValidatorsData] = useState< |
There was a problem hiding this comment.
A variable should always start with non-capital letters
| .map((rowId) => rows[parseInt(rowId.toString())].feeRecipient) | ||
| .flat(); | ||
|
|
||
| console.log(oldFeeRecipients); |
There was a problem hiding this comment.
These logs should not be included in the prod code
| let isAnyGiven = false; | ||
| for (const row of selectedRows) { | ||
| const withdrawalFormat = | ||
| rows[parseInt(row.toString())].withdrawalCredentials.format; |
| ); | ||
| } | ||
|
|
||
| const isAnyFormatValidatorSelected = ({ |
There was a problem hiding this comment.
I would say this function needs to be much clearer
| return isAnyGiven; | ||
| }; | ||
|
|
||
| const getSmoothValidatorsSelected = () => { |
There was a problem hiding this comment.
You could do something like:
const getSmoothValidatorsSelected = (): void => {
const smoothValidatorsPubkeys = selectedRows
.map(row => rows[+row].feeRecipient === mevSpAddress ? rows[+row].pubkey : null)
.filter(pubkey => pubkey !== null);
setSmoothValidatorsPubkeys(smoothValidatorsPubkeys);
};
(Try it first)
| setSmoothValidatorsPubkeys(auxList); | ||
| }; | ||
|
|
||
| const nonEcdsaValidatorsData = () => { |
There was a problem hiding this comment.
Same as above, you can try:
const nonEcdsaValidatorsData = (): void => {
const filteredValidators = selectedRows
.map(row => {
const rowData = rows[+row];
return rowData.withdrawalCredentials.format !== "ecdsa"
? { pubkey: rowData.pubkey, withdrawalFormat: rowData.withdrawalCredentials.format }
: null;
})
.filter(item => item !== null);
setNonEcdsaValidatorsData(filteredValidators);
};
You can make any changes, it is just a suggestion
|
|
||
| function SubscriptionCard(): JSX.Element { | ||
| return ( | ||
| // TODO: Set proper link to the Dappnode Smoothing Pool |
There was a problem hiding this comment.
This is already done right?
packages/ui/src/utils/addresses.ts
Outdated
| @@ -9,6 +10,12 @@ export const getSmoothAddressByNetwork = (network: Network) => { | |||
| return MEV_SP_ADDRESS_PRATER; | |||
| } else if (network == "mainnet") { | |||
| return MEV_SP_ADDRESS_MAINNET; | |||
| } else if (network == "holesky") { | |||
There was a problem hiding this comment.
for all networks different than mainnet or prater, this should return "null". its easier if you do
export const getSmoothAddressByNetwork = (network: Network) => {
if (network == "prater") {
return MEV_SP_ADDRESS_PRATER;
} else if (network == "mainnet") {
return MEV_SP_ADDRESS_MAINNET;
} else {
return null;
}
}
since we treat smooth address as null when outside prater or mainnet, there is no need to return error in this method anymore
