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

Use nodex-didcomm #341

Merged
merged 8 commits into from
Jul 10, 2024
Merged

Use nodex-didcomm #341

merged 8 commits into from
Jul 10, 2024

Conversation

curry-like
Copy link
Contributor

@curry-like curry-like commented Jun 23, 2024

Description

To simplify nodex, we want to use nodex-didcomm.

WARNING

This PR removed unused internal APIs. Please check them.

@curry-like curry-like force-pushed the feature/use-nodex-didcomm branch 5 times, most recently from f9993a6 to ef3a79a Compare June 23, 2024 16:18
@curry-like curry-like force-pushed the feature/use-nodex-didcomm branch from fa11799 to 96986a9 Compare June 25, 2024 12:30
@curry-like curry-like marked this pull request as ready for review June 25, 2024 12:30
@curry-like curry-like requested a review from a team as a code owner June 25, 2024 12:30
std::process::Command::new(&agent_path)
.spawn()
.expect("Failed to execute command");
Command::new(&agent_path).spawn()?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember that if the code by the original deamonize is deleted, the information of the parent process is inherited, which causes a problem in the operation to stop other processes when the process is started by the update.

#325

Copy link
Contributor

Choose a reason for hiding this comment

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

--- a/Cargo.toml
+++ b/Cargo.toml
@@ -34,12 +34,11 @@ anyhow = "1.0.81"
 arrayref = { version = "0.3.7" }
 async-trait = { version = "0.1.80" }
 base64 = { version = "0.22.0" }
-chrono = { version = "0.4" }
+chrono = { version = "0.4", features = ["serde"] }
 clap = { version = "4.5.2", features = ["cargo", "derive"] }
 cuid = { version = "1.3.2" }
 daemonize = "0.5.0"
 data-encoding = { version = "2.5.0" }
-didcomm-rs = { git = "https://github.com/nodecross/didcomm-rs.git", tag = "v0.8.0" }
 digest = { version = "0.10.7" }
 dirs = { version = "5.0.1" }
 dotenvy = "0.15.7"

I think that didcomm-rs is no longer needed.

project_verifier: Box<dyn ProjectVerifier + Send + Sync + 'static>,
message_activity_repository: Box<dyn MessageActivityRepository + Send + Sync + 'static>,
didcomm_encrypted_service: DIDCommEncryptedService,
pub struct DidcommMessageUseCase<D: DidRepository> {
Copy link
Contributor

Choose a reason for hiding this comment

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

pub struct DidcommMessageUseCase<V, R, D, A>
where
    V: ProjectVerifier,
    R: MessageActivityRepository,
    D: DidRepository,
    A: DidAccessor,
{
    project_verifier: V,
    message_activity_repository: R,
    didcomm_encrypted_service: DIDCommEncryptedService<D>,
    did_accessor: A,
}

impl<V, R, D, A> DidcommMessageUseCase<V, R, D, A>
where
    V: ProjectVerifier,
    R: MessageActivityRepository,
    D: DidRepository,
    A: DidAccessor,
{
    pub fn new(
        project_verifier: V,
        message_activity_repository: R,
        didcomm_encrypted_service: DIDCommEncryptedService<D>,
        did_accessor: A,
    ) -> Self {
        DidcommMessageUseCase {
            project_verifier,
            message_activity_repository,
            didcomm_encrypted_service,
            did_accessor,
        }
    }
}

I think you can avoid the Box::new idiom by doing the above.

(Maybe DIDCommEncryptedService should be a trait like below.)

#[async_trait::async_trait]
pub trait DIDCommEncryptedService {
...
}

#[async_trait::async_trait]
impl<R: DidRepository> DIDCommEncryptedService for R {
...
}

@curry-like curry-like requested a review from a team as a code owner July 7, 2024 16:33
@curry-like curry-like requested a review from da13da July 7, 2024 16:33
Cargo.toml Outdated
clap = { version = "4.5.2", features = ["cargo", "derive"] }
colored = { version = "2" }
cuid = { version = "1.3.2" }
daemonize = "0.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflict has caused daemonize and nix to be unix-only dependencies and overall dependencies.
By removing daemonize and nix from the overall dependencies, I was able to successfully build on the actual device.

@curry-like curry-like force-pushed the feature/use-nodex-didcomm branch from 1ff7b85 to fbbc7e6 Compare July 8, 2024 16:58
@curry-like curry-like requested a review from da13da July 10, 2024 13:45
@curry-like curry-like merged commit eff2c6b into main Jul 10, 2024
14 checks passed
@curry-like curry-like deleted the feature/use-nodex-didcomm branch July 10, 2024 16:41
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.

3 participants