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

Use RuntimeHoldReason for the NIS pallet HoldReason #7017

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

KiChjang
Copy link
Contributor

@KiChjang KiChjang commented Apr 6, 2023

Partial paritytech/polkadot-sdk#236.

Removes the user-defined enums on the runtime source file and instead uses the macro-generated RuntimeHoldReason for the NIS pallet's HoldReason.

@KiChjang KiChjang 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Apr 6, 2023
@paritytech-ci paritytech-ci requested review from a team April 6, 2023 11:28
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Still need to use RuntimeHoldReason in the balances pallet.

/// The NIS Pallet has reserved it for a non-fungible receipt.
#[codec(index = 0)]
NftReceipt = 0,
pub const NisHoldReason: RuntimeHoldReason = RuntimeHoldReason::Nis(pallet_nis::HoldReason::NftReceipt);
Copy link
Member

Choose a reason for hiding this comment

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

We only need this for tests or?
So can we move it to the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to make sure that the encoding didn't change before and after the code changes in this PR. In the future, it would probably be good to keep this around so that we don't accidentally change the encoding via modifying the composite_enum attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I misread. We do need this to configure the HoldReason on the NIS pallet.

@paritytech-ci paritytech-ci requested a review from a team April 9, 2023 22:23
@bkchr bkchr merged commit f1a45b3 into master Apr 13, 2023
@bkchr bkchr deleted the kckyeung/use-runtime-hold-reason branch April 13, 2023 08:00
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants