Unbox the Signer (#4005)

## Motivation

As a stopgap measure, when @deuszx first implemented the `Signer` trait,
he passed it as a trait object, boxing all its occurrences and
associated types. Now that we have
#3828, this is no
longer necessary, and will make implementing it for non-`Send` types
harder, as each boxing site will need to decide whether the resulting
type should be `Box<dyn Signer>` or `Box<dyn Signer + Send>`.

## Proposal

Unbox it, passing it in as a type parameter through the
`linera_core::Environment` trait.

## Test Plan

CI.

## Release Plan

- Nothing to do / These changes follow the usual release cycle.

## Links

- [reviewer
checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)

Author: Twey

Cargo.lock

@@ -63,6 +63,7 @@ tokio = { workspace = true, features = ["time"] }
 tokio-stream.workspace = true
 tracing.workspace = true
 tracing-subscriber = { workspace = true, features = ["json", "fmt", "ansi"] }
+trait-variant.workspace = true
 wasm-bindgen = { workspace = true, optional = true }
 wasm-bindgen-futures = { workspace = true, optional = true }
 wasm_thread = { workspace = true, optional = true }

examples/Cargo.lock

@@ -1,7 +1,6 @@
 // Copyright (c) Zefchain Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use async_trait::async_trait;
 pub use in_mem::InMemorySigner;
 
 use super::CryptoHash;
@@ -11,8 +10,11 @@ use crate::{
 };
 
 /// A trait for signing keys.
-#[async_trait]
-pub trait Signer: Send + Sync {
+#[cfg_attr(not(web), trait_variant::make(Send))]
+pub trait Signer {
+    /// The type of errors arising from operations on this `Signer`.
+    type Error: std::error::Error + Send + Sync + 'static;
+
     /// Creates a signature for the given `value` using the provided `owner`.
     // DEV: We sign `CryptoHash` type, rather than `&[u8]` to make sure we don't sign
     // things accidentally. See [`CryptoHash::new`] for how the type's name is included
@@ -21,38 +23,13 @@ pub trait Signer: Send + Sync {
         &self,
         owner: &AccountOwner,
         value: &CryptoHash,
-    ) -> Result<AccountSignature, Box<dyn std::error::Error>>;
+    ) -> Result<AccountSignature, Self::Error>;
 
     /// Returns the public key corresponding to the given `owner`.
-    async fn get_public_key(
-        &self,
-        owner: &AccountOwner,
-    ) -> Result<AccountPublicKey, Box<dyn std::error::Error>>;
+    async fn get_public_key(&self, owner: &AccountOwner) -> Result<AccountPublicKey, Self::Error>;
 
     /// Returns whether the given `owner` is a known signer.
-    async fn contains_key(&self, owner: &AccountOwner) -> Result<bool, Box<dyn std::error::Error>>;
-}
-
-#[async_trait]
-impl Signer for Box<dyn Signer> {
-    async fn sign(
-        &self,
-        owner: &AccountOwner,
-        value: &CryptoHash,
-    ) -> Result<AccountSignature, Box<dyn std::error::Error>> {
-        (**self).sign(owner, value).await
-    }
-
-    async fn get_public_key(
-        &self,
-        owner: &AccountOwner,
-    ) -> Result<AccountPublicKey, Box<dyn std::error::Error>> {
-        (**self).get_public_key(owner).await
-    }
-
-    async fn contains_key(&self, owner: &AccountOwner) -> Result<bool, Box<dyn std::error::Error>> {
-        (**self).contains_key(owner).await
-    }
+    async fn contains_key(&self, owner: &AccountOwner) -> Result<bool, Self::Error>;
 }
 
 /// In-memory implementation of the [`Signer`] trait.
@@ -62,7 +39,6 @@ mod in_mem {
         sync::{Arc, RwLock},
     };
 
-    use async_trait::async_trait;
     use serde::{Deserialize, Serialize};
 
     #[cfg(with_getrandom)]
@@ -72,6 +48,12 @@ mod in_mem {
         identifiers::AccountOwner,
     };
 
+    #[derive(Debug, thiserror::Error)]
+    pub enum Error {
+        #[error("no key found for the given owner")]
+        NoSuchOwner,
+    }
+
     /// In-memory signer.
     #[derive(Clone)]
     pub struct InMemorySigner(Arc<RwLock<InMemSignerInner>>);
@@ -173,42 +155,39 @@ mod in_mem {
         }
     }
 
-    #[async_trait]
     impl Signer for InMemorySigner {
+        type Error = Error;
+
         /// Creates a signature for the given `value` using the provided `owner`.
         async fn sign(
             &self,
             owner: &AccountOwner,
             value: &CryptoHash,
-        ) -> Result<AccountSignature, Box<dyn std::error::Error>> {
+        ) -> Result<AccountSignature, Error> {
             let inner = self.0.read().unwrap();
             if let Some(secret) = inner.keys.get(owner) {
                 let signature = secret.sign_prehash(*value);
                 Ok(signature)
             } else {
-                Err("No key found for the given owner".into())
+                Err(Error::NoSuchOwner)
             }
         }
 
         /// Returns the public key corresponding to the given `owner`.
-        async fn get_public_key(
-            &self,
-            owner: &AccountOwner,
-        ) -> Result<AccountPublicKey, Box<dyn std::error::Error>> {
-            let inner = self.0.read().unwrap();
-            match inner.keys.get(owner).map(|s| s.public()) {
-                Some(public) => Ok(public),
-                None => Err("No key found for the given owner".into()),
-            }
+        async fn get_public_key(&self, owner: &AccountOwner) -> Result<AccountPublicKey, Error> {
+            Ok(self
+                .0
+                .read()
+                .unwrap()
+                .keys
+                .get(owner)
+                .ok_or(Error::NoSuchOwner)?
+                .public())
         }
 
         /// Returns whether the given `owner` is a known signer.
-        async fn contains_key(
-            &self,
-            owner: &AccountOwner,
-        ) -> Result<bool, Box<dyn std::error::Error>> {
-            let inner = self.0.read().unwrap();
-            Ok(inner.keys.contains_key(owner))
+        async fn contains_key(&self, owner: &AccountOwner) -> Result<bool, Error> {
+            Ok(self.0.read().unwrap().keys.contains_key(owner))
         }
     }
 

linera-base/Cargo.toml

@@ -7,6 +7,7 @@
 
 #![deny(missing_docs)]
 #![deny(clippy::large_futures)]
+#![allow(async_fn_in_trait)]
 
 use std::fmt;
 

linera-base/src/crypto/signer.rs

@@ -2,10 +2,7 @@
 // Copyright (c) Zefchain Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use std::{
-    collections::{BTreeMap, BTreeSet, HashSet},
-    error::Error,
-};
+use std::collections::{BTreeMap, BTreeSet, HashSet};
 
 use async_graphql::SimpleObject;
 use custom_debug_derive::Debug;
@@ -501,12 +498,12 @@ pub struct ProposalContent {
 }
 
 impl BlockProposal {
-    pub async fn new_initial(
+    pub async fn new_initial<S: Signer + ?Sized>(
         owner: AccountOwner,
         round: Round,
         block: ProposedBlock,
-        signer: &(impl Signer + ?Sized),
-    ) -> Result<Self, Box<dyn Error>> {
+        signer: &S,
+    ) -> Result<Self, S::Error> {
         let content = ProposalContent {
             round,
             block,
@@ -523,12 +520,12 @@ impl BlockProposal {
         })
     }
 
-    pub async fn new_retry_fast(
+    pub async fn new_retry_fast<S: Signer + ?Sized>(
         owner: AccountOwner,
         round: Round,
         old_proposal: BlockProposal,
-        signer: &(impl Signer + ?Sized),
-    ) -> Result<Self, Box<dyn Error>> {
+        signer: &S,
+    ) -> Result<Self, S::Error> {
         let content = ProposalContent {
             round,
             block: old_proposal.content.block,
@@ -548,12 +545,12 @@ impl BlockProposal {
         })
     }
 
-    pub async fn new_retry_regular(
+    pub async fn new_retry_regular<S: Signer>(
         owner: AccountOwner,
         round: Round,
         validated_block_certificate: ValidatedBlockCertificate,
-        signer: &(impl Signer + ?Sized),
-    ) -> Result<Self, Box<dyn Error>> {
+        signer: &S,
+    ) -> Result<Self, S::Error> {
         let certificate = validated_block_certificate.lite_certificate().cloned();
         let block = validated_block_certificate.into_inner().into_inner();
         let (block, outcome) = block.into_proposal();

linera-base/src/lib.rs

@@ -83,22 +83,22 @@ pub trait BlockTestExt: Sized {
 
     /// Returns a block proposal in the first round in a default ownership configuration
     /// (`Round::MultiLeader(0)`) without any hashed certificate values or validated block.
-    async fn into_first_proposal(
+    async fn into_first_proposal<S: Signer + ?Sized>(
         self,
         owner: AccountOwner,
-        signer: &(impl Signer + ?Sized),
-    ) -> Result<BlockProposal, Box<dyn std::error::Error>> {
+        signer: &S,
+    ) -> Result<BlockProposal, S::Error> {
         self.into_proposal_with_round(owner, signer, Round::MultiLeader(0))
             .await
     }
 
     /// Returns a block proposal without any hashed certificate values or validated block.
-    async fn into_proposal_with_round(
+    async fn into_proposal_with_round<S: Signer + ?Sized>(
         self,
         owner: AccountOwner,
-        signer: &(impl Signer + ?Sized),
+        signer: &S,
         round: Round,
-    ) -> Result<BlockProposal, Box<dyn std::error::Error>>;
+    ) -> Result<BlockProposal, S::Error>;
 }
 
 impl BlockTestExt for ProposedBlock {
@@ -147,12 +147,12 @@ impl BlockTestExt for ProposedBlock {
         self
     }
 
-    async fn into_proposal_with_round(
+    async fn into_proposal_with_round<S: Signer + ?Sized>(
         self,
         owner: AccountOwner,
-        signer: &(impl Signer + ?Sized),
+        signer: &S,
         round: Round,
-    ) -> Result<BlockProposal, Box<dyn std::error::Error>> {
+    ) -> Result<BlockProposal, S::Error> {
         BlockProposal::new_initial(owner, round, self, signer).await
     }
 }

linera-chain/src/data_types.rs

@@ -7,7 +7,7 @@ use std::{collections::HashSet, sync::Arc};
 
 use futures::Future;
 use linera_base::{
-    crypto::{CryptoHash, Signer, ValidatorPublicKey},
+    crypto::{CryptoHash, ValidatorPublicKey},
     data_types::{BlockHeight, Timestamp},
     identifiers::{Account, AccountOwner, ChainId},
     ownership::ChainOwnership,
@@ -126,17 +126,13 @@ where
     }
 }
 
-impl<S, W> ClientContext<linera_core::environment::Impl<S, NodeProvider>, W>
+impl<S, Si, W> ClientContext<linera_core::environment::Impl<S, NodeProvider, Si>, W>
 where
     S: linera_core::environment::Storage,
+    Si: linera_core::environment::Signer,
     W: Persist<Target = Wallet>,
 {
-    pub fn new(
-        storage: S,
-        options: ClientContextOptions,
-        wallet: W,
-        signer: Box<dyn Signer>,
-    ) -> Self {
+    pub fn new(storage: S, options: ClientContextOptions, wallet: W, signer: Si) -> Self {
         let node_provider = NodeProvider::new(NodeOptions {
             send_timeout: options.send_timeout,
             recv_timeout: options.recv_timeout,
@@ -154,8 +150,8 @@ where
             linera_core::environment::Impl {
                 network: node_provider,
                 storage,
+                signer,
             },
-            signer,
             options.max_pending_message_bundles,
             wallet.genesis_admin_chain(),
             delivery,
@@ -181,7 +177,7 @@ where
     }
 
     #[cfg(with_testing)]
-    pub fn new_test_client_context(storage: S, wallet: W, signer: Box<dyn Signer>) -> Self {
+    pub fn new_test_client_context(storage: S, wallet: W, signer: Si) -> Self {
         use linera_core::DEFAULT_GRACE_PERIOD;
 
         let send_recv_timeout = Duration::from_millis(4000);
@@ -205,8 +201,8 @@ where
             linera_core::environment::Impl {
                 storage,
                 network: NodeProvider::new(node_options),
+                signer,
             },
-            signer,
             10,
             wallet.genesis_admin_chain(),
             delivery,

linera-chain/src/test/mod.rs

@@ -104,7 +104,7 @@ async fn test_chain_listener() -> anyhow::Result<()> {
     let config = ChainListenerConfig::default();
     let storage_builder = MemoryStorageBuilder::default();
     let clock = storage_builder.clock().clone();
-    let mut builder = TestBuilder::new(storage_builder, 4, 1, &mut signer).await?;
+    let mut builder = TestBuilder::new(storage_builder, 4, 1, signer.clone()).await?;
     let client0 = builder.add_root_chain(0, Amount::ONE).await?;
     let chain_id0 = client0.chain_id();
     let client1 = builder.add_root_chain(1, Amount::ONE).await?;
@@ -120,8 +120,8 @@ async fn test_chain_listener() -> anyhow::Result<()> {
             environment::Impl {
                 storage: storage.clone(),
                 network: builder.make_node_provider(),
+                signer,
             },
-            Box::new(signer),
             10,
             admin_id,
             delivery,

linera-client/src/client_context.rs

@@ -26,7 +26,7 @@ async fn test_save_wallet_with_pending_blobs() -> anyhow::Result<()> {
     let mut signer = InMemorySigner::new(Some(42));
     let new_pubkey = signer.generate_new();
     let clock = storage_builder.clock().clone();
-    let mut builder = TestBuilder::new(storage_builder, 4, 1, &mut signer).await?;
+    let mut builder = TestBuilder::new(storage_builder, 4, 1, signer.clone()).await?;
     builder.add_root_chain(0, Amount::ONE).await?;
     let chain_id = builder.admin_id();
     let storage = builder.make_storage().await?;
@@ -66,7 +66,7 @@ async fn test_save_wallet_with_pending_blobs() -> anyhow::Result<()> {
         },
         blobs: vec![Blob::new_data(b"blob".to_vec())],
     });
-    let mut context = ClientContext::new_test_client_context(storage, wallet, Box::new(signer));
+    let mut context = ClientContext::new_test_client_context(storage, wallet, signer);
     context.save_wallet().await?;
     Ok(())
 }