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

custody: fill in scope of CustodyService #3331

Merged
merged 1 commit into from
Nov 14, 2023
Merged

custody: fill in scope of CustodyService #3331

merged 1 commit into from
Nov 14, 2023

Conversation

hdevalence
Copy link
Member

This fills in missing methods that allow the CustodyService to be the interface to a device like a hardware wallet, or to allow websites to request confirmation for displayed addresses.

Result of pairing with @redshiftzero and @cronokirby .

This fills in missing methods that allow the CustodyService to be the interface
to a device like a hardware wallet, or to allow websites to request
confirmation for displayed addresses.

Co-Authored-By: Lucas Meier <lucas@cronokirby.com>
Co-Authored-By: Jen Helsby <jen@penumbralabs.xyz>
Comment on lines +23 to +27
// Requests the full viewing key from the custodian.
//
// Custody backends should decide whether to honor this request, and how to
// control access to it.
rpc ExportFullViewingKey(ExportFullViewingKeyRequest) returns (ExportFullViewingKeyResponse);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grod220 The web extension should implement this method by erroring out.

Comment on lines +29 to +35
// Displays an address to a user for confirmation.
//
// Custody backends with user interaction should present the address to the
// user and wait for explicit confirmation before returning.
//
// Non-interactive custody backends may return immediately.
rpc ConfirmAddress(ConfirmAddressRequest) returns (ConfirmAddressResponse);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grod220 The web extension should implement this method by creating an extension popup. The point of this is to allow web content to steer users to copy an address they'll send to someone else from trusted UI. Realistically, though, we won't be able to ensure that users don't ever copy an address from the web content, but we can give an option for trusted display.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will trigger a popup for a user to copy any of their addresses? Sort of like this?

Screenshot 2023-11-15 at 10 11 32 AM

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the idea is to provide a way to confirm an address on a trusted display path controlled by the custodian. For a Ledger this is viewing the address on screen, for the web extension this would be some kind of popup trigger. I don't think it's a super high priority in the web extension but something that would be nice to have eventually

@hdevalence hdevalence merged commit 2a9859a into main Nov 14, 2023
8 checks passed
@hdevalence hdevalence deleted the fill-in-custody branch November 14, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants