-
Notifications
You must be signed in to change notification settings - Fork 27
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
Disable delegation for validators, groups and their signers #135
base: main
Are you sure you want to change the base?
Disable delegation for validators, groups and their signers #135
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -43,7 +43,9 @@ export const wagmiConfig = createConfig({ | |||
chains: [config.chain], | |||
connectors, | |||
transports: { | |||
[celo.id]: fallback([http(config.chain.rpcUrls.default.http[0]), http(infuraRpcUrl)]), | |||
[celo.id]: fallback([http(config.chain.rpcUrls.default.http[0]), http(infuraRpcUrl)], { | |||
rank: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is to pick the currently best provider as per documentation
src/features/account/hooks.ts
Outdated
isAccountResult.isLoading || isValidatorResult.isLoading || isValidatorGroupResult.isLoading, | ||
refetch: () => | ||
Promise.all([ | ||
isAccountResult.refetch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont these need to be called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's correct, they need to be recalled and probably forward the options
refetch: (options) =>
Promise.all([
isAccountResult.refetch(options),
onClick={() => showTxModal()} | ||
>{`️🗳️ Delegate voting power`}</SolidButton> | ||
{(isValidator || isValidatorGroup) && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI guy hat on:
We have another red text that can also occur. but that one is a warning whereas this one is a hard NO. It is also theoretically possible for both to show and take up a lot of space.
IDEA:
dont disable here. rather allow delegate modal to open and then display then disable the button in the modal and dislay this explanation in the white blank space above button.
address: Addresses.Validators, | ||
abi: validatorsABI, | ||
functionName: 'isValidator', | ||
args: [addressOrVoteSigner || ZERO_ADDRESS], | ||
query: { enabled: !!addressOrVoteSigner }, | ||
}); | ||
|
||
const isValidatorGroupResult = useReadContract({ | ||
const isValidatorGroupOrVoteSignerResult = useReadContract({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: slightly prefer the old name. voteSigners in general CAN Delegate just not those who sign for validators/groups.
but i could have a locked gold contract that and add a signer that can vote o proposals.
This PR disables option to delegate when connected account is either validator or a group (or a vote signer for any of the two). Otherwise allowing to call the contract anyway was resulting in an error and bad experience.
Now - if delegation is not possible - the delegation button will be disabled and the user will be informed about this as per the screenshot:
as opposed to state where delegation is possible:
Fixes #110