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

Allow any error to be specified as the Err variant for ResultQuery #260

Open
KiChjang opened this issue Aug 24, 2022 · 5 comments
Open

Allow any error to be specified as the Err variant for ResultQuery #260

KiChjang opened this issue Aug 24, 2022 · 5 comments
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@KiChjang
Copy link
Contributor

KiChjang commented Aug 24, 2022

After paritytech/substrate#11257, we can now return a Result<T, E> for storage map APIs that return a QueryKind. The slight issue here is that the error type must be the pallet error type (i.e. the enum annotated with #[pallet::error]).

There's no good reason for this restriction other than ease of implementation, so I propose that we can in fact lift it and allow any type to be specified as the error variant for the Result returned.

However, maybe what really should happen is to quantify and allow any error that implements the PalletError trait to be specified as the Err variant for ResultQuery, because we may have size limitations on the ResultQuery type. It is in fact unknown to me whether the bounded size of ResultQuery actually matters; maybe we don't care and thus any type can be used as the Err variant.

@bkchr
Copy link
Member

bkchr commented Aug 24, 2022

It is in fact unknown to me whether the bounded size of ResultQuery actually matters

I don't get why this should be bounded? I mean the error at some point needs to be converted to something DispatchResult compatible anyway. However, this thing doesn't need to be bounded. We don't read the error from the storage.

@KiChjang
Copy link
Contributor Author

Okay, then we can just allow the Err variant to be of any type.

@shawntabrizi
Copy link
Member

Any type that implements Into<DispatchError>... what would be the use case for some other random error?

@KiChjang
Copy link
Contributor Author

There's no real reason to be restrictive on the kinds of types that we can allow for the Err variant, but it makes sense that we can only allow those that implement Into<DispatchError> to be specified.

Ultimately, the issue here is simply to allow for more error types other than the pallet error enum.

@ggwpez
Copy link
Member

ggwpez commented Aug 25, 2022

There could also be a default error in sp-primitives. Something like StorageItemNotFound.
Then the developer can use that or overwrite with a custom pallet error.

@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. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed J0-enhancement labels Aug 25, 2023
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 <alexandru.vasile@parity.io>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Backlog
Development

No branches or pull requests

6 participants