diff --git a/crates/signed_git/src/repo.rs b/crates/signed_git/src/repo.rs index de8a895..811f840 100644 --- a/crates/signed_git/src/repo.rs +++ b/crates/signed_git/src/repo.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeSet, HashSet}; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::Stdio; @@ -1262,55 +1262,6 @@ impl Repo { Ok(()) } - /// Checks that `url` advertises exactly the expected ref oids. - pub fn remote_has_refs(&self, url: &str, expected: &[(String, String)]) -> Result { - if expected.is_empty() { - return Ok(true); - } - - let url = Self::transport_url(url); - - let refspecs = expected - .iter() - .map(|(name, _)| { - gix::refspec::parse( - gix::bstr::BStr::new(format!("+{name}:{name}").as_bytes()), - gix::refspec::parse::Operation::Fetch, - ) - .map(|spec| spec.to_owned()) - }) - .collect::, _>>() - .into_anyhow() - .context("invalid refspec")?; - - let options = gix::remote::ref_map::Options { - extra_refspecs: refspecs, - ..Default::default() - }; - - let (refs, _) = self - .inner - .remote_at(url.as_str()) - .with_context(|| format!("cannot use remote {url}"))? - .connect(gix::remote::Direction::Fetch) - .with_context(|| format!("cannot connect to {url}"))? - .ref_map(Discard, options) - .with_context(|| format!("listing refs of {url} failed"))?; - - let advertised: HashMap = refs - .remote_refs - .iter() - .filter_map(|reference| { - let (name, object, _peeled) = reference.unpack(); - object.map(|oid| (String::from_utf8_lossy(name).into_owned(), oid.to_string())) - }) - .collect(); - - Ok(expected - .iter() - .all(|(name, oid)| advertised.get(name.as_str()) == Some(oid))) - } - /// Sets the `origin` URL when the repository has no remote yet. pub fn ensure_origin(&self, url: &str) -> Result<()> { if self.inner.find_remote("origin").is_ok() { diff --git a/crates/signed_git/src/tests.rs b/crates/signed_git/src/tests.rs index 59dca08..8e4a8d0 100644 --- a/crates/signed_git/src/tests.rs +++ b/crates/signed_git/src/tests.rs @@ -113,42 +113,6 @@ fn ref_state_matches_the_git_cli_ref_listing() { ); } -#[test] -fn remote_has_refs_reports_whether_pushed_refs_landed() { - let server = tempfile::tempdir().unwrap(); - bare_server(server.path(), "npub1test", "my-repo"); - let (dir, repo) = fixture(&[("a.txt", b"one")]); - commit_all(&repo, "initial"); - let dir = dir.path(); - let main = git_in(dir, &["rev-parse", "refs/heads/main"]).expect("main oid"); - let url = format!("file://{}/npub1test/my-repo.git", server.path().display()); - let expected = vec![("refs/heads/main".to_owned(), main.clone())]; - let repo = Repo::open(dir).expect("open"); - - assert!(!repo.remote_has_refs(&url, &expected).expect("probe")); - - repo.push_all( - &format!("file://{}", server.path().display()), - "npub1test", - "my-repo", - ) - .expect("push"); - - assert!(repo.remote_has_refs(&url, &expected).expect("probe")); - - let stale = vec![("refs/heads/main".to_owned(), "0".repeat(40))]; - assert!(!repo.remote_has_refs(&url, &stale).expect("probe")); - - git_run(dir, &["tag", "v1.0"]); - repo.push_all( - &format!("file://{}", server.path().display()), - "npub1test", - "my-repo", - ) - .expect("push"); - assert!(repo.remote_has_refs(&url, &expected).expect("probe")); -} - #[test] fn repo_ref_state_lists_branches_tags_and_head() { let (_dir, repo) = fixture(&[("a.txt", b"hello")]); diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 5c6a164..6d847db 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -5,9 +5,7 @@ use std::time::{Duration, Instant}; use anyhow::{Error, anyhow, bail}; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{ - App, AppContext, AsyncApp, Context, Entity, EventEmitter, Global, Task, TaskExt, WeakEntity, -}; +use gpui::{App, AppContext, AsyncApp, Context, Entity, EventEmitter, Global, Task, TaskExt}; use nostr::event::IntoEventBuilder; use nostr::nips::nip19::Nip19Coordinate; use nostr_connect::prelude::*; @@ -194,7 +192,7 @@ impl Backend { this.set_signer(keys, cx); })?; } else if content.starts_with("bunker://") { - let (base, keys) = extract_master_key(&content); + let (base, keys) = extract_master_key(&content)?; let uri = NostrConnectUri::parse(base)?; let mut signer = NostrConnect::new( uri, @@ -463,7 +461,7 @@ impl Backend { let event = builder.finalize_async(&signer).await?; let output = client.send_event(&event).broadcast().await?; - if output.success.is_empty() && !output.failed.is_empty() { + if output.success.is_empty() { let reasons = output .failed .values() @@ -539,7 +537,7 @@ impl Backend { folder.join(dir_name) }; - cx.spawn(async move |this, cx| { + cx.spawn(async move |_this, cx| { let work = cx.background_spawn({ let destination = destination.clone(); let name = name.clone(); @@ -577,7 +575,6 @@ impl Backend { ); let event = announce_repository_and_push( - &this, &client, &signer, announcement, @@ -643,7 +640,7 @@ impl Backend { let client = self.client.clone(); let signer = self.signer.clone(); - cx.spawn(async move |this, cx| { + cx.spawn(async move |_this, cx| { let (mut state, euc) = cx .background_spawn({ let path = path.clone(); @@ -672,7 +669,6 @@ impl Backend { let ann = build_announcement(&repo_id, &name, &description, &owner, &servers, euc); let event = announce_repository_and_push( - &this, &client, &signer, ann, @@ -1155,11 +1151,15 @@ impl Backend { cx.background_spawn(async move { let builder = EventDeletionRequest::new().id(event.id); let deletion = builder.finalize_async(&signer).await?; - client.send_event(&deletion).broadcast().await?; + let output = client.send_event(&deletion).broadcast().await?; - Ok::<(), anyhow::Error>(()) + if output.success.is_empty() { + bail!("deletion event not accepted by any relay"); + } + + Ok::<(), Error>(()) }) - .detach(); + .detach_and_log_err(cx); } } } @@ -1190,7 +1190,6 @@ fn build_announcement( // Announce the repository, then stage the state event and push. #[allow(clippy::too_many_arguments)] async fn announce_repository_and_push( - backend: &WeakEntity, client: &Client, signer: &UniversalSigner, announcement: GitRepositoryAnnouncement, @@ -1204,13 +1203,15 @@ async fn announce_repository_and_push( push: impl Fn(&Path, &str, &str, &str) -> Result<(), Error> + Send + 'static, ) -> Result { for url in servers { - client.add_relay(url).and_connect().await.ok(); + if let Err(e) = client.add_relay(url).and_connect().await { + log::warn!("failed to connect grasp relay {url}: {e}"); + } } let event = announcement.finalize_async(signer).await?; let output = client.send_event(&event).broadcast().await?; - if output.success.is_empty() && !output.failed.is_empty() { + if output.success.is_empty() { let reasons = output .failed .values() @@ -1252,17 +1253,34 @@ async fn announce_repository_and_push( if outcome.accepted() == 0 { // Retract the announcement so the repository is not left announced without content. - backend - .update(cx, |backend, cx| { - backend.retract_events(std::slice::from_ref(&event), cx); - }) - .ok(); + let retracted = async { + let deletion = EventDeletionRequest::new() + .id(event.id) + .finalize_async(signer) + .await?; + let output = client.send_event(&deletion).broadcast().await?; - return Err(anyhow!( - "The repository was announced, but the push to every grasp server failed: {}. \ - The announcement has been retracted", - outcome.failure_summary() - )); + // Without relay acceptance the deletion request stays live. + if output.success.is_empty() { + bail!("deletion event not accepted by any relay"); + } + + Ok::<(), Error>(()) + } + .await; + + return Err(match retracted { + Ok(()) => anyhow!( + "The repository was announced, but the push to every grasp server failed: {}. \ + The announcement has been retracted", + outcome.failure_summary() + ), + Err(e) => anyhow!( + "The repository was announced, but the push to every grasp server failed: {}. \ + Retracting the announcement failed: {e}; delete the repository to clean it up", + outcome.failure_summary() + ), + }); } // Fan the state out to the relays once a git server holds the objects. @@ -1313,15 +1331,13 @@ impl UpdateEvent { } } -fn extract_master_key(credential: &str) -> (&str, Keys) { +fn extract_master_key(credential: &str) -> Result<(&str, Keys), Error> { match credential.split_once("master=") { Some((base, nsec)) => { - let keys = SecretKey::parse(nsec) - .map(Keys::new) - .unwrap_or_else(|_| Keys::generate()); - (base.trim_end_matches(['?', '&']), keys) + let keys = Keys::new(SecretKey::parse(nsec)?); + Ok((base.trim_end_matches(['?', '&']), keys)) } - None => (credential, Keys::generate()), + None => Ok((credential, Keys::generate())), } }