SDK-client-UA-registration #34
Reference in New Issue
Block a user
Delete Branch "SDK-client-UA-registration"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
I have marked small issues by comments.
The bigger issue is design of
arbiter-client. You require client to supply private key together with target address and it's easy to confuse the two.Instead, I would do this:
ArbiterClientinstanceArbiterClientin turn could produceArbiterEvmWalletwith data fetched from server. this "wallet handle" structure would share the connection and implement relatedSignertraits@@ -80,6 +129,9 @@ message UserAgentRequest {arbiter.evm.EvmGrantDeleteRequest evm_grant_delete = 9;arbiter.evm.EvmGrantListRequest evm_grant_list = 10;ClientConnectionResponse client_connection_response = 11;SdkClientApproveRequest sdk_client_approve = 12;Rename remaining
Client*request-response pairs to matchSdk*prefix style@@ -13,0 +98,4 @@impl ArbiterSigner {pub async fn connect_grpc(url: ArbiterUrl,key: ed25519_dalek::SigningKey,introduce
Storageabstraction to store private key as file.On first start, it should be transparently created.
This aligns with online approval flow.
@@ -13,0 +99,4 @@pub async fn connect_grpc(url: ArbiterUrl,key: ed25519_dalek::SigningKey,address: Address,Address wrong. How could consumer know address beforehand?
What if it doesn't exist? Same goes for chain_id
@@ -13,0 +104,4 @@let anchor = webpki::anchor_from_trusted_cert(&url.ca_cert)?.to_owned();let tls = ClientTlsConfig::new().trust_anchor(anchor);// NOTE: We intentionally keep the same URL construction strategy as the user-agent crateobsolete comment
@@ -13,0 +112,4 @@.await?;let mut client = ArbiterServiceClient::new(channel);let (tx, rx) = mpsc::channel(16);move
16to const. Put it inarbiter-proto.@@ -13,0 +129,4 @@})}async fn sign_transaction_via_arbiter(Put this code directly in
TxSignerimpl?@@ -13,0 +154,4 @@)),};let mut transport = self.transport.lock().await;Generally, functions should do only one of two things: check something or do something.
I would have splitted transport flow (handshake?) into separate function
@@ -13,0 +216,4 @@.await.map_err(|_| ConnectError::UnexpectedAuthResponse)?;// Current server flow does not emit `AuthOk` for SDK clients, so we proceed afterlol, this should be fixed on server-side. Had same problem with user agent auth
@@ -64,0 +63,4 @@async fn get_nonce(db: &db::DatabasePool,pubkey: &VerifyingKey,) -> Result<Option<(i32, i32)>, Error> {I would create newtype for client-id here to not to confuse it with nonce
@@ -99,7 +99,6 @@ async fn request_client_approval(while let Some(result) = pool.join_next().await {match result {Ok(Ok(approved)) => {// cancel other pending requestsreturn comments back, please
@@ -31,2 +39,4 @@}impl TransportResponseError {pub fn is_terminal(&self) -> bool {https://lib.rs/crates/fatality
should be a better fit
@@ -334,0 +382,4 @@.execute(&mut conn).await;match insert_result {Sqlite supports
RETURNINGclauses on insert statement:https://docs.rs/diesel/latest/diesel/query_builder/struct.InsertStatement.html#method.returning
SDK-client-UA-registration + PoC-terrorsto SDK-client-UA-registration