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

pallet_section not work for storage #5320

Closed
2 tasks done
open-junius opened this issue Aug 12, 2024 · 3 comments · Fixed by #6023
Closed
2 tasks done

pallet_section not work for storage #5320

open-junius opened this issue Aug 12, 2024 · 3 comments · Fixed by #6023
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@open-junius
Copy link

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

For the pallet which include many storage, we try to use the pallet_section to put some storage in separate file. but we get the compilation error as "error[E0425]: cannot find value entries in this scope".

Steps to reproduce

source code: https://github.com/opentensor/subtensor/tree/junius-try-storage-macro/pallets/subtensor

steps:

  1. git clone https://github.com/opentensor/subtensor
  2. cd subtensor
  3. git checkout junius-try-storage-macro
  4. cd pallets/subtensor
  5. cargo build
@open-junius open-junius added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Aug 12, 2024
@sam0x17
Copy link
Contributor

sam0x17 commented Aug 12, 2024

just wanted to bring some attention to this @gupnik / @kianenigma looks like storages don't work with pallet_section. I played around with it a little bit experimentally and I'm fairly certain it is an issue with pallet_section

@kianenigma
Copy link
Contributor

Thanks for the report!

@gupnik is OOO for the next 2 weeks, but I am sure he will attend to this when he's back.

@sam0x17 sam0x17 mentioned this issue Oct 8, 2024
Closed
@eagr
Copy link
Contributor

eagr commented Oct 11, 2024

https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/support/procedural/src/pallet/expand/storage.rs#L912-L914

looks like the entries in the #( #entries_builder )* expansion won't be able to reference the entries defined outside, which is expected with the proc macro hygiene

@eagr eagr mentioned this issue Oct 11, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 12, 2024
@eagr
Fix storage expansion in pallet section (#6023)
d1c115b 
fixes #5320 @sam0x17 @gupnik 

# Description

The issue could be confirmed with the added example. The cause is for
macro hygiene, `entries` in the `#( #entries_builder )*` expansion won't
be able to reference the `entries` defined outside. The solution here is
to allow the reference to be passed into the expansion with closure.

Or we could just switch to the unhygienic span with `quote::quote!`
instead such that `entries` will resolve to the "outer" definition.
@eagr eagr mentioned this issue Oct 13, 2024
Closed
2 tasks
Zebedeusz pushed a commit that referenced this issue Oct 14, 2024
@eagr @Zebedeusz
Fix storage expansion in pallet section (#6023)
1c72d71 
fixes #5320 @sam0x17 @gupnik 

# Description

The issue could be confirmed with the added example. The cause is for
macro hygiene, `entries` in the `#( #entries_builder )*` expansion won't
be able to reference the `entries` defined outside. The solution here is
to allow the reference to be passed into the expansion with closure.

Or we could just switch to the unhygienic span with `quote::quote!`
instead such that `entries` will resolve to the "outer" definition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix storage expansion in pallet section eagr/polkadot-sdk
4 participants
@sam0x17 @kianenigma @eagr @open-junius