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

refactor: account balance #421

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

Daanvdplas
Copy link
Collaborator

@Daanvdplas Daanvdplas commented Jan 8, 2025

  • Main thing is that I combined the two tests relevant to the account balance (account balance update and deposit).
  • The deposit tests were not checking the reserved balance so added that.
  • Refactored the tests to common_functions as it much cleaner (separating unit tests to their own file).
  • Changed the depositor for the atomic swap to the destination accounts.
  • Left some comments to clarify how something works and why

[id; 32].into()
}

fn root() -> RuntimeOrigin {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed as it was used very inconsistently in the added code.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 97.98995% with 4 lines in your changes missing coverage. Please review.

Please upload report for BASE (frank/nfts-balance-deposit@e4020db). Learn more about missing BASE report.

Files with missing lines Patch % Lines
pallets/nfts/src/impl_nonfungibles.rs 0.00% 4 Missing ⚠️
@@                      Coverage Diff                      @@
##             frank/nfts-balance-deposit     #421   +/-   ##
=============================================================
  Coverage                              ?   71.36%           
=============================================================
  Files                                 ?       72           
  Lines                                 ?    13555           
  Branches                              ?    13555           
=============================================================
  Hits                                  ?     9674           
  Misses                                ?     3608           
  Partials                              ?      273           
Files with missing lines Coverage Δ
pallets/nfts/src/common_functions.rs 90.25% <100.00%> (ø)
pallets/nfts/src/features/atomic_swap.rs 90.78% <100.00%> (ø)
pallets/nfts/src/features/transfer.rs 83.68% <100.00%> (ø)
pallets/nfts/src/lib.rs 71.51% <ø> (ø)
pallets/nfts/src/tests.rs 99.91% <100.00%> (ø)
pallets/nfts/src/impl_nonfungibles.rs 22.12% <0.00%> (ø)

Copy link
Collaborator

@chungquantin chungquantin left a comment

Choose a reason for hiding this comment

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

LGTM!

@Daanvdplas Daanvdplas merged commit 986a5b6 into frank/nfts-balance-deposit Jan 8, 2025
13 of 15 checks passed
@Daanvdplas Daanvdplas deleted the daan/account_balance branch January 8, 2025 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants