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

Assets: can_decrease/increase for destroying asset is not successful #3286

Merged
merged 11 commits into from
Jul 7, 2024

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Feb 12, 2024

Functions can_decrease and can_increase do not return successful consequence results for assets undergoing destruction; instead, they return the UnknownAsset consequence variant.

This update aligns their behavior with similar functions, such as reducible_balance, increase_balance, decrease_balance, and burn, which return an AssetNotLive error for assets in the process of being destroyed.

@muharem muharem requested a review from a team as a code owner February 12, 2024 08:57
@muharem muharem changed the title Assets: cccccbegbjkdvnrdngllbkinttegtuuvvtrinlfbjtrfcan_decrease/increase for destroying asset is not successful Assets: can_decrease/increase for destroying asset is not successful Feb 12, 2024
@muharem muharem added the T2-pallets This PR/Issue is related to a particular pallet. label Feb 12, 2024
@@ -132,6 +132,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Some(details) => details,
None => return DepositConsequence::UnknownAsset,
};
if details.status == AssetStatus::Destroying {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd use matches! instead of ==. Same for usage below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the benefit? with == I follow the style within the function

Copy link
Contributor

Choose a reason for hiding this comment

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

Pattern matching is the standard way of comparing enums and it doesn't require PartialEq/Eq, but in this case it's not a big deal.

@bkchr
Copy link
Member

bkchr commented May 17, 2024

@muharem what is the status of this pr?

@muharem
Copy link
Contributor Author

muharem commented May 17, 2024

@muharem what is the status of this pr?

I did add it to the backlog for audit. But it is ready to be merged.

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6641273

@muharem
Copy link
Contributor Author

muharem commented Jul 7, 2024

bot merge

@command-bot
Copy link

command-bot bot commented Jul 7, 2024

@muharem bot merge and bot rebase are not supported anymore. Please use native Github "Auto-Merge" and "Update Branch" buttons instead.
image

@muharem muharem added this pull request to the merge queue Jul 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 7, 2024
@muharem muharem enabled auto-merge July 7, 2024 11:18
@muharem muharem added this pull request to the merge queue Jul 7, 2024
Merged via the queue into master with commit f7dd85d Jul 7, 2024
156 of 160 checks passed
@muharem muharem deleted the muharem-assets-destroy-lock branch July 7, 2024 12:14
TomaszWaszczyk pushed a commit to TomaszWaszczyk/polkadot-sdk that referenced this pull request Jul 7, 2024
…aritytech#3286)

Functions `can_decrease` and `can_increase` do not return successful
consequence results for assets undergoing destruction; instead, they
return the `UnknownAsset` consequence variant.

This update aligns their behavior with similar functions, such as
`reducible_balance`, `increase_balance`, `decrease_balance`, and `burn`,
which return an `AssetNotLive` error for assets in the process of being
destroyed.
ordian added a commit that referenced this pull request Jul 15, 2024
* master: (120 commits)
  network/tx: Ban peers with tx that fail to decode (#5002)
  Try State Hook for Bounties (#4563)
  [statement-distribution] Add metrics for distributed statements in V2 (#4554)
  added sync command (#4818)
  Bridges V2 refactoring backport and `pallet_bridge_messages` simplifications (#4935)
  xcm-executor: Improve logging (#4996)
  Remove usage of `sp-std` on templates (#5001)
  fixed cmd bot commenting not working (#5000)
  Explain usage of `<T: Config>` in FRAME storage + Update parachain pallet template  (#4941)
  Expose metadata-hash feature from polkadot crate (#4886)
  Add `MAX_INSTRUCTIONS_TO_DECODE` to XCMv2 (#4978)
  add notices to the implementer's guide docs that changed for elastic scaling (#4983)
  `polkadot-parachain` simplifications and deduplications (#4916)
  Update Templates README docs (#4980)
  allow clear_origin in safe xcm builder (#4777)
  litep2p/peerstore: Fix bump last updated time (#4971)
  Make `tracing::log` work in the runtime (#4863)
  sp-core: Improve docs generated by `generate_feature_enabled_macro` (#4968)
  [Backport] Version bumps  and  prdocs reordering from 1.14.0 (#4955)
  Assets: can_decrease/increase for destroying asset is not successful (#3286)
  ...
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…aritytech#3286)

Functions `can_decrease` and `can_increase` do not return successful
consequence results for assets undergoing destruction; instead, they
return the `UnknownAsset` consequence variant.

This update aligns their behavior with similar functions, such as
`reducible_balance`, `increase_balance`, `decrease_balance`, and `burn`,
which return an `AssetNotLive` error for assets in the process of being
destroyed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

7 participants