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

Vision: #[pallet::constant(non_volatile)] #244

Closed
kianenigma opened this issue Oct 30, 2022 · 8 comments
Closed

Vision: #[pallet::constant(non_volatile)] #244

kianenigma opened this issue Oct 30, 2022 · 8 comments
Labels
I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@kianenigma
Copy link
Contributor

This came up in a discussion with @gpestana recently. There are a number of Get<_> associated types in a runtime that should not really change after being once set. If they are to be changed, we probably need a migration. For example, all of the type MaxSomething: Get<u32> might imply a migration if we reduce them.

The idea is to mark these with a special flag that says they should not change. I am not sure if non_volatile is a good word here. Then, in construct_runtime!, if we optionally provide either an encoded metadata path, or a websocket endpoint from which the existing metadata can be fetched, we can compare the values, and if anything has changed, we emit a warning.

sub-xt can be useful to do all of these.

@ggwpez
Copy link
Member

ggwpez commented Oct 30, 2022

Having volatile constants is a weird design to begin with. I think a constant should be constant by definition.
Maybe:

#[pallet::constant] // non-volatile
#[pallet::variable] // volatile

@kianenigma
Copy link
Contributor Author

totally agree, and taking it a step further, I think pallet::constant was bad to begin with because the name doesn't have much to do with the semantic, which is "put it in metadata".

#[pallet::metadat(constant)]
#[pallet::metadat(variable)]

@kianenigma
Copy link
Contributor Author

related: #1139

@kianenigma kianenigma changed the title idea: #[pallet::constant(non_volatile)] Vision: #[pallet::constant(non_volatile)] Mar 10, 2023
@juangirini
Copy link
Contributor

const fn could help us here

@bkchr
Copy link
Member

bkchr commented May 5, 2023

const fn could help us here

No it doesn't help here. You could still change the value between different releases. This is nothing that const fn protects against.

@kianenigma
Copy link
Contributor Author

Yeah correct. The right tool that would help us here is a metadata comparison tool.

@ggwpez
Copy link
Member

ggwpez commented Jun 20, 2023

This is mixing different meanings of the word "const".
Currently we do not support it. We could restrict it to only change on runtime upgrades. But hence since anything can change in a runtime upgrade, i think that is fine.

The problem with Get is that it can be implemented as a storage query, so it can arbitrarily change at any time. So I think using the Rust compile time notion of const does already help a lot for reasoning about things within a runtime.

Yeah correct. The right tool that would help us here is a metadata comparison tool.

This cannot guarantee anything within the runtime. It would only be helpful for the release process, to compare things (similar to subwasm diff). It does already detect constant changes in the metadata.

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed J0-enhancement labels Aug 25, 2023
@kianenigma
Copy link
Contributor Author

I will close this in favor of #1139. The metadata comparison (nested in construct_runtime) I proposed here, if one day implemented, would be interested in conjuncture with #218

serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
This PR updates litep2p to the latest release.

- `KademliaEvent::PutRecordSucess` is renamed to fix word typo
- `KademliaEvent::GetProvidersSuccess` and
`KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
and will be utilized later


### Added

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](paritytech/litep2p#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](paritytech/litep2p#246))
- kad: Providers part 6: stop providing
([#245](paritytech/litep2p#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](paritytech/litep2p#236))
- kad: Providers part 4: refresh local providers
([#235](paritytech/litep2p#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](paritytech/litep2p#234))

### Changed

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](paritytech/litep2p#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](paritytech/litep2p#255))
- kad/executor: Add timeout for writting frames
([#277](paritytech/litep2p#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](paritytech/litep2p#233))
- peer_state: Robust state machine transitions
([#251](paritytech/litep2p#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](paritytech/litep2p#250))
- kad: Remove unused serde cfg
([#262](paritytech/litep2p#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](paritytech/litep2p#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](paritytech/litep2p#254))

### Fixed

- tcp/websocket/quic: Fix cancel memory leak
([#272](paritytech/litep2p#272))
- transport: Fix pending dials memory leak
([#271](paritytech/litep2p#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](paritytech/litep2p#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](paritytech/litep2p#273))
- kad: Fix not retrieving local records
([#221](paritytech/litep2p#221))

See release changelog for more details:
https://github.com/paritytech/litep2p/releases/tag/v0.8.0

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Done
Development

No branches or pull requests

5 participants