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

✨ Set balance to u128 instead of config type on pallet_funding #394

Merged
merged 1 commit into from
Sep 12, 2024
Merged

✨ Set balance to u128 instead of config type on pallet_funding #394

merged 1 commit into from
Sep 12, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Sep 3, 2024 •
edited
Loading

What?

  • Remove the Balance config type, and replace it for u128

Why?

  • Having to access our fundamental balance type through trait interfaces was making our code unnecessarily bloated. The only reason to have a generic balance type is if our pallet is written to be used in many other blockchains, which is not the case.
  • Now we can use directly u128, instead of having to convert with into()
  • We were already assuming 99% that it was u128 since it had to implement From

How?

  • Replace BalanceOf with Balance
  • Delete all unnecessary conversion
  • Remove the generic Balance types from the pallet funding types

Testing?

Normal tests

Anything Else?

This will now allow us to use abs_diff for the vesting api

This was referenced Sep 3, 2024
Merged
Closed
Merged
Merged
Merged
@JuaniRios Graphite App
Copy link
Contributor Author

JuaniRios commented Sep 3, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @JuaniRios and the rest of your teammates on Graphite Graphite

@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 5eb0414 to bedd4b9 Compare September 3, 2024 09:42
@JuaniRios JuaniRios changed the title Set balance to u128 instead of config type on pallet_funding ✨ Set balance to u128 instead of config type on pallet_funding Sep 3, 2024
@JuaniRios JuaniRios mentioned this pull request Sep 3, 2024
Merged
@JuaniRios JuaniRios marked this pull request as ready for review September 3, 2024 12:29
@graphite-app Graphite App
Copy link

graphite-app bot commented Sep 3, 2024

Graphite Automations

"Auto-assign PRs to author" took an action on this PR • (09/03/24)

1 assignee was added to this PR based on Juan Ignacio Rios's automation.

@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 9649ce5 to bf7b12e Compare September 3, 2024 13:20
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from bedd4b9 to 15aef18 Compare September 3, 2024 13:20
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from bf7b12e to 0c4ca25 Compare September 3, 2024 14:06
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 15aef18 to e6860ce Compare September 3, 2024 14:06
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 0c4ca25 to 4bab649 Compare September 3, 2024 14:20
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from e6860ce to 900d79d Compare September 3, 2024 14:21
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 4bab649 to c78d726 Compare September 3, 2024 14:30
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 900d79d to fb74fdb Compare September 3, 2024 14:30
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from c78d726 to 9b7c136 Compare September 3, 2024 14:39
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from fb74fdb to 425b95f Compare September 3, 2024 14:39
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 9b7c136 to cb4d134 Compare September 4, 2024 13:07
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 425b95f to 9fc2c56 Compare September 4, 2024 13:08
@JuaniRios JuaniRios mentioned this pull request Sep 4, 2024
Merged
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from cb4d134 to 5b86ab8 Compare September 9, 2024 11:10
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 9fc2c56 to 60d3f2a Compare September 9, 2024 11:11
@JuaniRios JuaniRios mentioned this pull request Sep 9, 2024
Merged
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 5b86ab8 to 470d279 Compare September 10, 2024 08:42
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 60d3f2a to 43ac0a3 Compare September 10, 2024 08:42
lrazovic
lrazovic approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@lrazovic lrazovic left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I see the simplification this brings in terms of syntax, but I would wait for feedback from @vstam1 before merging this

integration-tests/src/tests/e2e.rs Outdated Show resolved Hide resolved
@lrazovic lrazovic requested a review from vstam1 September 10, 2024 09:13
@lrazovic lrazovic force-pushed the 09-03-delete_funding_storage_migrations branch from 470d279 to 59416da Compare September 10, 2024 13:45
@lrazovic lrazovic force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 43ac0a3 to b321570 Compare September 10, 2024 13:45
@JuaniRios JuaniRios force-pushed the 09-03-delete_funding_storage_migrations branch from 59416da to 74e52d9 Compare September 11, 2024 11:15
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch 2 times, most recently from d95220e to 450081d Compare September 12, 2024 08:35
vstam1
vstam1 approved these changes Sep 12, 2024
View reviewed changes
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

Lgtm

vstam1
vstam1 approved these changes Sep 12, 2024
View reviewed changes
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

Lgtm

@JuaniRios Graphite App
Copy link
Contributor Author

JuaniRios commented Sep 12, 2024 •
edited
Loading

Merge activity

  • Sep 12, 9:39 AM EDT: @JuaniRios started a stack merge that includes this pull request via Graphite.
  • Sep 12, 9:47 AM EDT: Graphite rebased this pull request as part of a merge.
  • Sep 12, 9:48 AM EDT: @JuaniRios merged this pull request with Graphite.

@JuaniRios JuaniRios changed the base branch from 09-03-delete_funding_storage_migrations to graphite-base/394 September 12, 2024 13:43
@JuaniRios JuaniRios changed the base branch from graphite-base/394 to main September 12, 2024 13:45
@JuaniRios JuaniRios force-pushed the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch from 450081d to 56cf954 Compare September 12, 2024 13:46
@JuaniRios JuaniRios merged commit 951d9cb into main Sep 12, 2024
1 check passed
@JuaniRios JuaniRios deleted the 09-03-set_balance_to_u128_instead_of_config_type_on_pallet_funding branch September 12, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstam1 vstam1 vstam1 approved these changes

@lrazovic lrazovic lrazovic approved these changes

Assignees

@JuaniRios JuaniRios

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JuaniRios @lrazovic @vstam1