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

feat: add verify inclusion proof to cli #95

Conversation

DhananjayPurohit
Copy link
Contributor

No description provided.

Copy link
Contributor

@ariard ariard left a comment

Choose a reason for hiding this comment

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

Sounds good just the other API can be used.

@@ -143,3 +144,10 @@ message GenerateTxInclusionProofRequest {
message GenerateTxInclusionProofReply {
string merkle_block = 1;
}

message VerifyInclusionProofRequest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good if we can move those methods on the civkitservice.proto interface. That way they can be reused by notaryd in the future.

@@ -71,6 +73,7 @@ impl ServiceManager
send_bitcoind_request: Mutex::new(send_bitcoind_request),
send_events_gateway: Mutex::new(send_gateway_events),
our_service_pubkey: pubkey,
inclusion_proof: inclusion_proof,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to integrate it like this, when the mainstay module is more encapsulated, it can be moved in notaryd.

@ariard
Copy link
Contributor

ariard commented Nov 26, 2023

Note the inclusion proof verification interface can be integrated in civkit-sample, which is the main client. civkit-cli is more an admin tool to manage civkitd, even if a lot of commands have been integrated at the beginning to test quickly civkitd functionality, my bad.

@ariard
Copy link
Contributor

ariard commented Dec 13, 2023

See #95, it’s taking it over with fixes. Thanks for the fixes from latest review.

@ariard ariard closed this Dec 13, 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