fix: make 1-based indexing in tbls implementation

crates/app/src/obolapi/error.rs

@@ -59,6 +59,14 @@ pub enum Error {
     #[error("signature string has invalid size: {0}")]
     InvalidSignatureSize(usize),
 
+    /// Failed to convert share index to u8.
+    #[error("failed to convert share index to u8: {0}")]
+    FailedToConvertShareIndexToU8(#[from] std::num::TryFromIntError),
+
+    /// Math overflow error.
+    #[error("math overflow error")]
+    MathOverflow,
+
     /// Epoch parsing error.
     #[error("epoch parsing error: {0}")]
     EpochParse(#[from] std::num::ParseIntError),

crates/app/src/obolapi/exit.rs

@@ -346,9 +346,12 @@ impl Client {
             let mut sig = [0u8; 96];
             sig.copy_from_slice(&sig_bytes);
 
-            // `BlstImpl::threshold_aggregate` shifts the index to 1-based internally
+            // Convert 0-indexed array position to 1-indexed share ID (API stores signatures
+            // at array position share_id-1, e.g., share 1 at position 0)
             let share_idx = u8::try_from(sig_idx)
-                .map_err(|_| Error::InvalidSignatureSize(sig_idx.saturating_add(1)))?;
+                .map_err(Error::FailedToConvertShareIndexToU8)?
+                .checked_add(1)
+                .ok_or(Error::MathOverflow)?;
             raw_signatures.insert(share_idx, sig);
         }
 

crates/cluster/src/test_cluster.rs

@@ -46,7 +46,7 @@ pub fn new_for_test(
         let mut priv_shares: Vec<pluto_crypto::types::PrivateKey> = Vec::with_capacity(n as usize);
 
         for i in 0..n {
-            let share_priv_key = *shares.get(&i).unwrap(); // NOTE: Pluto implementation does not use 1-based indexing for shares
+            let share_priv_key = *shares.get(&i.checked_add(1).unwrap()).unwrap();
             let share_pub = blst.secret_to_public_key(&share_priv_key).unwrap();
 
             pub_shares.push(share_pub);

crates/crypto/src/blst_impl.rs

@@ -15,7 +15,7 @@ use rand_core::{CryptoRng, RngCore};
 
 use crate::{
     tbls::Tbls,
-    types::{BlsError, Error, Index, MathError, PrivateKey, PublicKey, Signature},
+    types::{BlsError, Error, Index, PrivateKey, PublicKey, Signature},
 };
 
 /// Domain Separation Tag for Ethereum 2.0 BLS signatures
@@ -89,7 +89,7 @@ impl Tbls for BlstImpl {
         let mut shares = HashMap::new();
         for i in 1..=total {
             let share = evaluate_polynomial(&poly, i)?;
-            shares.insert(i.saturating_sub(1), share.to_bytes());
+            shares.insert(i, share.to_bytes());
         }
 
         Ok(shares)
@@ -110,12 +110,9 @@ impl Tbls for BlstImpl {
             return Err(Error::SharesAreEmpty);
         }
 
-        // Convert share indices to 1-indexed (shares are stored 0-indexed, but
-        // evaluated at 1-indexed points)
-        let share_points: Vec<Index> = shares
-            .keys()
-            .map(|&k| k.checked_add(1).ok_or(MathError::IntegerOverflow))
-            .collect::<Result<Vec<_>, _>>()?;
+        // Share indices are already 1-indexed (matching their polynomial evaluation
+        // points)
+        let share_points: Vec<Index> = shares.keys().copied().collect();
 
         let share_secrets: Vec<BlstSecretKey> = shares
             .values()
@@ -162,12 +159,8 @@ impl Tbls for BlstImpl {
             return Err(Error::EmptySignatureArray);
         }
 
-        // Convert indices to 1-indexed points (shares are 0-indexed, evaluated at
-        // 1-indexed points)
-        let indices: Vec<Index> = partial_signatures_by_idx
-            .keys()
-            .map(|&k| k.checked_add(1).ok_or(MathError::IntegerOverflow))
-            .collect::<Result<Vec<_>, _>>()?;
+        // Signature indices are already 1-indexed (matching share evaluation points)
+        let indices: Vec<Index> = partial_signatures_by_idx.keys().copied().collect();
 
         let signatures: Vec<BlstSignature> = partial_signatures_by_idx
             .values()
@@ -506,7 +499,7 @@ fn scalar_div(
 ) -> Result<blst::blst_scalar, Error> {
     let zero = blst::blst_scalar::default();
     if *denominator == zero {
-        return Err(Error::MathError(MathError::DivisionByZero));
+        return Err(Error::DivisionByZero);
     }
 
     let mut inv_scalar = blst::blst_scalar::default();
@@ -951,4 +944,26 @@ mod tests {
             "Different secrets should produce different public keys"
         );
     }
+
+    #[test]
+    fn test_threshold_split_returns_1_indexed_keys() {
+        use rand::rngs::OsRng;
+
+        let blst = setup();
+        let sk = blst.generate_secret_key(OsRng).unwrap();
+
+        // Split into 5 shares
+        let shares = blst.threshold_split(&sk, 5, 3).unwrap();
+        assert_eq!(shares.len(), 5);
+
+        // Verify keys are 1-indexed (1, 2, 3, 4, 5)
+        assert!(shares.contains_key(&1), "Should contain key 1");
+        assert!(shares.contains_key(&2), "Should contain key 2");
+        assert!(shares.contains_key(&3), "Should contain key 3");
+        assert!(shares.contains_key(&4), "Should contain key 4");
+        assert!(shares.contains_key(&5), "Should contain key 5");
+
+        // Verify no 0-indexed key exists
+        assert!(!shares.contains_key(&0), "Should not contain key 0");
+    }
 }

crates/crypto/src/tbls.rs

@@ -28,6 +28,9 @@ pub trait Tbls {
     /// secret keys, with the given threshold. It returns a map that
     /// associates each private, compressed private key to its ID.
     ///
+    /// **Important:** Share IDs are 1-indexed (1, 2, 3, ..., n), matching
+    /// the Go implementation and TBLS polynomial evaluation points.
+    ///
     /// # Limitations
     ///
     /// Maximum of 255 shares (total <= 255) due to underlying BLS library
@@ -44,6 +47,9 @@ pub trait Tbls {
     /// keys, with the given threshold. It returns a map that associates
     /// each private, compressed private key to its ID.
     ///
+    /// **Important:** Share IDs are 1-indexed (1, 2, 3, ..., n), matching
+    /// the Go implementation and TBLS polynomial evaluation points.
+    ///
     /// # Limitations
     ///
     /// Maximum of 255 shares (total <= 255) due to underlying BLS library
@@ -57,6 +63,9 @@ pub trait Tbls {
 
     /// Recovers a secret from a set of shares
     ///
+    /// **Important:** Share IDs in the input HashMap must be 1-indexed
+    /// (1, 2, 3, ..., n), matching the IDs returned by threshold_split.
+    ///
     /// # Limitations
     ///
     /// Share IDs must be < 255 due to underlying BLS library constraints.
@@ -68,6 +77,9 @@ pub trait Tbls {
     /// Aggregates a set of partial signatures into a single
     /// signature
     ///
+    /// **Important:** Share IDs in the input HashMap must be 1-indexed
+    /// (1, 2, 3, ..., n), matching the share IDs used for key splitting.
+    ///
     /// # Limitations
     ///
     /// Share IDs must be < 255 due to underlying BLS library constraints.

crates/crypto/src/types.rs

@@ -78,9 +78,9 @@ pub enum Error {
     #[error("Signature array is empty")]
     EmptySignatureArray,
 
-    /// Math error during field operations.
-    #[error("Math error: {0}")]
-    MathError(#[from] MathError),
+    /// Division by zero.
+    #[error("Division by zero")]
+    DivisionByZero,
 
     /// Failed to convert secret key to blst scalar.
     #[error("Failed to convert secret key to blst scalar")]
@@ -189,26 +189,3 @@ impl From<BLST_ERROR> for Error {
         Error::BlsError(BlsError::from(err))
     }
 }
-
-/// Math error type.
-///
-/// This enum represents all possible math errors that can occur during
-/// arithmetic operations in the charon-crypto library.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)]
-pub enum MathError {
-    /// Integer overflow during share ID calculation.
-    #[error("Integer overflow")]
-    IntegerOverflow,
-
-    /// Integer underflow during arithmetic operation.
-    #[error("Integer underflow")]
-    IntegerUnderflow,
-
-    /// Division by zero attempted.
-    #[error("Division by zero")]
-    DivisionByZero,
-
-    /// Modulo by zero attempted.
-    #[error("Modulo by zero")]
-    ModuloByZero,
-}