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

including interface for nip07 #403

Merged
merged 5 commits into from
May 26, 2024

Conversation

antonioconselheiro
Copy link
Contributor

This pull request includes:

  • typescript interface for nip07;
  • centralized nip46 and nip07 relay map into a named type and export it;
  • include docs of how to include the nip07 type for browser environment;

I haven't finished testing it yet and it's not ready, but I opened the pull request to advance the code review

@alexgleason
Copy link
Collaborator

Please have it match this interface: https://gitlab.com/soapbox-pub/nostrify/-/blob/main/interfaces/NostrSigner.ts?ref_type=heads

Also, take a look at: https://nostrify.dev/sign/

@antonioconselheiro
Copy link
Contributor Author

antonioconselheiro commented May 21, 2024

Interface for nip07 included, then I compiled and imported the library into my project, it looks good.

image

Check if the interface looks good for you.

@fiatjaf
Copy link
Collaborator

fiatjaf commented May 25, 2024

Sorry, I missed this in the pile of GitHub notifications before. Looks good, can we merge @alexgleason?

nip07.ts Outdated

export interface Nip07 {
getPublicKey(): Promise<string>
signEvent(event: EventTemplate): Promise<VerifiedEvent>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
signEvent(event: EventTemplate): Promise<VerifiedEvent>
signEvent(event: EventTemplate): Promise<Event>

An extension won't be able to add our special symbol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is something that I have not understood, can you explain better this suggestion for me?
VerifiedEvent is exported and I use it and other special symbols like NostrEvent and EventTemplate in my apps, and I think that the extension will implement this interface using JavaScript and should return in the Promise a corresponding event with VerifiedEvent interface

Copy link
Collaborator

Choose a reason for hiding this comment

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

Look at the definition of VerifiedEvent. It has [verifiedSymbol]: true, which is something only nostr-tools can add to the object. An external extension can't do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, I changed it to NostrEvent

…vent returned by the signer
@fiatjaf fiatjaf merged commit 88454de into nbd-wtf:master May 26, 2024
1 of 2 checks passed
@antonioconselheiro antonioconselheiro deleted the feature/nip7-interface branch May 27, 2024 11:09
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.

None yet

3 participants