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

Upgrade #19

Merged
merged 21 commits into from
Jan 18, 2025
Merged

Upgrade #19

merged 21 commits into from
Jan 18, 2025

Conversation

iorveth
Copy link
Contributor

@iorveth iorveth commented Dec 31, 2024

Closes #16

@iorveth iorveth added the enhancement New feature or request label Dec 31, 2024
@iorveth iorveth self-assigned this Dec 31, 2024
Copy link
Contributor

@whalelephant whalelephant left a comment

Choose a reason for hiding this comment

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

Thank you for this! some organisation comments for your feedback!

@@ -23,9 +23,51 @@ pub enum SdjwtVerifierResultError {
IdxRevoked(u64),
}

impl std::fmt::Display for SdjwtVerifierResultError {
Copy link
Contributor

Choose a reason for hiding this comment

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

why impl and not just instead of using thiserror::Error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 52c864a

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this file? this is basically the expected ExecMsg right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


// Execute messages
#[cosmwasm_schema::cw_serde]
pub enum ExecuteMsg {
Copy link
Contributor

Choose a reason for hiding this comment

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

these can be moved to the package so to act as "interface".

Please look at https://github.com/CosmWasm/cw-plus/tree/main/packages/cw20 and how the packages are used in the contract implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done 19c8202

sd-jwt-rs = { workspace = true }
jsonwebtoken = { workspace = true }
serde_json = { workspace = true, default-features = false, features = ["alloc"]}
serde-json-wasm = {workspace = true}
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw this
CosmWasm/cosmwasm#2195

looks like we can replace remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, f816656


use crate::types::{PendingRoute, VerificationRequirements};

pub const MAX_PRESENTATION_LENGTH: Item<usize> = Item::new("max_presentation_length");
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say some of these states can be moved to the package to be shared across verifier implementations.

For example, we have the typeVerfiablePresentation as Binary in the package because it will be required for any verifier, sd-jwt, anoncreds, zk, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved MAX_PRESENTATION_LENGTH to package, let's see what we can reuse after we develop other verifiers.
52c864a

@iorveth iorveth requested a review from whalelephant January 2, 2025 14:23
scripts/build.sh Show resolved Hide resolved
Copy link
Contributor

@whalelephant whalelephant left a comment

Choose a reason for hiding this comment

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

great job!

@iorveth iorveth merged commit 09a2f21 into main Jan 18, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite AVIDA in vanilla cosmwasm
2 participants