Nostr Web Client

Lez 1y ago

From 3fc1a99b85fb20f78b1ea28e5daa75eef4688da5 Mon Sep 17 00:00:00 2001

From: Laszlo Megyer

Date: Mon, 2 Dec 2024 13:20:13 +0100

Subject: [PATCH 1/3] Parse NIP-05 address from a git remote URL

The nip05 address is stored as it is, because we can't run async code in

from_str().

We need to add an async resolve_nip05() to NostrUrlDecoded.

---

src/lib/git/nostr_url.rs | 27 +++++++++++++++++++--------

src/lib/repo_ref.rs | 1 +

2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/lib/git/nostr_url.rs b/src/lib/git/nostr_url.rs

index a501765..be4a825 100644

--- a/src/lib/git/nostr_url.rs

+++ b/src/lib/git/nostr_url.rs

@@ -59,6 +59,7 @@ pub struct NostrUrlDecoded {

pub coordinates: HashSet,

pub protocol: Option,

pub user: Option,

+ pub nip05: Option,

}

impl fmt::Display for NostrUrlDecoded {

@@ -98,6 +99,7 @@ impl std::str::FromStr for NostrUrlDecoded {

let mut protocol = None;

let mut user = None;

let mut relays = vec![];

+ let mut nip05 = None;

if !url.starts_with("nostr://") {

bail!("nostr git url must start with nostr://");

@@ -134,16 +136,24 @@ impl std::str::FromStr for NostrUrlDecoded {

.split('/')

.collect();

+ // split first part into user if present and hostname

+ let part = parts.first().context(INCORRECT_NOSTR_URL_FORMAT_ERROR)?;

+ let hostname = if let Some(at_index) = part.find('@') {

+ // TODO throw error if user already set

+ user = Some(part[..at_index].to_string());

+ &part[at_index + 1..]

+ } else {

+ part

+ };

+ // check if it uses a NIP-05 address

+ if hostname.contains('.') {

+ nip05 = Some(part.to_string())

+ }

// extract optional protocol

if protocol.is_none() {

- let part = parts.first().context(INCORRECT_NOSTR_URL_FORMAT_ERROR)?;

- let protocol_str = if let Some(at_index) = part.find('@') {

- user = Some(part[..at_index].to_string());

- &part[at_index + 1..]

- } else {

- part

- };

- protocol = match protocol_str {

+ protocol = match hostname {

"ssh" => Some(ServerProtocol::Ssh),

"https" => Some(ServerProtocol::Https),

"http" => Some(ServerProtocol::Http),

@@ -196,6 +206,7 @@ impl std::str::FromStr for NostrUrlDecoded {

coordinates,

protocol,

user,

+ nip05,

})

}

diff --git a/src/lib/repo_ref.rs b/src/lib/repo_ref.rs

index 69fbe64..877d1ee 100644

--- a/src/lib/repo_ref.rs

+++ b/src/lib/repo_ref.rs

@@ -229,6 +229,7 @@ impl RepoRef {

coordinates: HashSet::from_iter(vec![self.coordinate_with_hint()]),

protocol: None,

user: None,

+ nip05: None,

}

)

}

libgit2 1.8.1

Reply to this note

Please Login to reply.

Discussion

DanConwayDev 1y ago

Thanks for the update. I'm interested in how you submitted this. if you ran `git push --force` on the original branch (with the name `pr/nip05-lez`), it should have submitted it as a 'revision' to the existing PR instead of a new one. is that what you did? when you originally submitted it did you push with the '-u' flag to link the local branch with the upstream one? potentially there is a bug with ngit here.

Lez 1y ago

I think I just ran `git push` - but `push.autosetupremote=true` is set, so no need for `-u`.

DanConwayDev 1y ago

The integration test for force pushing a PR is passing so it would be interesting to identify why its not working in your context.

DanConwayDev 1y ago

A couple of ideas:

1) we could potentially store mapping of nip05 address to PublicKey in `git_repo.get_git_config_item("nostr.", Some(false))` so we only need to fetch it via the API the first time rather than every interaction. We wouldn't need to retain the nip05 relay hint because the repository announcement event is already in cache.

2) we could store NostrUrlDecoded in RepoRef and so that `to_nostr_git_url` could return the url with the nip05 address if it was originally used.

Lez 1y ago

Caching the pubkey for the nip05 makes sense. Config is a good place for it.

A couple of questions:

- RepoRef is used to directly map the 30617 event into a rust object?

- Should we add the nostr remote URL into the gitrepo event? It would reflect the choice of the repo owner how he wants to use it. For example, if my nip05 relays intersect with the repo relays, I will omit the relay hint. If they are distinct, I'll include. But it would be easier to just store the URL and use later, than to do the intersection repeatedly. And it's a problem for not cors-enabled nip05s on the web.

- Do you think it would make sense to further refactor the `coordinate` member of `NostrUrlDecoded`? If we could include the Coordinate members (identifier, pubkey, kind, relays) direct members of the `NostrUrlDecoded`, we could change pubkey to Option, and NostrUrlDecoded could hold the URL value after parsing but before looking up NIP05. Also, the duplication of the `identifier` wouldn't be needed. I duplicated that previously because I couln't instantiate Coordinate from a NIP05 nostr:// remote URL.

DanConwayDev 1y ago

1. Yes.

2. I'm not really sure what you mean here. I think its always worth having the relay hints in the 30317 event but if they are ommited, then fallback methods of identifying relays are used.

3. I think the original NostrUrlDecodeed struct is better with an extra `nip05: Option`property'. Whenever a url is decoded it should use an async parse function. Its a pain to pass around a version of the struct with so many optional properties which is barely useful without finding the pubkey.

Lez 1y ago

2. I called "repo relays" the "relays" tag value in the 30317 event, and "relay hint" the relay in the nostr remote URL, like: nostr://nip05/relay.hint/identifier

How will gitworkshop.dev find out which URL to show in the green "clone" dropdown? (nostr://nip05/relay.hint/identifier, or nostr://nip05/identifier? - or one of the npub / nevent / naddr variant?)

3. Fine, but then we need to add an "identifier: Option" member, too. Because we can't store it anywhere else until we haven't resolved the nip05 to a pubkey.

DanConwayDev 1y ago

2. Ah, I hadn't considered that the maintainer may want to express a view on whether contributors should use the nip05 or npub in the clone url.

I planned on it being a user preference and hadn't landed on a default for gitworkshop.dev.

Now we are on the topic of maintainers expressing a view, it's also conceivable that projects may want to suggest a generic project nip05 which may point to different maintainers over time as the project evolves.

Increasing reliance on DNS just feels a bit dirty and unsatisfactory, but I can see the easier UX.

Why don't we start off by adding basic support first, without adding anything to `30317`, and see what the feedback is like?

3. I'm not sure that we do. I've created this to demonstrate why not:

nostr:nevent1qvzqqqqx2ypzpgqgmmc409hm4xsdd74sf68a2uyf9pwel4g9mfdg8l5244t6x4jdqy88wumn8ghj7mn0wvhxcmmv9uq3wamnwvaz7tmjv4kxz7fwdehhxarj9e3xzmny9uq32amnwvaz7tmjv4kxz7fwv3sk6atn9e5k7tcppemhxue69uhkummn9ekx7mp0qythwumn8ghj7un9d3shjtnwdaehgu3wvfskuep0qy2hwumn8ghj7un9d3shjtnyv9kh2uewd9hj7qgwwaehxw309ahx7uewd3hkctcpzamhxue69uhhyetvv9ujumn0wd68ytnzv9hxgtcpz4mhxue69uhhyetvv9ujuerpd46hxtnfduhsqgzuxrs7ewutcjhy2k5nfghp9828u4ds5s80jtt6ud55uu2t5ntfe53rrwl2

Lez 1y ago

2. I'm fine by doing an MVP, let's not touch the 30317 for now. I think increasing UX is more important here. Especially that by the authorities taking the DNS they achieve nothing a.) it's a news flash for the project b.) somebody trusted will point a new nip05 to the project c.) educating the public it's a dependency d.) still being able to use the npub/naddr remote, which ngit could support out of the box (as all the info is cached in a checked out repo)

Lez 1y ago

Technical feedback: from nostrudel if I press "open on gitworkshop.dev", it tries to open nevent1qvzqqqqx2ypzpgqgmmc409hm4xsdd74sf68a2uyf9pwel4g9mfdg8l5244t6x4jdqqs9cv8pajach39wg4dfxj3wz2w50e2mpfqwlykh4cmffec5hfxknngf2ef2k there, which shows an error.

If I go through https://nostrapp.link/#nevent1qvzqqqqx2ypzpgqgmmc409hm4xsdd74sf68a2uyf9pwel4g9mfdg8l5244t6x4jdqqs9cv8pajach39wg4dfxj3wz2w50e2mpfqwlykh4cmffec5hfxknngf2ef2k?select=true it will open https://gitworkshop.dev/e/nevent1qqs9cv8pajach39wg4dfxj3wz2w50e2mpfqwlykh4cmffec5hfxknngzyzsq3hh327t0h2dq6matqn5064cgj2zanl2stkj6s0lg4t2h5dty67t3mez - which works.

I'll check the patch now. :)