[ci] processed comments from review

Signed-off-by: Julien Tinguely <[email protected]>

Author: julientinguely-da

.github/workflows/build.yml

@@ -163,7 +163,7 @@ jobs:
     with:
       runs_on: self-hosted-k8s-medium
       test_names_file: 'test-full-class-names-signatures.log'
-      start_canton_options: -B "scripts/bootstrap/bootstrap-canton-with-deprecated-keys.sc"
+      start_canton_options: -B "scripts/bootstrap/bootstrap-canton-with-unsigned-keys.sc"
       parallelism: 1
       test_name: signatures
       with_gcp_creds: false

apps/common/src/main/scala/org/lfdecentralizedtrust/splice/setup/NodeInitializer.scala

@@ -154,7 +154,7 @@ class NodeInitializer(
     } yield ()
   }
 
-  def rotateLocalCantonNodesOTKIfNeeded(
+  def rotateCantonNodesOTKIfNeeded(
       identifierName: String,
       nodeIdentity: UniqueIdentifier => Member & NodeIdentity,
       synchronizerId: SynchronizerId,
@@ -347,39 +347,43 @@ class NodeInitializer(
       _ = logger.info(s"AuthorizedStore snapshot is imported")
     } yield ()
 
+  // This method rotates OwnerToKeyMapping signing public keys that are not signed
   private def rotateOwnerToKeyMappingNotSignedByKeys(
       id: UniqueIdentifier,
       nodeIdentity: UniqueIdentifier => Member & NodeIdentity,
       synchronizerId: SynchronizerId,
-  )(implicit tc: TraceContext, ec: ExecutionContext): Future[Unit] =
+  )(implicit tc: TraceContext, ec: ExecutionContext): Future[Unit] = {
+    val member = nodeIdentity(id)
     for {
-      fullTxHistory <- connection.listAllTransactions(
+      nsTxHistory <- connection.listAllTransactions(
         store = TopologyStoreId.SynchronizerStore(synchronizerId),
         timeQuery = TimeQuery.Range(None, None),
         includeMappings = Set(OwnerToKeyMapping.code),
+        filterNamespace = Some(member.namespace),
       )
-      ownerToKeyMappingTxHistory = fullTxHistory.filter(_.transaction.mapping match {
-        case mapping: OwnerToKeyMapping if mapping.member == nodeIdentity(id) => true
+      ownerToKeyMappings = nsTxHistory.filter(_.transaction.mapping match {
+        case mapping: OwnerToKeyMapping if mapping.member == member => true
         case _ => false
       })
       _ <-
-        if (ownerToKeyMappingTxHistory.isEmpty) {
+        if (ownerToKeyMappings.isEmpty) {
           Future.unit
         } else {
-          performKeyRotation(ownerToKeyMappingTxHistory, nodeIdentity(id))
+          performKeyRotation(ownerToKeyMappings, nodeIdentity(id))
         }
     } yield ()
+  }
 
   private def performKeyRotation(
-      ownerToKeyMappingTxHistory: Seq[StoredTopologyTransaction[TopologyChangeOp, TopologyMapping]],
+      ownerToKeyMappings: Seq[StoredTopologyTransaction[TopologyChangeOp, TopologyMapping]],
       member: Member,
   )(implicit tc: TraceContext, ec: ExecutionContext): Future[Unit] = {
-    val allOtkSignatures = ownerToKeyMappingTxHistory
+    val allOtkSignatures = ownerToKeyMappings
       .map(_.transaction)
       .flatMap(_.signatures)
       .map(_.signedBy)
       .distinct
-    val latestKeys = ownerToKeyMappingTxHistory
+    val currentKeys = ownerToKeyMappings
       .map(_.transaction)
       .sortBy(_.transaction.serial)
       .lastOption
@@ -388,38 +392,37 @@ class NodeInitializer(
       case mapping: OwnerToKeyMapping =>
         mapping.keys.forgetNE
       case _ =>
-        throw new IllegalStateException("Latest transaction is not an OwnerToKeyMapping type.")
+        throw new IllegalStateException("Latest transaction is not of OwnerToKeyMapping type.")
     }
-    val (_, toRotate) = latestKeys.map(_.id).partition(allOtkSignatures.contains)
-    for {
-      _ <-
-        if (toRotate.nonEmpty) {
-          logger.info(s"The following keys with missing signature need to be rotated: $toRotate")
-          val rotatedKeys = latestKeys.map {
-            case key: SigningPublicKey if toRotate.contains(key.id) =>
-              connection.generateKeyPair(
-                key.keySpec.name,
-                key.usage,
-              )
-            case key => Future.successful(key)
-          }
-          for {
-            newKeys <- Future.sequence(rotatedKeys)
-            _ <- connection.ensureOwnerToKeyMapping(
-              member = member,
-              keys = NonEmpty.mk(
-                Seq,
-                newKeys.headOption.getOrElse(throw new IllegalStateException("newKeys is empty.")),
-                newKeys.drop(1)*
-              ),
-              retryFor = RetryFor.Automation,
+    val toRotate = currentKeys.map(_.id).filterNot(allOtkSignatures.contains)
+    if (toRotate.nonEmpty) {
+      logger.info(s"The following keys with missing signature need to be rotated: $toRotate")
+      for {
+        newKeys <- Future.traverse(currentKeys) {
+          case key: SigningPublicKey if toRotate.contains(key.id) =>
+            connection.generateKeyPair(
+              key.keySpec.name,
+              key.usage,
             )
-          } yield logger.info(
-            s"Rotating OTK mapping keys that did not sign the OTK topology transaction."
-          )
-        } else {
-          Future.unit
+          case key => Future.successful(key)
         }
-    } yield ()
+        newKeysNE <- NonEmpty.from(newKeys) match {
+          case Some(ne) => Future.successful(ne)
+          case None =>
+            Future.failed(
+              new IllegalStateException("newKeys collection cannot be empty after rotation.")
+            )
+        }
+        _ <- connection.ensureOwnerToKeyMapping(
+          member = member,
+          keys = newKeysNE,
+          retryFor = RetryFor.Automation,
+        )
+      } yield logger.info(
+        s"Rotating OTK mapping keys that did not sign the OTK topology transaction."
+      )
+    } else {
+      Future.unit
+    }
   }
 }

apps/common/src/main/scala/org/lfdecentralizedtrust/splice/setup/ParticipantInitializer.scala

@@ -118,7 +118,7 @@ class ParticipantInitializer(
 
   def ensureInitializedWithRotatedOTK(synchronizerId: SynchronizerId): Future[Unit] =
     for {
-      _ <- nodeInitializer.rotateLocalCantonNodesOTKIfNeeded(
+      _ <- nodeInitializer.rotateCantonNodesOTKIfNeeded(
         identifierName,
         ParticipantId.apply,
         synchronizerId,

apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/onboarding/NodeInitializerUtil.scala

@@ -25,11 +25,11 @@ import org.lfdecentralizedtrust.splice.store.{
 import org.lfdecentralizedtrust.splice.sv.LocalSynchronizerNode
 import org.lfdecentralizedtrust.splice.sv.automation.{SvDsoAutomationService, SvSvAutomationService}
 import org.lfdecentralizedtrust.splice.sv.cometbft.{CometBftNode, CometBftRequestSigner}
-import org.lfdecentralizedtrust.splice.sv.config.SvAppBackendConfig
+import org.lfdecentralizedtrust.splice.sv.config.{SvAppBackendConfig, SvCantonIdentifierConfig}
 import org.lfdecentralizedtrust.splice.sv.store.{SvDsoStore, SvStore, SvSvStore}
 import org.lfdecentralizedtrust.splice.util.TemplateJsonDecoder
 import com.digitalasset.canton.lifecycle.CloseContext
-import com.digitalasset.canton.logging.NamedLogging
+import com.digitalasset.canton.logging.{NamedLoggerFactory, NamedLogging}
 import com.digitalasset.canton.resource.Storage
 import com.digitalasset.canton.time.Clock
 import com.digitalasset.canton.topology.{ParticipantId, PartyId, SynchronizerId}
@@ -345,6 +345,33 @@ trait NodeInitializerUtil extends NamedLogging with Spanning with SynchronizerNo
     } yield initialRound
   }
 
+  // Ensure OTK mappings contain only signing keys that are signed
+  protected def ensureCantonNodesOTKRotatedIfNeeded(
+      skipSynchronizerInitialization: Boolean,
+      cantonIdentifierConfig: SvCantonIdentifierConfig,
+      localSynchronizerNode: Option[LocalSynchronizerNode],
+      clock: Clock,
+      loggerFactory: NamedLoggerFactory,
+      retryProvider: RetryProvider,
+      decentralizedSynchronizerId: SynchronizerId,
+  )(implicit tc: TraceContext, ec: ExecutionContext): Future[Unit] =
+    localSynchronizerNode match {
+      case Some(synchronizerNode) if !skipSynchronizerInitialization =>
+        SynchronizerNodeInitializer.rotateCantonNodesOTKIfNeeded(
+          cantonIdentifierConfig,
+          synchronizerNode,
+          clock,
+          loggerFactory,
+          retryProvider,
+          decentralizedSynchronizerId,
+        )
+      case _ =>
+        logger.info(
+          "Skipping OTK keys rotation because skipSynchronizerInitialization is enabled"
+        )
+        Future.unit
+    }
+
   private def setInitialRound(connection: BaseLedgerConnection, initialRound: Long)(implicit
       ec: ExecutionContext,
       tc: TraceContext,

apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/onboarding/SynchronizerNodeInitializer.scala

@@ -60,7 +60,7 @@ object SynchronizerNodeInitializer {
     } yield ()
   }
 
-  def rotateLocalCantonNodesOTKIfNeeded(
+  def rotateCantonNodesOTKIfNeeded(
       identifierConfig: SvCantonIdentifierConfig,
       synchronizerNode: LocalSynchronizerNode,
       clock: Clock,
@@ -75,12 +75,12 @@ object SynchronizerNodeInitializer {
       retryProvider,
     )
     for {
-      _ <- synchronizerNodeInitializer.sequencerInitializer.rotateLocalCantonNodesOTKIfNeeded(
+      _ <- synchronizerNodeInitializer.sequencerInitializer.rotateCantonNodesOTKIfNeeded(
         identifierConfig.sequencer,
         SequencerId.apply,
         synchronizerId,
       )
-      _ <- synchronizerNodeInitializer.mediatorInitializer.rotateLocalCantonNodesOTKIfNeeded(
+      _ <- synchronizerNodeInitializer.mediatorInitializer.rotateCantonNodesOTKIfNeeded(
         identifierConfig.mediator,
         MediatorId.apply,
         synchronizerId,

apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/onboarding/joining/JoiningNodeInitializer.scala

@@ -323,24 +323,15 @@ class JoiningNodeInitializer(
           )
           Future.unit
         }
-      _ <-
-        if (!config.skipSynchronizerInitialization) {
-          localSynchronizerNode.traverse(lsn =>
-            SynchronizerNodeInitializer.rotateLocalCantonNodesOTKIfNeeded(
-              cantonIdentifierConfig,
-              lsn,
-              clock,
-              loggerFactory,
-              retryProvider,
-              decentralizedSynchronizerId,
-            )
-          )
-        } else {
-          logger.info(
-            "Skipping OTK keys rotation because skipSynchronizerInitialization is enabled"
-          )
-          Future.unit
-        }
+      _ <- ensureCantonNodesOTKRotatedIfNeeded(
+        config.skipSynchronizerInitialization,
+        cantonIdentifierConfig,
+        localSynchronizerNode,
+        clock,
+        loggerFactory,
+        retryProvider,
+        decentralizedSynchronizerId,
+      )
       _ <- onboard(
         decentralizedSynchronizerId,
         dsoAutomation,

apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/onboarding/sv1/SV1Initializer.scala

@@ -167,22 +167,6 @@ class SV1Initializer(
         } else {
           bootstrapDomain(localSynchronizerNode)
         }
-      _ <-
-        if (!config.skipSynchronizerInitialization) {
-          SynchronizerNodeInitializer.rotateLocalCantonNodesOTKIfNeeded(
-            cantonIdentifierConfig,
-            localSynchronizerNode,
-            clock,
-            loggerFactory,
-            retryProvider,
-            synchronizerId,
-          )
-        } else {
-          logger.info(
-            "Skipping OTK keys rotation because skipSynchronizerInitialization is enabled"
-          )
-          Future.unit
-        }
       _ = logger.info("Domain is bootstrapped, connecting sv1 participant to domain")
       internalSequencerApi = localSynchronizerNode.sequencerInternalConfig
       _ <- participantAdminConnection.ensureDomainRegisteredAndConnected(
@@ -212,6 +196,16 @@ class SV1Initializer(
         retryFor = RetryFor.WaitingOnInitDependency,
       )
       _ = logger.info("Participant connected to domain")
+      _ <- ensureCantonNodesOTKRotatedIfNeeded(
+        config.skipSynchronizerInitialization,
+        cantonIdentifierConfig,
+        Some(localSynchronizerNode),
+        clock,
+        loggerFactory,
+        retryProvider,
+        synchronizerId,
+      )
+      _ = logger.info("Synchronizer rotated OTK keys that were not signed")
       (dsoParty, svParty, _) <- (
         setupDsoParty(synchronizerId, initConnection, namespace),
         SetupUtil.setupSvParty(

build.sbt

@@ -1854,7 +1854,7 @@ checkErrors := {
 
   splitAndCheckCantonLogFile("canton", usesSimtime = false)
   splitAndCheckCantonLogFile("canton-simtime", usesSimtime = true)
-  splitAndCheckCantonLogFile("canton-signatures", usesSimtime = false)
+  splitAndCheckCantonLogFile("canton-missing-signatures", usesSimtime = false)
   import better.files._
   val dir = File("log/")
   if (dir.exists())

project/ignore-patterns/canton-signatures.ignore.txt renamed to project/ignore-patterns/canton-missing-signatures.ignore.txt

@@ -171,9 +171,6 @@ bootstrapDomainWithUnsignedKeys(
   aliceParticipant,
 )
 
-sv1Participant.synchronizers.connect_local(globalSequencerSv1, "global")
-aliceParticipant.synchronizers.connect_local(globalSequencerSv1, "global")
-
 // These user allocations are only there
 // for local testing. Our tests allocate their own users.
 println(s"Allocating users for local testing...")