Skip to content

Remove async-trait from EncryptionService in favour of native async fn in traits (RPITIT) #401

Description

@coderdan

Context

EncryptionService (packages/cipherstash-proxy/src/proxy/mod.rs:152) is declared with #[async_trait::async_trait]. Native async fn in traits stabilised in Rust 1.75 (Dec 2023); the proxy builds on a much newer toolchain (currently 1.92) with no pinned MSRV, so the async-trait crate is no longer required by the language.

This is a code-hygiene follow-up surfaced while adding the BUG-300 regression test (#400 / PR #395) — not a blocker for that PR.

Why it isn't a trivial deletion

The encrypt/decrypt futures are driven inside a per-connection handler spawned on the multithreaded tokio runtime (main.rs tracker.spawn(...)pg::handler().awaitbackend.rs:488 self.context.decrypt(...).await). For that spawned task to be Send, the futures held across .await must be Send.

#[async_trait] desugars each method to Pin<Box<dyn Future + Send + '_>>, so it injects the Send bound for free. **Bare native async fn in a trait does not guarantee a Send future** through a generic S: EncryptionService, so a naive removal will fail to compile at the spawn` boundary.

Proposed approach

Convert to RPITIT with an explicit Send bound on the trait methods:

pub trait EncryptionService: Send + Sync {
    fn encrypt(&self, /* … */)
        -> impl std::future::Future<Output = Result<Vec<Option<EqlCiphertext>>, Error>> + Send;

    fn decrypt(&self, /* … */)
        -> impl std::future::Future<Output = Result<Vec<Option<Plaintext>>, Error>> + Send;
}

Impls can keep async fn as long as their concrete future is Send. (trait-variant's #[trait_variant::make(Send)] is an alternative that reduces boilerplate.)

This is viable because there is no dyn EncryptionService anywhere — all usage is generic/monomorphised — so RPITIT's lack of dyn-compatibility is not a blocker. (There is org precedent for dropping async-trait in another crate.)

Sites to update

  • Trait definition: packages/cipherstash-proxy/src/proxy/mod.rs:151-152
  • Production impl: packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs:150 (ZeroKms)
  • Test impls: packages/cipherstash-proxy/src/postgresql/context/mod.rs:1073 and packages/cipherstash-proxy/src/postgresql/backend.rs:745 (TestService)
  • Drop the dependency: async-trait = "0.1" in packages/cipherstash-proxy/Cargo.toml (only if no other #[async_trait] users remain)

Acceptance criteria

  • EncryptionService no longer uses #[async_trait]; spawned connection handling still compiles (Send preserved).
  • async-trait removed from Cargo.toml if it has no remaining users.
  • cargo fmt --check, cargo clippy --all-features --tests -- -D warnings, and the test suite all pass.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions