-
Notifications
You must be signed in to change notification settings - Fork 224
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
Modularization III-ish #941
Conversation
Codecov ReportBase: 64.26% // Head: 65.05% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #941 +/- ##
==========================================
+ Coverage 64.26% 65.05% +0.78%
==========================================
Files 112 111 -1
Lines 16412 16196 -216
==========================================
- Hits 10547 10536 -11
+ Misses 5865 5660 -205
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
cc09e2a
to
8977758
Compare
.register_async_method(path, move |params, state| { | ||
Box::pin(async move { | ||
let params = params.one::<serde_json::Value>()?; | ||
// Using AssertUnwindSafe here is far from ideal. In theory this means we could |
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.
BTW. the whole UnwindSafe
is one of a few broken things in Rust and possibly will be deprecated altogether. rust-lang/rust#40628
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 think it's a separate issue (created #943), I just copied the old code to adapt it.
modules/fedimint-ln/src/lib.rs
Outdated
type VerificationCache = LightningVerificationCache; | ||
|
||
fn module_key(&self) -> fedimint_api::encoding::ModuleKey { | ||
todo!() |
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.
Intentional or leftover?
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.
Good catch!
This automatically bridges the old modules into the new, type-erased module system
8977758
to
6b6116d
Compare
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.
LGTM
This is an attempt at finishing modularization in a way that is quick, will allow external modules soon, but leaves most of the code moving of existing modules for later.