Skip to content

Commit 717390e

Browse files
JasonVranekJasonVranek
committed
address review comments
1 parent 4afbf35 commit 717390e

6 files changed

Lines changed: 81 additions & 104 deletions

File tree

‎crates/common/src/config/mod.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,12 @@ impl CommitBoostConfig {
137137
})
138138
}
139139

140-
/// Helper to return if signer uses TLS
141140
pub fn signer_uses_tls(&self) -> bool {
142141
self.signer
143142
.as_ref()
144143
.is_some_and(|signer_config| matches!(signer_config.tls_mode, TlsMode::Certificate(_)))
145144
}
146145

147-
/// Helper to return signer's server URL
148146
pub fn signer_server_url(&self, default_port: u16) -> String {
149147
if let Some(SignerConfig { inner: SignerType::Remote { url }, .. }) = &self.signer {
150148
url.to_string()
@@ -155,7 +153,6 @@ impl CommitBoostConfig {
155153
}
156154
}
157155

158-
/// Helper to return the path to the signer's TLS certificates if any
159156
pub fn signer_certs_path(&self) -> Option<&PathBuf> {
160157
self.signer
161158
.as_ref()

‎crates/common/src/config/signer.rs‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,10 @@ pub struct ModuleSigningConfig {
4646

4747
impl ModuleSigningConfig {
4848
pub fn validate(&self) -> Result<()> {
49-
// Ensure the JWT secret is not empty
5049
if self.jwt_secret.is_empty() {
5150
bail!("JWT secret cannot be empty");
5251
}
5352

54-
// Ensure the signing ID is a valid B256
5553
if self.signing_id.is_zero() {
5654
bail!("Signing ID cannot be zero");
5755
}
@@ -249,7 +247,6 @@ impl StartSignerConfig {
249247

250248
let (admin_secret, jwt_secrets) = load_jwt_secrets()?;
251249

252-
// Load the module signing configs
253250
let mod_signing_configs = load_module_signing_configs(&config, &jwt_secrets)
254251
.wrap_err("Failed to load module signing configs")?;
255252

@@ -380,22 +377,18 @@ pub fn load_module_signing_configs(
380377
let mut seen_jwt_secrets = HashMap::new();
381378
let mut seen_signing_ids = HashMap::new();
382379
for module in modules {
383-
// Validate the module ID
384380
ensure!(!module.id.is_empty(), "Module ID cannot be empty");
385381

386-
// Make sure it hasn't been used yet
387382
ensure!(
388383
!mod_signing_configs.contains_key(&module.id),
389384
"Duplicate module config detected: ID {} is already used",
390385
module.id
391386
);
392387

393-
// Make sure the JWT secret is present
394388
let jwt_secret = match jwt_secrets.get(&module.id) {
395389
Some(secret) => secret.clone(),
396390
None => bail!("JWT secret for module {} is missing", module.id),
397391
};
398-
// Create the module signing config and validate it
399392
let module_signing_config = ModuleSigningConfig {
400393
module_name: module.id.clone(),
401394
jwt_secret,
@@ -405,7 +398,6 @@ pub fn load_module_signing_configs(
405398
.validate()
406399
.wrap_err(format!("Invalid signing config for module {}", module.id))?;
407400

408-
// Check for duplicates in JWT secrets and signing IDs
409401
if let Some(existing_module) =
410402
seen_jwt_secrets.insert(module_signing_config.jwt_secret.clone(), &module.id)
411403
{
@@ -441,7 +433,7 @@ mod tests {
441433

442434
fn make_local_signer_config(tls_mode: TlsMode) -> SignerConfig {
443435
SignerConfig {
444-
host: Ipv4Addr::new(127, 0, 0, 1),
436+
host: Ipv4Addr::LOCALHOST,
445437
port: 20000,
446438
docker_image: SIGNER_IMAGE_DEFAULT.to_string(),
447439
jwt_auth_fail_limit: 3,
@@ -468,7 +460,7 @@ mod tests {
468460
pbs: StaticPbsConfig {
469461
docker_image: String::from("cb-fake-repo/fake-cb:latest"),
470462
pbs_config: PbsConfig {
471-
host: Ipv4Addr::new(127, 0, 0, 1),
463+
host: Ipv4Addr::LOCALHOST,
472464
port: 0,
473465
relay_check: false,
474466
wait_all_registrations: false,

‎crates/common/src/utils.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ pub fn decode_admin_jwt(jwt: Jwt) -> eyre::Result<JwtAdminClaims> {
419419
Ok(claims)
420420
}
421421

422-
/// Validate a JWT with the given secret
423422
pub fn validate_jwt(
424423
jwt: Jwt,
425424
secret: &str,
@@ -457,7 +456,6 @@ pub fn validate_jwt(
457456
Ok(())
458457
}
459458

460-
/// Validate an admin JWT with the given secret
461459
pub fn validate_admin_jwt(
462460
jwt: Jwt,
463461
secret: &str,

‎crates/signer/src/manager/dirk.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,9 @@ impl DirkManager {
372372
pub async fn generate_proxy_key(
373373
&mut self,
374374
module: &ModuleId,
375-
consensus: BlsPublicKey,
375+
consensus: &BlsPublicKey,
376376
) -> Result<SignedProxyDelegation<BlsPublicKey>, SignerModuleError> {
377-
let proxy_account = match self.consensus_accounts.get(&consensus) {
377+
let proxy_account = match self.consensus_accounts.get(consensus) {
378378
Some(Account::Simple(account)) => {
379379
self.generate_simple_proxy_account(account, module).await?
380380
}
@@ -393,7 +393,7 @@ impl DirkManager {
393393
proxy: proxy_account.inner.public_key().clone(),
394394
};
395395
let delegation_signature =
396-
self.request_consensus_signature(&consensus, &message.tree_hash_root(), None).await?;
396+
self.request_consensus_signature(consensus, &message.tree_hash_root(), None).await?;
397397

398398
let delegation = SignedProxyDelegation { message, signature: delegation_signature };
399399

‎crates/signer/src/manager/local.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ impl LocalSigningManager {
9494
pub async fn create_proxy_bls(
9595
&mut self,
9696
module_id: ModuleId,
97-
delegator: BlsPublicKey,
97+
delegator: &BlsPublicKey,
9898
) -> Result<SignedProxyDelegationBls, SignerModuleError> {
9999
let signer = BlsSigner::new_random();
100100
let proxy_pubkey = signer.pubkey();
101101

102102
let message = ProxyDelegationBls { delegator: delegator.clone(), proxy: proxy_pubkey };
103-
let signature = self.sign_consensus(&delegator, &message.tree_hash_root(), None).await?;
103+
let signature = self.sign_consensus(delegator, &message.tree_hash_root(), None).await?;
104104
let delegation = SignedProxyDelegationBls { signature, message };
105105
let proxy_signer = BlsProxySigner { signer, delegation: delegation.clone() };
106106

@@ -113,13 +113,13 @@ impl LocalSigningManager {
113113
pub async fn create_proxy_ecdsa(
114114
&mut self,
115115
module_id: ModuleId,
116-
delegator: BlsPublicKey,
116+
delegator: &BlsPublicKey,
117117
) -> Result<SignedProxyDelegationEcdsa, SignerModuleError> {
118118
let signer = EcdsaSigner::new_random();
119119
let proxy_address = signer.address();
120120

121121
let message = ProxyDelegationEcdsa { delegator: delegator.clone(), proxy: proxy_address };
122-
let signature = self.sign_consensus(&delegator, &message.tree_hash_root(), None).await?;
122+
let signature = self.sign_consensus(delegator, &message.tree_hash_root(), None).await?;
123123
let delegation = SignedProxyDelegationEcdsa { signature, message };
124124
let proxy_signer = EcdsaProxySigner { signer, delegation: delegation.clone() };
125125

@@ -351,7 +351,7 @@ mod tests {
351351
let (mut signing_manager, consensus_pk) = init_signing_manager();
352352

353353
let signed_delegation =
354-
signing_manager.create_proxy_bls(MODULE_ID.clone(), consensus_pk).await.unwrap();
354+
signing_manager.create_proxy_bls(MODULE_ID.clone(), &consensus_pk).await.unwrap();
355355

356356
let validation_result = signed_delegation.validate(CHAIN);
357357

@@ -372,7 +372,7 @@ mod tests {
372372
let (mut signing_manager, consensus_pk) = init_signing_manager();
373373

374374
let mut signed_delegation =
375-
signing_manager.create_proxy_bls(MODULE_ID.clone(), consensus_pk).await.unwrap();
375+
signing_manager.create_proxy_bls(MODULE_ID.clone(), &consensus_pk).await.unwrap();
376376

377377
signed_delegation.signature = BlsSignature::test_random();
378378

@@ -386,7 +386,7 @@ mod tests {
386386
let (mut signing_manager, consensus_pk) = init_signing_manager();
387387

388388
let signed_delegation =
389-
signing_manager.create_proxy_bls(MODULE_ID.clone(), consensus_pk).await.unwrap();
389+
signing_manager.create_proxy_bls(MODULE_ID.clone(), &consensus_pk).await.unwrap();
390390
let proxy_pk = signed_delegation.message.proxy;
391391

392392
let data_root = B256::random();
@@ -433,7 +433,7 @@ mod tests {
433433
let (mut signing_manager, consensus_pk) = init_signing_manager();
434434

435435
let signed_delegation =
436-
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), consensus_pk).await.unwrap();
436+
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), &consensus_pk).await.unwrap();
437437

438438
let validation_result = signed_delegation.validate(CHAIN);
439439

@@ -454,7 +454,7 @@ mod tests {
454454
let (mut signing_manager, consensus_pk) = init_signing_manager();
455455

456456
let mut signed_delegation =
457-
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), consensus_pk).await.unwrap();
457+
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), &consensus_pk).await.unwrap();
458458

459459
signed_delegation.signature = BlsSignature::test_random();
460460

@@ -468,7 +468,7 @@ mod tests {
468468
let (mut signing_manager, consensus_pk) = init_signing_manager();
469469

470470
let signed_delegation =
471-
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), consensus_pk).await.unwrap();
471+
signing_manager.create_proxy_ecdsa(MODULE_ID.clone(), &consensus_pk).await.unwrap();
472472
let proxy_pk = signed_delegation.message.proxy;
473473

474474
let data_root = B256::random();

0 commit comments

Comments
 (0)