Move fee calculation to cdk-common and add fee methods to Wallet trait#1805
Move fee calculation to cdk-common and add fee methods to Wallet trait#1805crodas wants to merge 1 commit intocashubtc:mainfrom
Conversation
crodas
force-pushed
the
feature/fees-functions
branch
from
826d72c to
a42fae0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1805 +/- ##
==========================================
- Coverage 62.51% 62.49% -0.03%
==========================================
Files 329 329
Lines 53644 53704 +60
==========================================
+ Hits 33535 33561 +26
- Misses 20109 20143 +34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Move `calculate_fee` and `ProofsFeeBreakdown` from `cdk/src/fees.rs` to `cdk-common/src/wallet/fees.rs` so downstream consumers of cdk-common can compute fees without depending on the full cdk crate. Add `get_proofs_fee`, `get_proofs_fee_by_count`, and `get_proofs_with` to the Wallet trait, and make `get_proofs_by_states` a default method that delegates to `get_proofs_with`. Expose corresponding FFI bindings.
crodas
force-pushed
the
feature/fees-functions
branch
from
a42fae0 to
a5e66c0
Compare
| /// Fee required to redeem proof set by count per keyset | ||
| async fn get_proofs_fee_by_count( | ||
| &self, | ||
| proofs_per_keyset: HashMap<Id, u64>, | ||
| ) -> Result<fees::ProofsFeeBreakdown, Self::Error>; |
There was a problem hiding this comment.
Do we think this fn should be in the public api, Is it even used privately? Imagine most will use the get_proofs_fee
| /// from the database. | ||
| async fn get_proofs_by_states(&self, states: Vec<State>) -> Result<Proofs, Self::Error>; | ||
| async fn get_proofs_by_states(&self, states: Vec<State>) -> Result<Proofs, Self::Error> { | ||
| self.get_proofs_with(Some(states), None).await |
There was a problem hiding this comment.
We're inconsistent about if the traits have an implementation here or just a definition. In cdk-coommon I think they should only have a definition.
|
@crodas Think this maybe this just fell of your radar? Do we still need it think so? |
|
We certainly needed but we agreed last week to work on the wallet documentation and planning first. I'm working on understanding what's really needed and what is an internal function used only for Rust. Once that document is done and approved, I'll adjust this PR accordingly |
2 participants
Description
Move
calculate_feeandProofsFeeBreakdownfromcdk/src/fees.rstocdk-common/src/wallet/fees.rsso downstream consumers of cdk-common can compute fees without depending on the full cdk crate.Add
get_proofs_fee,get_proofs_fee_by_count, andget_proofs_withto the Wallet trait, and makeget_proofs_by_statesa default method that delegates toget_proofs_with. Expose corresponding FFI bindings.Notes to the reviewers
Suggested CHANGELOG Updates
CHANGED
ADDED
REMOVED
FIXED
Checklist
just final-checkbefore committing