linera-protocol
linera-protocol copied to clipboard
`linera_views::Context`: extract `KeyValueStore` functions
Motivation
The Context trait currently duplicates the ReadableKeyValue and WritableKeyValue traits.
Proposal
Instead, provide direct access to the store so there's no need to pass through these calls.
While we're at it, factor BaseKey out into a separate type for the same reason.
Additionally, I noticed we've maintained the Local/Send split for our traits, a distinction that we chose to abandon in the codebase in favour of conditional compilation, so I merged those. This led to some knock-on effects:
- we get to remove
async-traitin a lot of places, replacing its trait transformation functionality withtrait-variantwhere necessary — this simplifies the implementation of the traits, since implementers don't need to re-iterate theSendness of the trait - since we can now no longer implement things for
*Storewithout implementing them forLocal*Store, we need to be more careful about the propagation ofSend/Synctraits. Notably this required making GraphQL support conditional, which could even be extended to a feature flag in the future (just by modifyingCargo.tomlandbuild.rs, since I made use of thewith_*alias pattern).
Test Plan
CI.
Release Plan
- Nothing to do / These changes follow the usual release cycle.
Links
You're right, getting tests passing resulted in a few more changes than originally intended :sweat_smile: I'll update the description. (The Cargo.toml changes were I think a rebase artefact — oops!)
You're right, getting tests passing resulted in a few more changes than originally intended 😅 I'll update the description. (The
Cargo.tomlchanges were I think a rebase artefact — oops!)
To be clear, removing unused crates is an unconditional positive. I just would like it to be documented.
To be clear, removing unused crates is an unconditional positive. I just would like it to be documented.
Done! I think this may have happened when I ran cargo machete on the workspace by accident, but I guess since CI is happy they weren't necessary.
Looks great! But why are so many snap files getting deleted? Were these unused all along or did some tests get disabled due to conditional compilation?
They were unused all along. I realized by accident when in order to reset the snapshots I deleted them and then reran the tests. The only feature flag that affects these snapshots is metrics. When we change the snapshot name in the code (which can happen by accident, since it includes function name and type information) we currently have no means to automatically remove or highlight for removal the snapshots under the old name.