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.

Sign up for GitHub

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

Jump to bottom

Fix Maker fees & Document fees workflow. #355

Merged
merged 5 commits into from
Jan 1, 2025
Merged

Fix Maker fees & Document fees workflow. #355

merged 5 commits into from
Jan 1, 2025

Conversation

KnowWhoami
Copy link
Collaborator

@KnowWhoami KnowWhoami commented Dec 30, 2024 •
edited
Loading

This pr is an extension of #312 which aims to :

@KnowWhoami KnowWhoami force-pushed the fees branch 3 times, most recently from fee00c0 to 13f8bb4 Compare December 30, 2024 14:16
@KnowWhoami KnowWhoami mentioned this pull request Dec 30, 2024
Merged
@KnowWhoami
enhancements in test section.
4a3c042 
 This involves:
 - Docmentation about fee workflow for every IT.
 - Refactor the IT's as almost IT's uses same code -> thus created some api's to reduce redundancy.
 - Fix some bugs in calculating coinswap fees.
@codecov Codecov
Copy link

codecov bot commented Dec 30, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.13514% with 11 lines in your changes missing coverage. Please review.

Project coverage is 73.04%. Comparing base (bcbd223) to head (cedfe95).
Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
src/maker/server.rs 80.00% 3 Missing ⚠️
src/taker/api.rs 84.61% 2 Missing ⚠️
src/bin/directoryd.rs 0.00% 1 Missing ⚠️
src/bin/makerd.rs 0.00% 1 Missing ⚠️
src/bin/taker.rs 0.00% 1 Missing ⚠️
src/maker/handlers.rs 90.00% 1 Missing ⚠️
src/taker/offers.rs 80.00% 1 Missing ⚠️
src/wallet/rpc.rs 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
- Coverage   73.43%   73.04%   -0.40%     
==========================================
  Files          33       33              
  Lines        4134     4107      -27     
==========================================
- Hits         3036     3000      -36     
- Misses       1098     1107       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mojoX911
mojoX911 requested changes Dec 31, 2024
View reviewed changes
Copy link

@mojoX911 mojoX911 left a comment

Choose a reason for hiding this comment

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

ACK!!. I love the new claulation tables in the test. Just some more changes and nits and this is good to go.

maker.toml Outdated Show resolved Hide resolved
src/maker/handlers.rs Outdated Show resolved Hide resolved
src/maker/handlers.rs Outdated
&& total_funding_amount < self.wallet.read()?.store.offer_maxsize
// TODO: Taker must not be allowed to send the amount beyond this range?
// why can't it be <= ?
Copy link

Choose a reason for hiding this comment

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

It can be.

Copy link

Choose a reason for hiding this comment

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

I think it would be <=

src/maker/handlers.rs Outdated Show resolved Hide resolved
src/maker/handlers.rs Outdated Show resolved Hide resolved
src/taker/api.rs Outdated Show resolved Hide resolved
src/taker/routines.rs Show resolved Hide resolved
src/taker/routines.rs Outdated Show resolved Hide resolved
src/taker/routines.rs Show resolved Hide resolved
tests/standard_swap.rs Outdated Show resolved Hide resolved
mojoX911
mojoX911 approved these changes Jan 1, 2025
View reviewed changes
Copy link

@mojoX911 mojoX911 left a comment

Choose a reason for hiding this comment

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

Ack

@mojoX911 mojoX911 merged commit 406f59c into citadel-tech:master Jan 1, 2025
7 of 8 checks passed
@mojoX911 mojoX911 mentioned this pull request Jan 1, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mojoX911 mojoX911 mojoX911 approved these changes

@Shourya742 Shourya742 Awaiting requested review from Shourya742

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Doc] Analyze and document the balances.
2 participants
@KnowWhoami @mojoX911