Skip to content

feat(nfts + nonfungibles): sync polkadot stable2503#558

Merged
chungquantin merged 19 commits intomainfrom
chungquantin/sync-pallet-nfts-stable2503
May 8, 2025
Merged

feat(nfts + nonfungibles): sync polkadot stable2503#558
chungquantin merged 19 commits intomainfrom
chungquantin/sync-pallet-nfts-stable2503

Conversation

@chungquantin
Copy link
Contributor

@chungquantin chungquantin commented May 5, 2025

This PR syncs all changes from v1.15.0 to polkadot-stable2503 for the pallet-nfts and pallet-nonfugibles

Requires rebasing to this commit on main.

Related upstream commits:

Commit comments are identical to the commits in pallet-nfts | stable2503

@chungquantin chungquantin self-assigned this May 5, 2025
@chungquantin
Copy link
Contributor Author

[sc-3710]

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2025

Codecov Report

❌ Patch coverage is 9.05512% with 462 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.44%. Comparing base (8e5c719) to head (1bc0974).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
pallets/nfts/src/weights.rs 5.85% 450 Missing ⚠️
pallets/nfts/src/types.rs 0.00% 12 Missing ⚠️
@@            Coverage Diff             @@
##             main     #558      +/-   ##
==========================================
- Coverage   63.56%   63.44%   -0.13%     
==========================================
  Files         121      121              
  Lines       23801    23850      +49     
  Branches    23801    23850      +49     
==========================================
+ Hits        15129    15131       +2     
- Misses       7628     7675      +47     
  Partials     1044     1044              
Files with missing lines Coverage Δ
pallets/api/src/mock.rs 74.46% <ø> (ø)
pallets/api/src/nonfungibles/mod.rs 95.80% <ø> (ø)
pallets/api/src/nonfungibles/weights.rs 50.00% <ø> (ø)
pallets/nfts/src/features/approvals.rs 97.11% <100.00%> (ø)
pallets/nfts/src/features/atomic_swap.rs 90.84% <100.00%> (+0.06%) ⬆️
pallets/nfts/src/features/attributes.rs 86.97% <100.00%> (ø)
pallets/nfts/src/features/create_delete_item.rs 88.46% <100.00%> (ø)
pallets/nfts/src/features/settings.rs 87.67% <100.00%> (-0.65%) ⬇️
pallets/nfts/src/lib.rs 69.69% <100.00%> (ø)
pallets/nfts/src/mock.rs 93.75% <ø> (ø)
... and 4 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@al3mart al3mart force-pushed the al3mart/sync-polkadot-stable2503 branch from 06f0e43 to 6cb9ad9 Compare May 5, 2025 09:02
@chungquantin chungquantin force-pushed the chungquantin/sync-pallet-nfts-stable2503 branch from a3089ab to fd99b50 Compare May 5, 2025 11:06
@chungquantin chungquantin requested a review from a team May 5, 2025 13:21
Copy link
Member

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Awesome! It looks great!

I've left one comment to clarify if we really need BlockNumberProvider in the api.
Given that we are using
type BlockNumberFor<T> = pallet_nfts::BlockNumberFor<T, NftsInstanceOf<T>>; it seems that we could be good without adding a new type to the trait ?

Once that is clarified I'm happy to approve 👌

Daanvdplas
Daanvdplas previously approved these changes May 5, 2025
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Looking good, two nitpicks but nothing to stop this from going in.

al3mart
al3mart previously approved these changes May 6, 2025
Copy link
Member

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@r0gue-io r0gue-io locked and limited conversation to collaborators May 6, 2025
@r0gue-io r0gue-io unlocked this conversation May 6, 2025
Base automatically changed from al3mart/sync-polkadot-stable2503 to main May 7, 2025 12:13
@evilrobot-01 evilrobot-01 dismissed stale reviews from al3mart and Daanvdplas May 7, 2025 12:13

The base branch was changed.

@chungquantin chungquantin force-pushed the chungquantin/sync-pallet-nfts-stable2503 branch 2 times, most recently from 54bab63 to 2e0fb15 Compare May 7, 2025 12:35
@chungquantin chungquantin force-pushed the chungquantin/sync-pallet-nfts-stable2503 branch from 2e0fb15 to 21accad Compare May 7, 2025 12:43
@chungquantin chungquantin force-pushed the chungquantin/sync-pallet-nfts-stable2503 branch from 848b9f1 to 182486a Compare May 7, 2025 12:46
@chungquantin chungquantin requested review from Daanvdplas and al3mart May 7, 2025 12:48
Copy link
Member

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Looks great!

Only one nit. I believe there's some more cases (at least one) of frame_system::Pallet::<T>::block_number(); within benchmarking.rs that could be substituted for T::BlockNumberProvider::current_block_number();

Copy link
Member

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Meant to request changes 😅

@chungquantin
Copy link
Contributor Author

chore: benchmarking replace with BlockNumberProvider

Thanks Ale, great catch! I have resolved it in 986a3b4

Copy link
Member

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Awesome, @chungquantin! Thanks

@chungquantin chungquantin enabled auto-merge (squash) May 8, 2025 18:23
@Daanvdplas Daanvdplas self-requested a review May 8, 2025 18:28
@chungquantin chungquantin merged commit 83091b0 into main May 8, 2025
18 checks passed
@chungquantin chungquantin deleted the chungquantin/sync-pallet-nfts-stable2503 branch May 8, 2025 18:28
chungquantin added a commit that referenced this pull request May 16, 2025
* fix visibility for pallet_nfts types used as call arguments

Changes from the commit: paritytech/polkadot-sdk#3634

* [pallet-nfts, pallet_uniques] - Expose private structs

Changes made from upstream: paritytech/polkadot-sdk#6087

* Adds BlockNumberProvider in multisig, proxy and nft pallets

Upstream commit: paritytech/polkadot-sdk#5723

* Removed unused dependencies (partial progress)

Upstream commit: paritytech/polkadot-sdk#7329

* implement DecodeWithMemTracking for frame pallets

paritytech/polkadot-sdk#7598

* chore: bump pallet-nfts version to 34.1.0

* feat: adds BlockNumberProvider in nonfungbiles pallet

* chore: update lock file of pop-api to sync nfts

* chore: BlockNumberProvider for devnet nonfungibles

* chore: fix benchmarking to use new BlockNumberFor

* chore: revert changes

* fix: missing DecodeWithMemTracking

* chore: update weights of pallets

* refactor(nonfungibles): remove unused BlockNumberProvider type

* chore: remove unused Zero trait

* chore: revert non-relevant changes

* chore: revert non-relevant changes

This reverts commit 182486a.

* chore: benchmarking replace with BlockNumberProvider

* chore(pallet-nfts): update weights
@al3mart al3mart mentioned this pull request May 16, 2025
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants