Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Various minor fixes #13945

Merged
merged 11 commits into from
Apr 26, 2023
Merged

Various minor fixes #13945

merged 11 commits into from
Apr 26, 2023

Conversation

gavofyork
Copy link
Member

No description provided.

@gavofyork gavofyork added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”. labels Apr 18, 2023
@@ -784,7 +784,7 @@ pub mod pallet {
return false
}
a.flags.set_new_logic();
if !a.reserved.is_zero() || !a.frozen.is_zero() {
if !a.reserved.is_zero() && a.frozen.is_zero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This logic here is essentially saying that frozen funds don't contribute any provider refs; is this true?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it's that frozen funds already require a consumer ref in the original logic, so if we have non-zero frozen funds, then we already have the consumer we need (and if we have a consumer, we must have a provider).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, so it's reserved funds in the old logic that did not have a consumer, and so we have to add it here?

frame/balances/src/impl_currency.rs Show resolved Hide resolved
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 24, 2023
@ggwpez
Copy link
Member

ggwpez commented Apr 26, 2023

bot merge

@paritytech-processbot
Copy link

Error: Response error (status 500 Internal Server Error):

{"error_message":""}

1 similar comment
@paritytech-processbot
Copy link

Error: Response error (status 500 Internal Server Error):

{"error_message":""}

@ggwpez
Copy link
Member

ggwpez commented Apr 26, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Statuses failed for 26ced3c

@ggwpez
Copy link
Member

ggwpez commented Apr 26, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@ggwpez
Copy link
Member

ggwpez commented Apr 26, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 943c520 into master Apr 26, 2023
@paritytech-processbot paritytech-processbot bot deleted the gav-fixes branch April 26, 2023 14:26
gpestana pushed a commit that referenced this pull request May 4, 2023
* Fix: Incorrect implementation of can_reserve check

* Fix: Incorrect migration of consumer counting for existing accounts with frozen amounts

* Fix: Inconsistent implementation between assets can_deposit and new_account

* Fixes

* Fixes

* Another fix

* Update tests.rs

* Update fungible_tests.rs

* Use `can_accrue_consumers` in the body of `can_inc_consumer`

---------

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Fix: Incorrect implementation of can_reserve check

* Fix: Incorrect migration of consumer counting for existing accounts with frozen amounts

* Fix: Inconsistent implementation between assets can_deposit and new_account

* Fixes

* Fixes

* Another fix

* Update tests.rs

* Update fungible_tests.rs

* Use `can_accrue_consumers` in the body of `can_inc_consumer`

---------

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants