Skip to content

Commit a32b25b

Browse files
danwtclaude
andcommitted
claude: verify ISM signer for confirmation signatures
Add signature verification for confirmation requests to match deposit behavior. Previously, confirmations did not validate that the signature came from the expected ISM address, allowing misconfigured validators to return signatures from wrong keys. Changes: - Add verify_ism_signer() helper for DRY signature validation - Update get_deposit_sigs() to use the shared helper - Add signature verification to get_confirmation_sigs() - Add SignableProgressIndication::new() constructor for external use - Always validate (ISM addresses are always required for sorting) Also includes minor formatting fixes from cargo fmt. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d536c0b commit a32b25b

File tree

4 files changed

+129
-97
lines changed

4 files changed

+129
-97
lines changed

rust/main/chains/dymension-kaspa/src/providers/validators.rs

Lines changed: 91 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
use tonic::async_trait;
22

33
use hyperlane_core::{
4-
rpc_clients::BlockNumberGetter, ChainCommunicationError, ChainResult, Signature,
5-
SignedCheckpointWithMessageId, H160,
4+
rpc_clients::BlockNumberGetter, ChainCommunicationError, ChainResult, Signable, Signature,
5+
SignedCheckpointWithMessageId, SignedType, H160,
66
};
77

88
use bytes::Bytes;
99
use eyre::Result;
1010
use reqwest::StatusCode;
1111
use std::str::FromStr;
12-
use tracing::{error, info, warn};
12+
use tracing::{error, info};
1313

1414
use crate::ConnectionConf;
15+
use crate::SignableProgressIndication;
1516
use futures::stream::{FuturesUnordered, StreamExt};
1617
use std::sync::Arc;
1718
use std::time::Instant;
@@ -23,6 +24,55 @@ use crate::ops::{
2324
};
2425
use kaspa_wallet_pskt::prelude::Bundle;
2526

27+
/// Verifies that a signature was produced by the expected ISM address.
28+
/// Returns true if valid, false otherwise (with error logging).
29+
fn verify_ism_signer<T: Signable>(
30+
index: usize,
31+
host: &str,
32+
expected_address: &str,
33+
signed: &SignedType<T>,
34+
) -> bool {
35+
let expected_h160 = match H160::from_str(expected_address) {
36+
Ok(h) => h,
37+
Err(e) => {
38+
error!(
39+
validator = ?host,
40+
validator_index = index,
41+
expected_address = ?expected_address,
42+
error = ?e,
43+
"kaspa: invalid ISM address format"
44+
);
45+
return false;
46+
}
47+
};
48+
49+
match signed.recover() {
50+
Ok(recovered_signer) => {
51+
if recovered_signer != expected_h160 {
52+
error!(
53+
validator = ?host,
54+
validator_index = index,
55+
expected_signer = ?expected_h160,
56+
actual_signer = ?recovered_signer,
57+
"kaspa: signature verification failed - signer mismatch"
58+
);
59+
false
60+
} else {
61+
true
62+
}
63+
}
64+
Err(e) => {
65+
error!(
66+
validator = ?host,
67+
validator_index = index,
68+
error = ?e,
69+
"kaspa: signature recovery failed"
70+
);
71+
false
72+
}
73+
}
74+
}
75+
2676
#[derive(Clone)]
2777
pub struct ValidatorsClient {
2878
pub conf: ConnectionConf,
@@ -230,9 +280,7 @@ impl ValidatorsClient {
230280
) -> ChainResult<Vec<SignedCheckpointWithMessageId>> {
231281
let threshold = self.multisig_threshold_hub_ism();
232282
let client = self.http_client.clone();
233-
// Use ISM validators for deposit signatures
234283
let hosts = self.hosts_ism();
235-
// Extract ISM addresses from ISM validators for signature verification
236284
let expected_addresses: Vec<String> = self
237285
.validators_ism()
238286
.iter()
@@ -241,62 +289,15 @@ impl ValidatorsClient {
241289
let metrics = self.metrics.clone();
242290
let fxg = fxg.clone();
243291

244-
// Only validate signatures if ISM addresses are configured (non-empty)
245-
let has_ism_addresses = expected_addresses.iter().any(|a| !a.is_empty());
246-
let validator = if !has_ism_addresses {
247-
None
248-
} else {
249-
Some(
250-
move |index: usize,
251-
host: &String,
252-
signed_checkpoint: &SignedCheckpointWithMessageId| {
253-
if let Some(expected) = expected_addresses.get(index) {
254-
if expected.is_empty() {
255-
return true;
256-
}
257-
match H160::from_str(expected) {
258-
Ok(expected_h160) => match signed_checkpoint.recover() {
259-
Ok(recovered_signer) => {
260-
if recovered_signer != expected_h160 {
261-
error!(
262-
validator = ?host,
263-
validator_index = index,
264-
expected_signer = ?expected_h160,
265-
actual_signer = ?recovered_signer,
266-
"kaspa: signature verification failed - signer mismatch"
267-
);
268-
false
269-
} else {
270-
true
271-
}
272-
}
273-
Err(e) => {
274-
error!(
275-
validator = ?host,
276-
validator_index = index,
277-
error = ?e,
278-
"kaspa: signature recovery failed"
279-
);
280-
false
281-
}
282-
},
283-
Err(e) => {
284-
error!(
285-
validator = ?host,
286-
validator_index = index,
287-
expected_address = ?expected,
288-
error = ?e,
289-
"kaspa: invalid ISM address format"
290-
);
291-
false
292-
}
293-
}
294-
} else {
295-
true
296-
}
297-
},
298-
)
299-
};
292+
let validator =
293+
move |index: usize,
294+
host: &String,
295+
signed_checkpoint: &SignedCheckpointWithMessageId| {
296+
expected_addresses
297+
.get(index)
298+
.map(|expected| verify_ism_signer(index, host, expected, signed_checkpoint))
299+
.unwrap_or(true)
300+
};
300301

301302
let indexed_sigs = Self::collect_with_threshold(
302303
hosts,
@@ -308,12 +309,11 @@ impl ValidatorsClient {
308309
let fxg = fxg.clone();
309310
Box::pin(async move { request_validate_new_deposits(&client, host, &fxg).await })
310311
},
311-
validator,
312+
Some(validator),
312313
)
313314
.await?;
314315

315-
// Extract signatures and sort by recovered signer address (lexicographic order required by Hub ISM)
316-
// Recovery should not fail here since validation already verified each signature
316+
// Sort by recovered signer address (lexicographic order required by Hub ISM)
317317
let mut sigs: Vec<_> = indexed_sigs.into_iter().map(|(_, sig)| sig).collect();
318318
sigs.sort_by_cached_key(|sig| {
319319
sig.recover()
@@ -330,28 +330,29 @@ impl ValidatorsClient {
330330
) -> ChainResult<Vec<Signature>> {
331331
let threshold = self.multisig_threshold_hub_ism();
332332
let client = self.http_client.clone();
333-
// Use ISM validators for confirmation signatures
334333
let hosts = self.hosts_ism();
334+
let expected_addresses: Vec<String> = self
335+
.validators_ism()
336+
.iter()
337+
.map(|v| v.ism_address.clone())
338+
.collect();
335339
let metrics = self.metrics.clone();
336340
let fxg = fxg.clone();
337341

338-
// Get ISM addresses for sorting from ISM validators
339-
let ism_addresses: Vec<H160> = self
340-
.validators_ism()
341-
.iter()
342-
.enumerate()
343-
.map(|(idx, v)| {
344-
H160::from_str(&v.ism_address).unwrap_or_else(|e| {
345-
warn!(
346-
validator_index = idx,
347-
ism_address = %v.ism_address,
348-
error = ?e,
349-
"kaspa: ISM address parse failed, using default for sorting"
350-
);
351-
H160::default()
352-
})
342+
// Capture progress_indication for signature verification
343+
let progress_indication = fxg.progress_indication.clone();
344+
345+
let validator = move |index: usize, host: &String, signature: &Signature| {
346+
expected_addresses.get(index).map_or(true, |expected| {
347+
// Construct SignedType to enable signer recovery
348+
let signable = SignableProgressIndication::new(progress_indication.clone());
349+
let signed = SignedType {
350+
value: signable,
351+
signature: *signature,
352+
};
353+
verify_ism_signer(index, host, expected, &signed)
353354
})
354-
.collect();
355+
};
355356

356357
let indexed_sigs = Self::collect_with_threshold(
357358
hosts,
@@ -365,11 +366,18 @@ impl ValidatorsClient {
365366
async move { request_validate_new_confirmation(&client, host, &fxg).await },
366367
)
367368
},
368-
None::<fn(usize, &String, &Signature) -> bool>,
369+
Some(validator),
369370
)
370371
.await?;
371372

372-
// Pair signatures with ISM addresses and sort by ISM address (lexicographic order required by Hub ISM)
373+
// Get ISM addresses for sorting
374+
let ism_addresses: Vec<H160> = self
375+
.validators_ism()
376+
.iter()
377+
.map(|v| H160::from_str(&v.ism_address).expect("ISM address must be valid"))
378+
.collect();
379+
380+
// Sort by ISM address (lexicographic order required by Hub ISM)
373381
let mut sigs_with_addr: Vec<_> = indexed_sigs
374382
.into_iter()
375383
.map(|(idx, sig)| {

rust/main/chains/dymension-kaspa/src/validator/server.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,14 @@ pub struct SignableProgressIndication {
431431
progress_indication: ProgressIndication,
432432
}
433433

434+
impl SignableProgressIndication {
435+
pub fn new(progress_indication: ProgressIndication) -> Self {
436+
Self {
437+
progress_indication,
438+
}
439+
}
440+
}
441+
434442
impl Serialize for SignableProgressIndication {
435443
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
436444
where
@@ -519,13 +527,11 @@ async fn respond_validate_confirmed_withdrawals<
519527
info!("validator: confirmed withdrawal is valid");
520528
}
521529

522-
let progress_indication = &conf_fxg.progress_indication;
523-
524530
let sig = res
525531
.must_signing()
526-
.sign_with_fallback(SignableProgressIndication {
527-
progress_indication: progress_indication.clone(),
528-
})
532+
.sign_with_fallback(SignableProgressIndication::new(
533+
conf_fxg.progress_indication.clone(),
534+
))
529535
.await
530536
.map_err(AppError)?;
531537

rust/main/chains/dymension-kaspa/src/validator/startup.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@ pub fn check_migration_lock(
4343
return Ok(());
4444
}
4545

46-
let expected_escrow = fs::read_to_string(&lock_path).map_err(|e| MigrationLockError::ReadError {
47-
path: lock_path.display().to_string(),
48-
reason: e.to_string(),
49-
})?;
46+
let expected_escrow =
47+
fs::read_to_string(&lock_path).map_err(|e| MigrationLockError::ReadError {
48+
path: lock_path.display().to_string(),
49+
reason: e.to_string(),
50+
})?;
5051

5152
let expected_escrow = expected_escrow.trim();
5253

rust/main/hyperlane-base/src/kas_hack/migration.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,20 @@ where
3838
let tx_ids = execute_or_detect_migration(provider, &target_addr, &new_escrow).await;
3939

4040
// Step 2: Wait for TX confirmation then sync hub
41-
info!(delay_secs = SYNC_DELAY_SECS, "Waiting for TX confirmation before hub sync");
41+
info!(
42+
delay_secs = SYNC_DELAY_SECS,
43+
"Waiting for TX confirmation before hub sync"
44+
);
4245
tokio::time::sleep(Duration::from_secs(SYNC_DELAY_SECS)).await;
4346

44-
sync_hub(provider, hub_mailbox, &old_escrow, &new_escrow, &format_signatures).await;
47+
sync_hub(
48+
provider,
49+
hub_mailbox,
50+
&old_escrow,
51+
&new_escrow,
52+
&format_signatures,
53+
)
54+
.await;
4555

4656
Ok(tx_ids)
4757
}
@@ -84,14 +94,21 @@ async fn sync_hub<F>(
8494
old_escrow: &str,
8595
new_escrow: &str,
8696
format_signatures: &F,
87-
)
88-
where
97+
) where
8998
F: Fn(&mut Vec<Signature>) -> ChainResult<Vec<u8>>,
9099
{
91100
let mut attempt: u64 = 0;
92101
loop {
93102
attempt += 1;
94-
match ensure_hub_synced(provider, hub_mailbox, old_escrow, new_escrow, format_signatures).await {
103+
match ensure_hub_synced(
104+
provider,
105+
hub_mailbox,
106+
old_escrow,
107+
new_escrow,
108+
format_signatures,
109+
)
110+
.await
111+
{
95112
Ok(_) => {
96113
info!("Post-migration hub sync completed");
97114
return;

0 commit comments

Comments
 (0)