-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add wasm foundation #363
Add wasm foundation #363
Conversation
2041c15
to
d8504f0
Compare
@@ -8,9 +8,12 @@ setup: | |||
git submodule update --init --recursive | |||
fi | |||
if [[ "$(cargo 2>&1)" == *"rustup could not choose a version of cargo to run"* ]]; then | |||
rustup default 1.74.0 | |||
rustup default 1.76.0 # TODO undo this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diehuxx I know this is no bueno but thinking of leaving it for now. I had to do it because of the wasm-pack
version we're using... though we could look into moving to a lower wasm-pack
I'm not sure what the consequences would be. Lmk your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine. We CI job that checks if the web5 crate has MSRV 1.74. The CI job checks crates/web5/Cargo.toml
to get rust-version = 1.74
, so as long as we don't change that, we're fine.
The only drawback of using a higher rust version during development is that we may accidentally use features in crates/web5
that aren't supported in version 1.74. If that happens, we'll just notice them in the CI job failure and fix afterward.
} | ||
|
||
#[wasm_bindgen] | ||
extern "C" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's an example of a foreign implementation, for which we'll use this same pattern for a foreign fetch
implementation
impl ToJson for JwtClaims {} | ||
|
||
pub struct Jwt { | ||
pub struct Jws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh btw @diehuxx @nitro-neal forgot to mention earlier, but I've also added Jws
here b/c tbdex uses a detatched JWS for message signatures
* main: Revert Revert Remove reqwest dependency (#366) Revert "Remove reqwest dependency (#354)" (#365) Remove reqwest dependency (#354) [TBD Release Manager 🚀] Setting next development version after 0.0.5 to: 0.0.0-main-SNAPSHOT [TBD Release Manager 🚀] Setting version to: 0.0.5 Update release workflow (#362) [TBD Release Manager 🚀] Setting next development version after 0.0.1 to: 0.0.0-main-SNAPSHOT [TBD Release Manager 🚀] Setting version to: 0.0.1 remove circular dependency (#360) Artifact Publishing (#358)
f9a69bd
to
4d58a72
Compare
# This script is really a workaround for https://github.com/rustwasm/wasm-pack/issues/1074. | ||
# | ||
# Currently, the only reliable way to load WebAssembly in all the JS | ||
# environments we want to target seems to be to pack the WASM into base64, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is awesome
const wasmJwk = this.wasmKeyManager.import_private_jwk(privateJwk.toWasmJwk()); | ||
return Jwk.fromWasmJwk(wasmJwk); | ||
} catch (error) { | ||
throw catchWeb5Error(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its strange throwing a function,
Chad says we can do this:
// Define the decorator
function CatchWeb5Error(
target: Object,
propertyKey: string,
descriptor: PropertyDescriptor
) {
const originalMethod = descriptor.value;
descriptor.value = function (...args: any[]) {
try {
return originalMethod.apply(this, args);
} catch (error) {
throw catchWeb5Error(error);
}
};
return descriptor;
}
// Apply the decorator to your methods
class YourClass {
@CatchWeb5Error
computeThumbprint(): string {
const wasmJwk = this.toWasmJwk();
return wasmJwk.compute_thumbprint();
}
@CatchWeb5Error
anotherMethod(): void {
// Your method implementation
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can think about it and if we want to go this way or another can make an issue to update error handling across the board
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! I'm in favor or custom attributes for this behavior, given that's a viable option in ts land.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah, in a follow up PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like decorators are still considered an experimental feature so I may settle on a higher order function for now
@@ -38,6 +38,7 @@ pub fn sign_with_did( | |||
additional_properties.insert("vc".to_string(), vc_claim.to_json_value()?); | |||
|
|||
let claims = JwtClaims { | |||
aud: None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
@@ -4,8 +4,7 @@ pub mod dids; | |||
|
|||
mod datetime; | |||
pub mod errors; | |||
mod http; | |||
mod jose; | |||
pub mod jose; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't catch this before merge, but why are we exposing jose
now? IIRC we wanted to keep this as an implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbdex uses detached compact jws as it's signature field
No description provided.