-
Notifications
You must be signed in to change notification settings - Fork 1
Grant logs permission on a machine and restart #391
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
base: master
Are you sure you want to change the base?
Conversation
Deployed to Cloudflare Pages
|
| useEffect(() => { | ||
| if (open) { | ||
| form.reset({ | ||
| evmAddress: '0x', | ||
| }) | ||
| } | ||
| }, [open, form]) | ||
|
|
||
| const handleOpenChange = (newOpen: boolean) => { | ||
| setOpen(newOpen) | ||
| if (!newOpen) { | ||
| form.reset() | ||
| } | ||
| } |
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.
Isn't this logic duplicated?
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.
ah yeah. and maybe we dont even need to clear - especially if user cancels and reopens
| import { FileText } from 'lucide-react' | ||
|
|
||
| const formSchema = z.object({ | ||
| evmAddress: z.string().regex(/^0x[a-fA-F0-9]{40}$/, { |
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.
Maybe checking the checksum of the address would be nice.
import { getAddress } from 'viem'
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.
+1
| <DialogTrigger asChild> | ||
| <Button variant="outline" className="w-full md:w-auto" disabled={disabled}> | ||
| <FileText /> | ||
| Grant logs permission |
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 a bit misleading, after I granted logs permissions, I am suddenly able to restart/top-up the machine on the granted account - I would not expect this to happen. I am also able to modify the secrets of the app, even though it fails - here it would be nice that UI would be hidden, for actions I can not complete.
And then recursively I am able to grant logs permissions, on the "log permissioned account", which means, that that account can add another account. But on that account I am only able to top up and restart the machine. And not able to actually see the logs.
Not sure what about this is expected or not.
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.
Not sure. Should discuss on slack
Part of #367
Closes #370
Limitations: