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

Add AWM sign request #99

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/avalanche-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub mod txs;
pub mod units;
pub mod utils;
pub mod verify;

pub mod warp;
#[cfg(feature = "avalanchego")]
#[cfg_attr(docsrs, doc(cfg(feature = "avalanchego")))]
pub mod avalanchego;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
},
};
use tokio::sync::mpsc::Sender;

use crate::warp::WarpSignerClient_;
/// Vm describes the trait that all consensus VMs must implement.
///
/// ref. <https://pkg.go.dev/github.com/ava-labs/avalanchego/snow/engine/common#Vm>
Expand All @@ -29,7 +29,7 @@ pub trait CommonVm: AppHandler + Connector + Checkable {
type ChainHandler: Handle;
type StaticHandler: Handle;
type ValidatorState: validators::State;

type WarpSigner: WarpSignerClient_;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this type alias?

async fn initialize(
&mut self,
ctx: Option<Context<Self::ValidatorState>>,
Expand All @@ -40,6 +40,7 @@ pub trait CommonVm: AppHandler + Connector + Checkable {
to_engine: Sender<Message>,
fxs: &[Fx],
app_sender: Self::AppSender,
warp_signer: Self::WarpSigner,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just pass in the concrete type here. It's unclear if there would ever be any associated type other than SignerClient<Channel> that was ever used.

) -> Result<()>;
async fn set_state(&self, state: State) -> Result<()>;
async fn shutdown(&self) -> Result<()>;
Expand Down
5 changes: 4 additions & 1 deletion crates/avalanche-types/src/subnet/rpc/vm/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use prost::bytes::Bytes;
use semver::Version;
use tokio::sync::{broadcast, mpsc, RwLock};
use tonic::{Request, Response};
use crate::warp::client::WarpSignerClient;

pub struct Server<V> {
/// Underlying Vm implementation.
Expand Down Expand Up @@ -96,6 +97,7 @@ where
DatabaseManager = DatabaseManager,
AppSender = AppSenderClient,
ValidatorState = ValidatorStateClient,
WarpSigner=WarpSignerClient,
> + Send
+ Sync
+ 'static,
Expand Down Expand Up @@ -182,7 +184,7 @@ where
return tonic::Status::unknown("engine receiver closed unexpectedly");
}
});

let warp_signer = WarpSignerClient::new(client_conn.clone());
let mut inner_vm = self.vm.write().await;
inner_vm
.initialize(
Expand All @@ -194,6 +196,7 @@ where
tx_engine,
&[()],
AppSenderClient::new(client_conn.clone()),
warp_signer
)
.await
.map_err(|e| tonic::Status::unknown(e.to_string()))?;
Expand Down
50 changes: 50 additions & 0 deletions crates/avalanche-types/src/warp/client.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use std::io::{Error, ErrorKind, Read, Result};
use std::str::FromStr;

use crate::proto::pb::warp::{
signer_client,
SignRequest,
SignResponse,
};
use prost::bytes::Bytes;
use tonic::transport::Channel;
use crate::ids::Id;

#[derive(Clone)]
pub struct WarpSignerClient {
inner: signer_client::SignerClient<Channel>,
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of the wrapper here? Why not just use the SignerClient directly?


impl WarpSignerClient {
pub fn new(client_conn: Channel) -> Self {
Self {
inner: signer_client::SignerClient::new(client_conn)
.max_decoding_message_size(usize::MAX)
.max_encoding_message_size(usize::MAX),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are already the defaults

Suggested change
inner: signer_client::SignerClient::new(client_conn)
.max_decoding_message_size(usize::MAX)
.max_encoding_message_size(usize::MAX),
inner: signer_client::SignerClient::new(client_conn),

}
}
}

#[tonic::async_trait]
impl super::WarpSignerClient_ for WarpSignerClient {
async fn sign(&self,
network_id: u32,
source_chain_id: &str,
payload: &[u8]) -> Result<SignResponse> {
let mut client = self.inner.clone();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you have to create a new signer client every time you need to sign? The clone here should be explicit by the user (or they can wrap their channel in some kind of lock). This masks the internal functionality and doesn't give users the option to do things more efficiently.

let res = client
.sign(SignRequest {
network_id,
source_chain_id: Bytes::from(Id::from_str(source_chain_id).unwrap().to_vec()),
payload: Bytes::from(payload.to_vec()),
})
.await
.map_err(|e| {
Error::new(
ErrorKind::Other,
format!("sign failed: {:?}", e),
)
})?;
Ok(res.into_inner())
}
}
33 changes: 33 additions & 0 deletions crates/avalanche-types/src/warp/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
pub mod client;

use std::io::Result;
use crate::proto::warp::SignResponse;

#[tonic::async_trait]
pub trait WarpSignerClient_: Send + Sync + CloneBox {
async fn sign(
&self,
network_id: u32,
source_chain_id: &str,
payload: &[u8],
) -> Result<SignResponse>;
}

pub trait CloneBox {
fn clone_box(&self) -> Box<dyn WarpSignerClient_ + Send + Sync>;
}

impl<T> CloneBox for T
where
T: 'static + WarpSignerClient_ + Clone + Send + Sync,
{
fn clone_box(&self) -> Box<dyn WarpSignerClient_ + Send + Sync> {
Box::new(self.clone())
}
}

impl Clone for Box<dyn WarpSignerClient_ + Send + Sync> {
fn clone(&self) -> Box<dyn WarpSignerClient_ + Send + Sync> {
self.clone_box()
}
}
Loading