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

Add nip 98 support for nostr.build #1471

Closed

Conversation

fishcakeday
Copy link
Contributor

This change implements support for NIP-98 and uses it for uploads to nostr.build. It is also changing the nostr.build API end-point to V2. Implements #823

@fishcakeday fishcakeday force-pushed the add-nip-98-support-nostr-build branch from f79e14f to 0782cfb Compare August 12, 2023 12:17
@fishcakeday
Copy link
Contributor Author

I have no idea why the test failed, given that I am using the identical way of creating event as any other function, e.g., "let ev = NostrEvent(content: content, pubkey: pubkey, kind: 7, tags: tags)"

@fishcakeday
Copy link
Contributor Author

Ok, submitted a potential fix for the test, but I am not sure if that will help.

@fishcakeday
Copy link
Contributor Author

It seems that test expects me to use NostrEventOld which is not even available in master. Please advise.

@jb55
Copy link
Collaborator

jb55 commented Aug 18, 2023 via email

@fishcakeday fishcakeday force-pushed the add-nip-98-support-nostr-build branch from 0782cfb to 505d4f0 Compare August 18, 2023 22:59
@fishcakeday
Copy link
Contributor Author

@jb55 I fixed the issue, and it fixed the tests too. It seems that my local clone was broken and that's why the original patch was not aligned with the latest changes. Please let me know if I missed anything else. Thanks.

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@fishcakeday
Copy link
Contributor Author

let res = await image_upload.start(media: media, uploader: uploader, keypair: damus_state?.keypair)

Since keypair is not optional in damus_state, I cannot use 'damus_state?.keypair', otherwise I'll get: Cannot use optional chaining on non-optional value of type 'DamusState'

@fishcakeday fishcakeday force-pushed the add-nip-98-support-nostr-build branch from 505d4f0 to 9641330 Compare August 20, 2023 01:10
@fishcakeday
Copy link
Contributor Author

Patches 3,4,5,6 can all be squashed together. Since patch 3 updates the function, you would need to update all the callsites in the same patch. Looking good, getting there. Thanks! Will

All comments are addressed, and I managed to reduce change by one less file with optional parameter. Please take a look and let me know if anything else is missing. I've tested it, too, so it works on my iPhone. Thank you!

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55
Copy link
Collaborator

jb55 commented Aug 20, 2023 via email

@jb55 jb55 closed this in 1432087 Aug 20, 2023
jb55 pushed a commit that referenced this pull request Aug 20, 2023
jb55 pushed a commit that referenced this pull request Aug 20, 2023
suhailsaqan pushed a commit to suhailsaqan/damus that referenced this pull request Sep 19, 2023
Closes: damus-io#1471
Signed-off-by: William Casarin <[email protected]>
suhailsaqan pushed a commit to suhailsaqan/damus that referenced this pull request Sep 19, 2023
suhailsaqan pushed a commit to suhailsaqan/damus that referenced this pull request Sep 19, 2023
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