SDK-client-UA-registration #34

Merged
Skipper merged 21 commits from SDK-client-UA-registration into main 2026-03-22 11:11:11 +00:00
Member
No description provided.
CleverWild added 5 commits 2026-03-16 17:35:56 +00:00
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(PoC): terrors crate usage
Some checks failed
ci/woodpecker/pr/server-lint Pipeline failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-test Pipeline was successful
099f76166e
feat(poc): enhance SDK client error handling in user agent module
Some checks failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-test Pipeline was successful
a5a9bc73b0
CleverWild added 1 commit 2026-03-16 17:46:54 +00:00
fix(server): restore online client approval UX with sdk management
Some checks failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-test Pipeline was successful
c90af9c196
CleverWild added 1 commit 2026-03-17 18:42:45 +00:00
chore: remove invalidly committed PoC crate
Some checks failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline was successful
ci/woodpecker/pr/server-test Pipeline was successful
6f03ce4d1d
CleverWild added 1 commit 2026-03-17 18:45:02 +00:00
feat: compat migrations
Some checks failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline was successful
ci/woodpecker/pr/server-test Pipeline was successful
77c3babec7
Skipper requested changes 2026-03-18 21:11:23 +00:00
Skipper left a comment
Owner

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:

  • Connect stream in ArbiterClient instance
  • This ArbiterClient in turn could produce ArbiterEvmWallet with data fetched from server. this "wallet handle" structure would share the connection and implement related Signer traits
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: - Connect stream in `ArbiterClient` instance - This `ArbiterClient` in turn could produce `ArbiterEvmWallet` with data fetched from server. this "wallet handle" structure would share the connection and implement related `Signer` traits
@@ -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;
Owner

Rename remaining Client* request-response pairs to match Sdk* prefix style

Rename remaining `Client*` request-response pairs to match `Sdk*` prefix style
CleverWild marked this conversation as resolved
@@ -13,0 +98,4 @@
impl ArbiterSigner {
pub async fn connect_grpc(
url: ArbiterUrl,
key: ed25519_dalek::SigningKey,
Owner

introduce Storage abstraction to store private key as file.
On first start, it should be transparently created.

This aligns with online approval flow.

introduce `Storage` abstraction to store private key as file. On first start, it should be transparently created. This aligns with online approval flow.
CleverWild marked this conversation as resolved
@@ -13,0 +99,4 @@
pub async fn connect_grpc(
url: ArbiterUrl,
key: ed25519_dalek::SigningKey,
address: Address,
Owner

Address wrong. How could consumer know address beforehand?
What if it doesn't exist? Same goes for chain_id

Address wrong. How could consumer know address beforehand? What if it doesn't exist? Same goes for chain_id
CleverWild marked this conversation as resolved
@@ -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 crate
Owner

obsolete comment

obsolete comment
CleverWild marked this conversation as resolved
@@ -13,0 +112,4 @@
.await?;
let mut client = ArbiterServiceClient::new(channel);
let (tx, rx) = mpsc::channel(16);
Owner

move 16 to const. Put it in arbiter-proto.

move `16` to const. Put it in `arbiter-proto`.
CleverWild marked this conversation as resolved
@@ -13,0 +129,4 @@
})
}
async fn sign_transaction_via_arbiter(
Owner

Put this code directly in TxSigner impl?

Put this code directly in `TxSigner` impl?
CleverWild marked this conversation as resolved
@@ -13,0 +154,4 @@
)),
};
let mut transport = self.transport.lock().await;
Owner

Generally, functions should do only one of two things: check something or do something.
I would have splitted transport flow (handshake?) into separate function

Generally, functions should do only one of two things: check something or do something. I would have splitted transport flow (handshake?) into separate function
CleverWild marked this conversation as resolved
@@ -13,0 +216,4 @@
.await
.map_err(|_| ConnectError::UnexpectedAuthResponse)?;
// Current server flow does not emit `AuthOk` for SDK clients, so we proceed after
Owner

lol, this should be fixed on server-side. Had same problem with user agent auth

lol, this should be fixed on server-side. Had same problem with user agent auth
CleverWild marked this conversation as resolved
@@ -64,0 +63,4 @@
async fn get_nonce(
db: &db::DatabasePool,
pubkey: &VerifyingKey,
) -> Result<Option<(i32, i32)>, Error> {
Owner

I would create newtype for client-id here to not to confuse it with nonce

I would create newtype for client-id here to not to confuse it with nonce
CleverWild marked this conversation as resolved
@@ -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 requests
Owner

return comments back, please

return comments back, please
CleverWild marked this conversation as resolved
@@ -31,2 +39,4 @@
}
impl TransportResponseError {
pub fn is_terminal(&self) -> bool {
Owner

https://lib.rs/crates/fatality
should be a better fit

https://lib.rs/crates/fatality should be a better fit
CleverWild marked this conversation as resolved
@@ -334,0 +382,4 @@
.execute(&mut conn)
.await;
match insert_result {
Owner
Sqlite supports `RETURNING` clauses on insert statement: https://docs.rs/diesel/latest/diesel/query_builder/struct.InsertStatement.html#method.returning
CleverWild marked this conversation as resolved
Skipper changed title from SDK-client-UA-registration + PoC-terrors to SDK-client-UA-registration 2026-03-18 23:33:18 +00:00
CleverWild added 9 commits 2026-03-19 18:09:18 +00:00
CleverWild added 1 commit 2026-03-21 20:14:54 +00:00
Merge remote-tracking branch 'origin/main' into SDK-client-UA-registration
Some checks failed
ci/woodpecker/pr/server-audit Pipeline failed
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-lint Pipeline was successful
ci/woodpecker/pr/server-test Pipeline was successful
f015d345f4
CleverWild requested review from Skipper 2026-03-21 20:38:31 +00:00
Skipper added 1 commit 2026-03-21 23:10:58 +00:00
chore(deps): update Rust dependencies and add cargo-edit
Some checks failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-lint Pipeline was successful
ci/woodpecker/pr/server-test Pipeline was successful
e135519c06
Skipper added 2 commits 2026-03-22 11:06:07 +00:00
refactor(client): redesign of wallet handle
Some checks failed
ci/woodpecker/pr/server-audit Pipeline was successful
ci/woodpecker/pr/server-lint Pipeline was successful
ci/woodpecker/pr/server-vet Pipeline failed
ci/woodpecker/pr/server-test Pipeline was successful
eb37ee0a0c
Skipper merged commit 2148faa376 into main 2026-03-22 11:11:11 +00:00
Skipper deleted branch SDK-client-UA-registration 2026-03-22 11:11:11 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: MarketTakers/arbiter#34