-
Notifications
You must be signed in to change notification settings - Fork 598
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
Add accountsChanged event for NIP-07 #1552
Comments
Yes please... Using multiple accounts with extensions sucks. |
This is probably necessary, but it will also break clients. Extensions should have some way to detect this feature, maybe by disabling it in the UI if the client doesn't add a handler. |
Kinda need this too. |
Doesn't Alby already emit some kind of event when the user switches accounts? @bumi |
@alexgleason I think Alby doesn't have multi accounts yet, try Nostrame: https://chromewebstore.google.com/detail/nostrame/phfdiknibomfgpefcicfckkklimoniej |
I did something like that: https://gist.github.com/Anderson-Juhasc/e78e1ff349eb4299328038eedd3fe3b2 |
I've put together a little demo of how it's currently implemented in Alby: https://codepen.io/reneaaron/pen/RwXMBZO?editors=1111 It's basically offering these events through an window.nostr.on("accountChanged", async (e) => {
console.log("⚡️ window.nostr.on('accountChanged') fired!");
const pubkey = await window.nostr.getPublicKey();
console.log("🔑 new public key", pubkey);
});
Alby does have multi-account since basically ever. We really need to work on the UX of that 😅 |
I thought so. I remembered seeing it in the Alby code while digging around. But I think extending How about this? window.addEventListener("nostr.change", callback); eg: window.addEventListener("nostr.change", async (e) => {
console.log("⚡️ nostr.change fired!");
const pubkey = await window.nostr.getPublicKey();
console.log("🔑 new public key", pubkey);
}); Reasons:
Event object: As for the new CustomEvent('nostr.change', { detail: ['pubkey'] }); // pubkey changed
new CustomEvent('nostr.change', { detail: ['relays'] }); // relay list changed
new CustomEvent('nostr.change', { detail: ['pubkey', 'relays'] }); // pubkey & relay list changed I think this would help with adoption of the feature. |
For reference: Ultimately I think it's a personal preference thing: Each implementation ( |
@neilck the suggestion of @alexgleason LGTM. |
@1l0 I implemented @alexgleason 's point of making it easier for clients to integrate is good (window.addEventLister vs windows.nostr.on), as there are more clients than extensions, and we want wider adoption of this feature. I think a better solution for clients is not having to listen and react to an account changed notification, but rather to modify NIP-07 function signEvent to accept an optional pubkey parameter. If the pubkey exists in the extension and the extension has the user's permission to signing events for that client with that pubkey, then it doesn't matter what the current pubkey is returned by getPublicKey. |
I think all the options have some pros and cons and we can find arguments for each of them. Imo it's ultimately boils down to personal taste. I personally don't see how such a different syntax is making it easier for devs. In the current implementation I like that it does not leak anything outside of the nip07 object. The same API works also when no |
Not only This should be done regardless of how the main account's onChange event is defined. |
@neilck That's a beautiful solution. In that case I believe clients want GetPublicKeys() instead of GetPublicKey(). |
@1l0 @vitorpamplona Yes, I think its better to have the client determine the account / accounts, and make the signer extension responsible for signing and permissions. I wouldn't return all public keys from getPublicKeys() since it's leaky. I'd prefer to have getSharedPublicKeys(). My permission model is based off nos2x, where the extension remembers permissions per domain. getSharedPublicKeys() would only return public keys that the user has given the client permission to access. |
I prefer the web standard style to the node.js style. Because it is based on event bubbing, it allows for open-and-wide ecosystem use (in browser-side javascript though).
|
I've updated akaprofiles extension based on this conversation. Anyone have a client willing to integrate these changes?
It didn't make sense to add a new signEventWithPubkey function, since a client can specify the pubkey they want right in the event. I also created a Next.js NIP-07 tester that can test any signing extension by calling it like a client (including the current version of onAccountChanged). |
I think many clients that use NIP-07 will need something to watch when the user change of account, something like occurs on metamask.
Usage:
The text was updated successfully, but these errors were encountered: