From ba3d0ffd4e8674660719a088cb0d5b19cd5583bc Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Mon, 5 Oct 2026 16:28:19 -0400 Subject: [PATCH 01/11] feat: let the sync operator set an outage traffic allowance [ci] During a global-synchronizer outage, the operator sets outage-traffic-allowance in the sync operator app's config and restarts the app. On start, the app sets each member's limit to its purchased total plus the allowance, or back to the purchased total once it is removed. It warns while the allowance is set, when a purchase lands meanwhile, and for any limit that does not reach its target. The validator's top-up buys a member's shortfall together with its normal top-up in one purchase. Closes ChainSafe/canton-extending-mainnet#129 Signed-off-by: Timothy Wu --- .../SyncOperatorTrafficIntegrationTest.scala | 293 +++++++++++++++++- .../splice/util/SyncOperatorTestUtil.scala | 11 +- .../store/db/MemberTrafficQueries.scala | 36 +++ .../splice/syncoperator/SyncOperatorApp.scala | 2 + .../OutageTrafficAllowanceTrigger.scala | 193 ++++++++++++ ...OutageTrafficAllowanceWarningTrigger.scala | 47 +++ ...cileDedicatedSequencerTrafficTrigger.scala | 25 ++ .../SyncOperatorAutomationService.scala | 28 ++ .../config/SyncOperatorAppConfig.scala | 10 +- .../store/SyncOperatorStore.scala | 7 + .../store/db/DbSyncOperatorStore.scala | 12 + .../store/db/SyncOperatorStoreTest.scala | 25 ++ .../OutageTrafficAllowanceTriggerTest.scala | 116 +++++++ .../TopupMemberTrafficTrigger.scala | 27 +- .../TopupMemberTrafficTriggerTest.scala | 33 ++ .../splice/wallet/util/TopupUtil.scala | 38 ++- .../splice/wallet/util/TopupUtilTest.scala | 40 +++ test-full-class-names-non-integration.log | 2 + 18 files changed, 919 insertions(+), 26 deletions(-) create mode 100644 apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala create mode 100644 apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala create mode 100644 apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala create mode 100644 apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index 5c0f50fb33f..f93440d63b5 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -4,12 +4,15 @@ package org.lfdecentralizedtrust.splice.integration.tests import com.digitalasset.canton.SynchronizerAlias +import com.digitalasset.canton.config.CantonRequireTypes.InstanceName import com.digitalasset.canton.config.NonNegativeFiniteDuration import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, NonNegativeNumeric} -import com.digitalasset.canton.topology.SynchronizerId +import com.digitalasset.canton.logging.SuppressionRule +import com.digitalasset.canton.topology.{Member, SynchronizerId} import monocle.macros.syntax.lens.* import org.lfdecentralizedtrust.splice.codegen.java.splice.decentralizedsynchronizer.GovernanceParameters import org.lfdecentralizedtrust.splice.codegen.java.splice.wallet.topupstate.ValidatorTopUpState +import org.lfdecentralizedtrust.splice.console.ParticipantClientReference import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.TopologySnapshot import org.lfdecentralizedtrust.splice.config.ConfigTransforms import org.lfdecentralizedtrust.splice.http.v0.definitions as d0 @@ -26,12 +29,15 @@ import org.lfdecentralizedtrust.splice.util.{ } import org.lfdecentralizedtrust.splice.validator.automation.TopupMemberTrafficTrigger import org.lfdecentralizedtrust.splice.wallet.store.TxLogEntry +import org.slf4j.event.Level import scala.concurrent.duration.* import scala.jdk.CollectionConverters.* +import scala.util.Try /** Buys traffic for a registered synchronizer via `AmuletRules_BuyMemberTraffic` with the - * registration disclosed, and checks the operator grants it on that synchronizer's sequencer. + * registration disclosed, and checks the operator grants it on that synchronizer's sequencer, + * also through a global-synchronizer outage on the operator's outage traffic allowance. */ @org.lfdecentralizedtrust.splice.util.scalatesttags.SpliceDsoGovernance_0_1_30 class SyncOperatorTrafficIntegrationTest @@ -46,6 +52,20 @@ class SyncOperatorTrafficIntegrationTest private val walletRequestPurchase = 3_000_000L private val discount = BigDecimal("0.5") private val splitwellAlias = SynchronizerAlias.tryCreate("splitwell") + private val globalAlias = SynchronizerAlias.tryCreate("global") + + // The outage test's amounts: small enough that a few dozen pings take bob past what was bought + // for him and on through the allowance. + private val outageTrafficAllowance = 50_000L + private val bobPurchase = 50_000L + // Bought once the global synchronizer is back but before the allowance is removed. + private val bobPurchaseWithAllowanceSet = 20_000L + // Less than the shortfall bob is left with once the allowance is removed, so one top-up only + // covers it if it buys the shortfall too. + private val bobTopupAmount = 12_000L + // The same operator app with the allowance set, which the outage test runs in place of + // syncOperator. Both use one store, so they never run at the same time. + private val syncOperatorWithAllowanceName = "syncOperatorWithAllowance" override def environmentDefinition: SpliceEnvironmentDefinition = EnvironmentDefinition @@ -53,14 +73,49 @@ class SyncOperatorTrafficIntegrationTest Seq("simple-topology-1sv.conf", "sync-operator-topology.conf"), this.getClass.getSimpleName, ) - .withOnlyAliceValidatorConnectingToSplitwell + // Only alice and bob connect to splitwell: alice for the purchases, bob for the outage test, + // which needs a member whose balance starts small. + .addConfigTransform((_, conf) => + conf.copy( + splitwellApps = Map.empty, + validatorApps = + conf.validatorApps.updatedWith(InstanceName.tryCreate("splitwellValidator")) { + _.map(c => c.copy(domains = c.domains.copy(extra = Seq.empty))) + }, + ) + ) .withStandardSetup - // withStandardSetup turns top-ups on for the decentralized synchronizer. Move alice's - // target onto splitwell instead, so the operator's synchronizer is the only one she tops - // up automatically. + // Down from 200 KB, so bob's small purchases are allowed. + .addConfigTransform((_, conf) => + ConfigTransforms.updateAllSvAppFoundDsoConfigs_( + _.focus(_.initialSynchronizerFeesConfig.minTopupAmount) + .replace(NonNegativeLong.tryCreate(10_000L)) + )(conf) + ) + // withStandardSetup turns top-ups on for the decentralized synchronizer. Move alice's and + // bob's targets onto splitwell instead, so the operator's synchronizer is the only one they + // top up automatically. .addConfigTransform((_, config) => ConfigTransforms.updateAllValidatorConfigs { case (name, validatorConfig) => - if (name == "aliceValidator") + if (name == "bobValidator") + validatorConfig + .focus(_.domains.global.buyExtraTraffic.targetThroughput) + .replace(NonNegativeNumeric.tryCreate(BigDecimal(0))) + .focus(_.domains.extra) + .modify(_.map { extra => + if (extra.alias == splitwellAlias) + extra + // 100 bytes a second over a two-minute interval, so bobTopupAmount per top-up + .focus(_.topup.targetThroughput) + .replace(NonNegativeNumeric.tryCreate(BigDecimal(100))) + .focus(_.topup.minTopupInterval) + .replace(NonNegativeFiniteDuration.ofMinutes(2)) + else extra + }) + // Resumed only once the outage test needs bob's top-up to cover his shortfall. + .focus(_.automation) + .modify(_.withPausedTrigger[TopupMemberTrafficTrigger]) + else if (name == "aliceValidator") validatorConfig .focus(_.domains.global.buyExtraTraffic.targetThroughput) .replace(NonNegativeNumeric.tryCreate(BigDecimal(0))) @@ -74,15 +129,37 @@ class SyncOperatorTrafficIntegrationTest .replace(NonNegativeFiniteDuration.ofMinutes(1)) else extra }) - // Resumed only at the end of the test: a trigger buying on splitwell for the same - // member would break the exact-equality assertions on the two manual purchases. - // Alice's validator only: pausing a trigger an app does not register warns, and the - // SV validator never registers this one. + // Resumed only at the end of the first test: a trigger buying on splitwell for the + // same member would break the exact-equality assertions on the two manual purchases. + // Alice's and bob's validators only: pausing a trigger an app does not register warns, + // and the SV validator never registers this one. .focus(_.automation) .modify(_.withPausedTrigger[TopupMemberTrafficTrigger]) else validatorConfig }(config) ) + // Last, so it is a copy of syncOperator as every transform above left it. + .addConfigTransform { (_, conf) => + val allowance = Some(NonNegativeLong.tryCreate(outageTrafficAllowance)) + val syncOperator = conf.syncOperatorApps(InstanceName.tryCreate("syncOperator")) + conf.copy(syncOperatorApps = + conf.syncOperatorApps.updated( + InstanceName.tryCreate(syncOperatorWithAllowanceName), + syncOperator.copy(outageTrafficAllowance = allowance), + ) + ) + } + // Every app but syncOperatorWithAllowance starts as it would without manual start, and the + // multi-synchronizer flag that withManualStart's reset drops is set again once they have. + .withManualStart + .withAdditionalSetup(implicit env => + startAllSync( + env.amuletNodes.local + .distinctBy(_.name) + .filterNot(_.name == syncOperatorWithAllowanceName)* + ) + ) + .withMultiSyncFeatureFlag() "sync operator" should { @@ -271,8 +348,202 @@ class SyncOperatorTrafficIntegrationTest } } } + + "keep members transacting through a global-synchronizer outage on the operator's allowance" in { + implicit env => + val synchronizerId = syncOperatorBackend.appState.store.key.synchronizerId + val dsoParty = sv1Backend.getDsoInfo().dsoParty + // Registered by the test above. + val registration = eventually() { + sv1ScanBackend.lookupSynchronizerRegistration(synchronizerId.toProtoPrimitive).value + } + val aliceParticipant = aliceValidatorBackend.participantClientWithAdminToken + val alice = aliceParticipant.id + val bobParticipant = bobValidatorBackend.participantClientWithAdminToken + val bob = bobParticipant.id + // The operator app sees purchases, which are made on the global synchronizer, through + // this participant. + val operatorParticipant = splitwellValidatorBackend.participantClientWithAdminToken + val syncOperatorWithAllowance = syncop(syncOperatorWithAllowanceName) + def purchased() = + runningSyncOperatorBackend.appState.store.listTotalPurchasedMemberTraffic().futureValue + def limitsAreTotalsPlus(allowance: Long, totals: Map[Member, Long]) = + forAll(totals) { case (member, total) => + extraTrafficLimit(member) shouldBe total + allowance + } + + clue("bob holds no traffic on splitwell yet") { + extraTrafficLimit(bob) shouldBe 0L + } + + // Anyone can fund a member's traffic, so alice pays for bob's. + val aliceParty = onboardWalletUser(aliceWalletClient, aliceValidatorBackend) + aliceWalletClient.tap(walletUsdToAmulet(50.0)) + actAndCheck( + "alice buys a little traffic for bob on splitwell", + buyTraffic(aliceParty, bob, synchronizerId, Some(registration), dsoParty, bobPurchase), + )( + "the purchase is granted", + _ => extraTrafficLimit(bob) shouldBe bobPurchase, + ) + val totals = purchased() + totals.keySet shouldBe Set(alice, bob) + + try { + // The other apps on the operator's participant warn about the submissions that fail + // while it is cut off from the global synchronizer, so this tolerates their warnings. + loggerFactory.assertLogsSeq(SuppressionRule.LevelAndAbove(Level.WARN))( + { + // Cut off from the global synchronizer, the operator's participant sees no + // purchases, which is what an outage looks like to the operator. + operatorParticipant.synchronizers.disconnect(globalAlias) + + actAndCheck( + "the operator sets the allowance and restarts its app", { + syncOperatorBackend.stop() + syncOperatorWithAllowance.startSync() + }, + )( + "each member's limit rises to its purchased total plus the allowance, and the app " + + "warns that the allowance is set", + _ => { + limitsAreTotalsPlus(outageTrafficAllowance, totals) + recordedWarnings(allowanceSetWarning) should not be empty + }, + ) + + clue("a restart with the allowance still set moves no limit") { + syncOperatorWithAllowance.stop() + syncOperatorWithAllowance.startSync() + always(durationOfSuccess = 10.seconds, pollIntervalMs = 1000) { + limitsAreTotalsPlus(outageTrafficAllowance, totals) + } + } + + clue("bob keeps transacting past what was bought for him") { + pingWhile(bobParticipant, synchronizerId)(consumed(bob) <= bobPurchase) + consumed(bob) should be > bobPurchase + } + + clue("and is refused once he has used the allowance") { + Iterator + .continually(Try(ping(bobParticipant, synchronizerId))) + .take(maxPings) + .exists(_.isFailure) shouldBe true + consumed(bob) should be <= bobPurchase + outageTrafficAllowance + } + + clue("alice runs on what was bought for her, without the allowance") { + consumed(alice) should be <= totals(alice) + } + }, + // Once per start of the app. + logs => forAtLeast(2, logs)(_.warningMessage should include(allowanceSetWarning)), + ) + + loggerFactory.assertLogsSeq(SuppressionRule.LevelAndAbove(Level.WARN))( + { + operatorParticipant.synchronizers.reconnect(globalAlias) + + actAndCheck( + "alice buys more traffic for bob once the global synchronizer is back", + buyTraffic( + aliceParty, + bob, + synchronizerId, + Some(registration), + dsoParty, + bobPurchaseWithAllowanceSet, + ), + )( + "the operator warns that the allowance is still set", + _ => recordedWarnings(purchaseLandedWarning) should not be empty, + ) + + clue("the purchase pays down bob's credit instead of raising his limit") { + purchased()(bob) shouldBe bobPurchase + bobPurchaseWithAllowanceSet + extraTrafficLimit(bob) shouldBe bobPurchase + outageTrafficAllowance + } + + // In here, as the app warns that the allowance is set until it has stopped. + syncOperatorWithAllowance.stop() + }, + logs => forAtLeast(1, logs)(_.warningMessage should include(purchaseLandedWarning)), + ) + } finally { + if ( + !operatorParticipant.synchronizers + .list_connected() + .exists(_.synchronizerAlias == globalAlias) + ) + operatorParticipant.synchronizers.reconnect(globalAlias) + } + + val totalsWithoutAllowance = actAndCheck( + "the operator restarts its app without the allowance", { + syncOperatorBackend.startSync() + purchased() + }, + )( + "each member's limit goes back down to its purchased total", + totals => limitsAreTotalsPlus(0L, totals), + )._1 + + clue("alice, who did not use the allowance, keeps transacting") { + ping(aliceParticipant, synchronizerId) + } + + clue("bob used more than was bought for him, so he is refused until a purchase covers it") { + consumed(bob) should be > totalsWithoutAllowance(bob) + assertThrowsAndLogsCommandFailures( + ping(bobParticipant, synchronizerId), + _.errorMessage should include("Unable to ping"), + ) + } + + val topupTrigger = + bobValidatorBackend.appState.automation.trigger[TopupMemberTrafficTrigger] + setTriggersWithin(triggersToResumeAtStart = Seq(topupTrigger)) { + clue("one top-up buys bob's shortfall with it, and he transacts again") { + // Inside bob's two-minute top-up interval, so a single purchase covered it. + eventually(100.seconds) { + extraTrafficLimit(bob) should be > consumed(bob) + } + // More than the top-up alone, which is less than the shortfall. + purchased()(bob) - totalsWithoutAllowance(bob) should be > bobTopupAmount + ping(bobParticipant, synchronizerId) + } + } + } } + private val allowanceSetWarning = + s"The outage traffic allowance of $outageTrafficAllowance bytes is set, covering 2 members" + private val purchaseLandedWarning = "landed while the outage traffic allowance is set" + + // The warnings suppressed so far in the enclosing log assertion, for waiting on one. + private def recordedWarnings(text: String) = + loggerFactory.fetchRecordedLogEntries.filter(_.message.contains(text)) + + // Bounds the ping loops, so a ping that costs less than expected fails the test instead of + // running forever. + private val maxPings = 300 + + private def ping(participant: ParticipantClientReference, synchronizerId: SynchronizerId) = + participant.health.ping(participant.id, synchronizerId = Some(synchronizerId)) + + private def pingWhile(participant: ParticipantClientReference, synchronizerId: SynchronizerId)( + condition: => Boolean + ): Unit = + Iterator + .continually(()) + .take(maxPings) + .takeWhile(_ => condition) + .foreach(_ => ping(participant, synchronizerId)) + + private def consumed(member: Member)(implicit env: SpliceTestConsoleEnvironment): Long = + trafficState(member).fold(0L)(_.extraTrafficConsumed.value) + // The sync-operator CI job bootstraps splitwell with traffic control and a zero base rate; // see start-canton.sh -t. private def synchronizerParameters(implicit env: SpliceTestConsoleEnvironment) = diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/util/SyncOperatorTestUtil.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/util/SyncOperatorTestUtil.scala index f74537b0050..e26fee6fd91 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/util/SyncOperatorTestUtil.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/util/SyncOperatorTestUtil.scala @@ -9,6 +9,7 @@ import org.lfdecentralizedtrust.splice.codegen.java.splice import org.lfdecentralizedtrust.splice.codegen.java.splice.decentralizedsynchronizer.RegisteredSynchronizer import org.lfdecentralizedtrust.splice.codegen.java.splice.round.IssuingMiningRound import org.lfdecentralizedtrust.splice.codegen.java.splice.types.Round +import org.lfdecentralizedtrust.splice.console.SyncOperatorAppBackendReference import org.lfdecentralizedtrust.splice.environment.SequencerAdminConnection import org.lfdecentralizedtrust.splice.integration.tests.SpliceTests.{ SpliceTestConsoleEnvironment, @@ -82,10 +83,18 @@ trait SyncOperatorTestUtil extends TestCommon { .exerciseResult } + /** The sync operator app that is running, as a test may run another instance in its place. */ + protected def runningSyncOperatorBackend(implicit + env: SpliceTestConsoleEnvironment + ): SyncOperatorAppBackendReference = + env.syncOperators.local + .find(_.is_running) + .getOrElse(fail("No sync operator app is running")) + protected def trafficState( member: Member )(implicit env: SpliceTestConsoleEnvironment): Option[SequencerAdminConnection.TrafficState] = - syncOperatorBackend.appState.sequencerAdminConnection + runningSyncOperatorBackend.appState.sequencerAdminConnection .lookupSequencerTrafficControlState(member) .futureValue diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala index 23dd4de7b7d..4a66d5b4bee 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala @@ -48,4 +48,40 @@ trait MemberTrafficQueries extends AcsJdbcTypes with LimitHelpers { this: NamedL ) .value } yield sum.getOrElse(0L) + + /** Total traffic purchased on `synchronizerId`, for every member with a purchase on record. */ + protected def sumPurchasedMemberTrafficPerMember( + storage: DbStorage, + acsTableName: String, + acsStoreId: AcsStoreId, + migrationId: Long, + synchronizerId: SynchronizerId, + )(implicit + ec: ExecutionContext, + tc: TraceContext, + closeContext: CloseContext, + ): Future[Map[Member, Long]] = + for { + rows <- storage + .query( + sql""" + select member_traffic_member, sum(total_traffic_purchased) + from #$acsTableName + where store_id = $acsStoreId + and migration_id = $migrationId + and package_name = ${MemberTraffic.PACKAGE_NAME} + and template_id_qualified_name = ${QualifiedName( + MemberTraffic.TEMPLATE_ID_WITH_PACKAGE_ID + )} + and member_traffic_domain = $synchronizerId + and member_traffic_member is not null + group by member_traffic_member + """.as[(String, Long)], + // the callers' method name, which is what the DB retry logs report + "listTotalPurchasedMemberTraffic", + ) + } yield rows.flatMap { case (member, total) => + // ingestion stores only members that parse, so this never drops a row in practice + Member.fromProtoPrimitive_(member).toOption.map(_ -> total) + }.toMap } diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala index 58893cc4b24..4af8fa9d3e8 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala @@ -173,6 +173,8 @@ class SyncOperatorApp( config.lsu, config.lsuDumpPath, config.trafficBalanceReconciliationDelay, + config.outageTrafficAllowance, + config.outageTrafficAllowanceWarningInterval, loggerFactory, packageVersionSupport, ) diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala new file mode 100644 index 00000000000..bbb394e11df --- /dev/null +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala @@ -0,0 +1,193 @@ +// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package org.lfdecentralizedtrust.splice.syncoperator.automation + +import cats.syntax.apply.* +import com.digitalasset.canton.config.NonNegativeFiniteDuration +import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} +import com.digitalasset.canton.topology.{MediatorId, Member} +import com.digitalasset.canton.tracing.TraceContext +import io.grpc.{Status, StatusRuntimeException} +import io.opentelemetry.api.trace.Tracer +import org.apache.pekko.stream.Materializer +import org.lfdecentralizedtrust.splice.automation.{ + PollingParallelTaskExecutionTrigger, + TaskFailed, + TaskOutcome, + TaskSuccess, + TriggerContext, +} +import org.lfdecentralizedtrust.splice.environment.SynchronizerNodeService +import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.TopologySnapshot +import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode +import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTrigger.Task +import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore + +import java.util.concurrent.atomic.AtomicBoolean +import scala.concurrent.{ExecutionContext, Future} + +/** Sets each member's traffic limit on this operator's sequencer from the outage traffic allowance + * in this app's config, when the app starts. + * + * While the global synchronizer is unavailable, nothing can be bought for this synchronizer, so + * members run only on what was bought for them before. The operator then sets + * `outage-traffic-allowance` and restarts this app: each member with a purchase on record gets a + * limit of exactly its purchased total plus the allowance, so it can run that many bytes past what + * was bought for it. Once the global synchronizer is back, the operator removes the setting and + * restarts again: each limit is set back to the purchased total, so a member that used the + * allowance is below zero until its purchases cover what it consumed. + * + * The targets only move with the config, which only changes on a restart, so once a poll finds + * every member at its target this does no more work per member. Purchases that land in the + * meantime are granted by [[ReconcileDedicatedSequencerTrafficTrigger]], which only raises a limit + * to the purchased total: while the allowance is set, they pay down the credit instead of adding + * to it. + * + * On a synchronizer whose sequencers several operators run, a limit changes only once the + * sequencer group's threshold of operators set the same one, so they set and remove the allowance + * together. + */ +class OutageTrafficAllowanceTrigger( + override protected val context: TriggerContext, + store: SyncOperatorStore, + synchronizerNodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], + allowance: Option[NonNegativeLong], + trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, +)(implicit + override val ec: ExecutionContext, + mat: Materializer, + override val tracer: Tracer, +) extends PollingParallelTaskExecutionTrigger[Task] { + + // Set once a poll finds every member at its target, after which polls do nothing. + private val settled = new AtomicBoolean(false) + + override protected def retrieveTasks()(implicit tc: TraceContext): Future[Seq[Task]] = + if (settled.get()) Future.successful(Seq.empty) + else + for { + connection <- synchronizerNodeService.sequencerAdminConnection() + totals <- store.listTotalPurchasedMemberTraffic() + // An empty filter would list every member of the synchronizer. + states <- + if (totals.isEmpty) Future.successful(Seq.empty) + else connection.listSequencerTrafficControlState(totals.keys.toSeq) + } yield { + val limits = states.map(state => state.member -> state.extraTrafficLimit.value).toMap + val tasks = + OutageTrafficAllowanceTrigger.tasks(totals, limits, allowance.map(_.value)) + if (tasks.isEmpty && !settled.getAndSet(true)) { + val target = allowance.fold("its purchased total")(a => + s"its purchased total plus the outage traffic allowance of ${a.value} bytes" + ) + logger.info(s"Every member with a purchase on record is at $target") + } + tasks + } + + override protected def completeTask(task: Task)(implicit tc: TraceContext): Future[TaskOutcome] = + for { + connection <- synchronizerNodeService.sequencerAdminConnection() + // Read again, so the grant carries the serial the sequencer expects next. + (trafficState, sequencerState) <- ( + connection.getSequencerTrafficControlState(task.member), + connection.getSequencerSynchronizerState(TopologySnapshot.Effective), + ).tupled + limit = trafficState.extraTrafficLimit.value + outcome <- connection + .setSequencerTrafficControlState( + trafficState, + sequencerState, + NonNegativeLong.tryCreate(task.target), + context.pollingClock, + trafficBalanceReconciliationDelay, + ) + .map[TaskOutcome](_ => + TaskSuccess(s"Set the traffic limit of ${task.member} from $limit to ${task.target}") + ) + .recover { + // The grant did not land, for example because too few of the synchronizer's operators + // sent the same one. The next poll tries again, as the targets are not all in place. + case ex: StatusRuntimeException + if ex.getStatus.getCode == Status.Code.DEADLINE_EXCEEDED => + TaskFailed(task.notApplied(limit)) + } + } yield outcome + + override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = + for { + connection <- synchronizerNodeService.sequencerAdminConnection() + state <- connection.lookupSequencerTrafficControlState(task.member) + } yield state.forall(state => task.isDone(state.extraTrafficLimit.value)) +} + +object OutageTrafficAllowanceTrigger { + + /** Whether the allowance applies to a member with a purchase on record. Not to a mediator: with + * no purchase path, it is granted unlimited traffic by [[MediatorUnlimitedTrafficTrigger]], + * which a purchase someone made for it must not lower. + */ + def isCovered(member: Member): Boolean = member match { + case _: MediatorId => false + case _ => true + } + + /** The grants that bring each member with a purchase on record to its target, in a fixed order. + * Members without a traffic state on the sequencer are left out, as there is nothing to set yet. + */ + def tasks( + totals: Map[Member, Long], + limits: Map[Member, Long], + allowance: Option[Long], + ): Seq[Task] = + totals.toSeq + .filter { case (member, _) => isCovered(member) } + .sortBy { case (member, _) => member.toProtoPrimitive } + .flatMap { case (member, total) => + limits.get(member).flatMap { limit => + val task = Task(member, total, allowance) + Option.when(!task.isDone(limit))(task) + } + } + + final case class Task(member: Member, total: Long, allowance: Option[Long]) + extends PrettyPrinting { + + /** The purchased total, plus the allowance while it is set, capped at the largest limit there + * is. + */ + def target: Long = + allowance.fold(total)(a => if (a > Long.MaxValue - total) Long.MaxValue else total + a) + + /** Whether `limit` leaves nothing to do. Exact both ways: setting the allowance raises a limit + * and setting a smaller one lowers it again, while removing it lowers a limit the allowance + * raised and raises one a purchase ingested during the removal left behind. + */ + def isDone(limit: Long): Boolean = limit == target + + /** Why a limit still at `limit` matters to the operator. */ + def notApplied(limit: Long): String = allowance match { + case Some(a) => + s"Could not set the traffic limit of $member to its purchased total of $total plus the " + + s"outage traffic allowance of $a bytes: it is still $limit. On a synchronizer whose " + + "sequencers several operators run, it changes only once enough of them set the same " + + "allowance." + case None => + val gap = limit - total + val side = if (gap > 0) "above" else "below" + s"Could not bring the traffic limit of $member back to its purchased total of $total: " + + s"it is still $limit, ${math.abs(gap)} bytes $side it. On a synchronizer whose " + + "sequencers several operators run, it changes only once enough of them have removed " + + "the outage traffic allowance." + } + + override def pretty: Pretty[Task] = prettyOfClass( + param("member", _.member), + param("total", _.total), + paramIfDefined("allowance", _.allowance), + param("target", _.target), + ) + } +} diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala new file mode 100644 index 00000000000..92a682bf7fa --- /dev/null +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala @@ -0,0 +1,47 @@ +// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package org.lfdecentralizedtrust.splice.syncoperator.automation + +import com.digitalasset.canton.config.NonNegativeFiniteDuration +import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.tracing.TraceContext +import io.opentelemetry.api.trace.Tracer +import org.lfdecentralizedtrust.splice.automation.{ + PeriodicTaskTrigger, + TaskOutcome, + TaskSuccess, + TriggerContext, +} +import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore + +import scala.concurrent.{ExecutionContext, Future} + +/** Warns, from startup and then every `interval`, that the outage traffic allowance is set, so the + * operator removes it once the global synchronizer is back. Only registered while it is set. + */ +class OutageTrafficAllowanceWarningTrigger( + interval: NonNegativeFiniteDuration, + triggerContext: TriggerContext, + store: SyncOperatorStore, + allowance: NonNegativeLong, +)(implicit + override val ec: ExecutionContext, + override val tracer: Tracer, +) extends PeriodicTaskTrigger(interval, triggerContext) { + + override def completeTask( + task: PeriodicTaskTrigger.PeriodicTask + )(implicit tc: TraceContext): Future[TaskOutcome] = + store.listTotalPurchasedMemberTraffic().map { totals => + val members = totals.keys.count(OutageTrafficAllowanceTrigger.isCovered) + val credit = BigInt(allowance.value) * members + logger.warn( + s"The outage traffic allowance of ${allowance.value} bytes is set, covering $members " + + s"members with a purchase on record, for a total credit outstanding of up to $credit " + + "bytes. Once the global synchronizer is reachable again, remove " + + "outage-traffic-allowance from this app's config and restart it." + ) + TaskSuccess(s"Warned that the outage traffic allowance covers $members members") + } +} diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index afb1e5b5f4d..ed251602ff5 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -4,18 +4,22 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.digitalasset.canton.config.NonNegativeFiniteDuration +import com.digitalasset.canton.config.RequireTypes.NonNegativeLong import com.digitalasset.canton.topology.Member import com.digitalasset.canton.tracing.TraceContext import io.opentelemetry.api.trace.Tracer import org.apache.pekko.stream.Materializer import org.lfdecentralizedtrust.splice.automation.{ ReconcileSequencerLimitWithMemberTrafficTriggerBase, + TaskOutcome, TriggerContext, } +import org.lfdecentralizedtrust.splice.codegen.java.splice.decentralizedsynchronizer.MemberTraffic import org.lfdecentralizedtrust.splice.environment.SequencerAdminConnection import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore import org.lfdecentralizedtrust.splice.environment.SynchronizerNodeService import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode +import org.lfdecentralizedtrust.splice.util.AssignedContract import scala.concurrent.{ExecutionContext, Future} @@ -25,6 +29,7 @@ class ReconcileDedicatedSequencerTrafficTrigger( store: SyncOperatorStore, synchronizerNodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, + outageTrafficAllowance: Option[NonNegativeLong], )(implicit ec: ExecutionContext, mat: Materializer, @@ -34,6 +39,9 @@ class ReconcileDedicatedSequencerTrafficTrigger( trafficBalanceReconciliationDelay, ) { + // The purchases on record when this app started are the ones the allowance was set over. + private val startedAt = context.clock.now + override protected def sequencerAdminConnection()(implicit tc: TraceContext ): Future[SequencerAdminConnection] = @@ -49,4 +57,21 @@ class ReconcileDedicatedSequencerTrafficTrigger( ): Future[Either[String, Long]] = // No prior consumption to carry. Future.successful(Right(0L)) + + override def completeTask( + memberTraffic: AssignedContract[MemberTraffic.ContractId, MemberTraffic] + )(implicit tc: TraceContext): Future[TaskOutcome] = { + // Purchases are only made on the global synchronizer, so one that lands means it is back. The + // grant below only raises a limit to the purchased total, so it pays down the member's credit. + if ( + outageTrafficAllowance.isDefined && + memberTraffic.contract.createdAt.isAfter(startedAt.toInstant) + ) + logger.warn( + s"A traffic purchase for ${memberTraffic.payload.memberId} landed while the outage " + + "traffic allowance is set, so the global synchronizer is reachable again. Remove " + + "outage-traffic-allowance from this app's config and restart it." + ) + super.completeTask(memberTraffic) + } } diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala index 04c0b6a3df2..33ec4a0b076 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala @@ -4,6 +4,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.digitalasset.canton.config.NonNegativeFiniteDuration +import com.digitalasset.canton.config.RequireTypes.NonNegativeLong import com.digitalasset.canton.logging.NamedLoggerFactory import com.digitalasset.canton.resource.DbStorage import com.digitalasset.canton.time.Clock @@ -48,6 +49,8 @@ class SyncOperatorAutomationService( lsuConfig: Option[SyncOperatorLsuConfig], lsuDumpPath: Option[Path], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, + outageTrafficAllowance: Option[NonNegativeLong], + outageTrafficAllowanceWarningInterval: NonNegativeFiniteDuration, protected val loggerFactory: NamedLoggerFactory, packageVersionSupport: PackageVersionSupport, )(implicit @@ -99,6 +102,29 @@ class SyncOperatorAutomationService( store, synchronizerNodeService, trafficBalanceReconciliationDelay, + outageTrafficAllowance, + ) + ) + + // Also without an allowance, so that a restart after removing one sets the limits back. + registerTrigger( + new OutageTrafficAllowanceTrigger( + triggerContext, + store, + synchronizerNodeService, + outageTrafficAllowance, + trafficBalanceReconciliationDelay, + ) + ) + + outageTrafficAllowance.foreach(allowance => + registerTrigger( + new OutageTrafficAllowanceWarningTrigger( + outageTrafficAllowanceWarningInterval, + triggerContext, + store, + allowance, + ) ) ) @@ -155,6 +181,8 @@ object SyncOperatorAutomationService extends AutomationServiceCompanion { aTrigger[ReconcileDedicatedSynchronizerParametersTrigger], aTrigger[MediatorUnlimitedTrafficTrigger], aTrigger[ReconcileDedicatedSequencerTrafficTrigger], + aTrigger[OutageTrafficAllowanceTrigger], + aTrigger[OutageTrafficAllowanceWarningTrigger], aTrigger[DedicatedLsuAnnouncementTrigger], aTrigger[DedicatedLsuTrigger], aTrigger[LsuTransferTrafficTrigger], diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala index 33044bd6c12..2ccdd835c8c 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala @@ -9,7 +9,7 @@ import com.digitalasset.canton.admin.api.client.data.{ SynchronizerLimits, } import com.digitalasset.canton.config.* -import com.digitalasset.canton.config.RequireTypes.NonNegativeInt +import com.digitalasset.canton.config.RequireTypes.{NonNegativeInt, NonNegativeLong} import com.digitalasset.canton.version.ProtocolVersion import org.lfdecentralizedtrust.splice.config.{ AutomationConfig, @@ -96,6 +96,14 @@ case class SyncOperatorAppBackendConfig( parameters: SpliceParametersConfig = SpliceParametersConfig(batching = BatchingConfig()), trafficBalanceReconciliationDelay: NonNegativeFiniteDuration = NonNegativeFiniteDuration.ofSeconds(10), + // Bytes each member with a purchase on record may use past what has been bought for it. The + // operator sets it while the global synchronizer is unavailable, so nothing can be bought, and + // removes it once it is back, each time followed by a restart: on start, every such member's + // limit is set to its purchased total plus this, or back to its purchased total when unset. + outageTrafficAllowance: Option[NonNegativeLong] = None, + // How often this app warns that the outage traffic allowance is set. + outageTrafficAllowanceWarningInterval: NonNegativeFiniteDuration = + NonNegativeFiniteDuration.ofMinutes(5), // Set to false to disable the DB-level exclusive lock that prevents two sync operator instances // from running concurrently against the same database. Only disable for migration scenarios // where intentional overlap is required. diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala index 6e0756994bd..eae0711758b 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala @@ -37,6 +37,13 @@ trait SyncOperatorStore extends AppStore { def getTotalPurchasedMemberTraffic(memberId: Member)(implicit tc: TraceContext ): Future[Long] + + /** Total traffic purchased on this operator's synchronizer, per member with a purchase on + * record. + */ + def listTotalPurchasedMemberTraffic()(implicit + tc: TraceContext + ): Future[Map[Member, Long]] } object SyncOperatorStore { diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala index 8c8f126cb75..b945fe705ef 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala @@ -94,4 +94,16 @@ class DbSyncOperatorStore( key.synchronizerId, ) } + + override def listTotalPurchasedMemberTraffic()(implicit + tc: TraceContext + ): Future[Map[Member, Long]] = waitUntilAcsIngested { + sumPurchasedMemberTrafficPerMember( + storage, + SyncOperatorTables.acsTableName, + acsStoreId, + domainMigrationId, + key.synchronizerId, + ) + } } diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala index 3e2e038659b..5b1e5fd50e9 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala @@ -141,6 +141,31 @@ abstract class SyncOperatorStoreTest extends StoreTestBase with HasExecutionCont total <- store.getTotalPurchasedMemberTraffic(alice) } yield total shouldBe 0L } + + "list the total of every member with a purchase for this synchronizer" in { + for { + store <- mkStore() + _ <- ingest( + store, + Seq( + memberTraffic(alice, 100L), + memberTraffic(alice, 250L), + memberTraffic(bob, 700L), + memberTraffic(bob, 900L, synchronizerId = foreignSynchronizer), + memberTraffic(bob, 300L, operator = Some(otherOperator)), + ), + ) + totals <- store.listTotalPurchasedMemberTraffic() + } yield totals shouldBe Map(alice -> 350L, bob -> 700L) + } + + "list no totals before any purchase" in { + for { + store <- mkStore() + _ <- ingest(store, Seq.empty) + totals <- store.listTotalPurchasedMemberTraffic() + } yield totals shouldBe empty + } } } diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala new file mode 100644 index 00000000000..df86c738d22 --- /dev/null +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala @@ -0,0 +1,116 @@ +// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package org.lfdecentralizedtrust.splice.syncoperator.automation + +import com.digitalasset.canton.BaseTest +import com.digitalasset.canton.topology.{MediatorId, Member, ParticipantId, UniqueIdentifier} +import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTrigger.Task +import org.scalatest.wordspec.AnyWordSpec + +class OutageTrafficAllowanceTriggerTest extends AnyWordSpec with BaseTest { + + private val alice: Member = ParticipantId.tryFromProtoPrimitive("PAR::alice::default") + private val bob: Member = ParticipantId.tryFromProtoPrimitive("PAR::bob::default") + private val carol: Member = ParticipantId.tryFromProtoPrimitive("PAR::carol::default") + private val mediator: Member = MediatorId(UniqueIdentifier.tryCreate("mediator", "default")) + + private val allowance = Some(50L) + + "OutageTrafficAllowanceTrigger.tasks" should { + + "raise each member to its purchased total plus the allowance" in { + OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L, bob -> 200L), + limits = Map(alice -> 100L, bob -> 200L), + allowance, + ) shouldBe Seq(Task(alice, 100L, allowance), Task(bob, 200L, allowance)) + } + + "lower a limit a larger allowance raised to the total plus a smaller one" in { + val tasks = OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L), + limits = Map(alice -> 200L), + allowance, + ) + tasks shouldBe Seq(Task(alice, 100L, allowance)) + tasks.map(_.target) shouldBe Seq(150L) + } + + "leave a member already at its total plus the allowance alone, so a restart adds nothing" in { + OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L), + limits = Map(alice -> 150L), + allowance, + ) shouldBe empty + } + + "set each limit back to the purchased total once the allowance is removed" in { + val tasks = OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L, bob -> 200L), + limits = Map(alice -> 150L, bob -> 200L), + allowance = None, + ) + tasks shouldBe Seq(Task(alice, 100L, None)) + tasks.map(_.target) shouldBe Seq(100L) + } + + "raise a limit left below its total by a purchase ingested while the allowance was removed" in { + OutageTrafficAllowanceTrigger + .tasks( + totals = Map(alice -> 300L), + limits = Map(alice -> 100L), + allowance = None, + ) + .map(_.target) shouldBe Seq(300L) + } + + "skip a member the sequencer has no traffic state for" in { + OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L, carol -> 10L), + limits = Map(alice -> 100L), + allowance, + ) shouldBe Seq(Task(alice, 100L, allowance)) + } + + "leave a member with no purchase on record alone" in { + OutageTrafficAllowanceTrigger.tasks( + totals = Map(alice -> 100L), + limits = Map(alice -> 100L, carol -> 0L), + allowance, + ) shouldBe Seq(Task(alice, 100L, allowance)) + } + + "leave a mediator's unlimited traffic alone, even with a purchase on record" in { + forEvery(Seq(allowance, None)) { allowance => + OutageTrafficAllowanceTrigger.tasks( + totals = Map(mediator -> 10L), + limits = Map(mediator -> Long.MaxValue), + allowance, + ) shouldBe empty + } + } + + "list the members in a fixed order" in { + OutageTrafficAllowanceTrigger + .tasks( + totals = Map(carol -> 1L, alice -> 1L, bob -> 1L), + limits = Map(carol -> 1L, alice -> 1L, bob -> 1L), + allowance, + ) + .map(_.member) shouldBe Seq(alice, bob, carol) + } + } + + "OutageTrafficAllowanceTrigger.Task" should { + + "cap a target the allowance would take past the largest limit" in { + Task(alice, 100L, Some(Long.MaxValue)).target shouldBe Long.MaxValue + } + + "tell the operator the gap of a limit it could not bring back to the purchased total" in { + Task(alice, 100L, None).notApplied(150L) should include("50 bytes above") + Task(alice, 100L, None).notApplied(80L) should include("20 bytes below") + } + } +} diff --git a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala index d25083443a7..43d797918e7 100644 --- a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala +++ b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala @@ -131,7 +131,7 @@ class TopupMemberTrafficTrigger( _ = unfunded.foreach(task => logger.warn( s"Insufficient funds to buy configured traffic amount. Please ensure that the validator's wallet has enough amulets to purchase " + - s"${BigDecimal(task.target.topupParameters.topupAmount) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." + s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." ) ) } yield funded @@ -166,12 +166,15 @@ class TopupMemberTrafficTrigger( if (!wantsTopup(target, currentTrafficState, topupState)) Future.successful(None) else TopupUtil - // The cost is derived from this target's throughput and interval. + // The cost is derived from this target's throughput and interval, plus any + // shortfall the top-up also buys. .minWalletBalanceForTopup( scanConnection, target.topupConfig, clock, registration, + extraTraffic = TopupMemberTrafficTrigger + .shortfall(currentTrafficState.extraTrafficRemainder), ) .map(Some(_)) } yield cost.map( @@ -188,7 +191,7 @@ class TopupMemberTrafficTrigger( task: TopupMemberTrafficTrigger.Task )(implicit tc: TraceContext): Future[TaskOutcome] = { val coBuyMemberTraffic = new CO_BuyMemberTraffic( - task.target.topupParameters.topupAmount, + task.trafficToBuy, task.topupState.payload.memberId, task.topupState.payload.synchronizerId, task.topupState.payload.migrationId, @@ -422,6 +425,17 @@ object TopupMemberTrafficTrigger { // Two aliases can resolve to one synchronizer; topupTargets is global-first, so global wins. .distinctBy(_.synchronizerId) + /** Traffic below zero, as once a dedicated synchronizer's operator removes an outage traffic + * allowance the member used. The member cannot submit until its purchases cover it. + */ + def shortfall(extraTrafficRemainder: Long): Long = math.max(0L, -extraTrafficRemainder) + + /** The configured top-up plus any shortfall, bought in one purchase: the configured amount alone + * would leave the member blocked until several top-ups had covered the shortfall. + */ + def trafficToBuy(topupAmount: Long, extraTrafficRemainder: Long): Long = + topupAmount + shortfall(extraTrafficRemainder) + final case class Task( target: Target, topupState: Contract[ValidatorTopUpState.ContractId, ValidatorTopUpState], @@ -430,6 +444,13 @@ object TopupMemberTrafficTrigger { ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], ) extends PrettyPrinting { + + def trafficToBuy: Long = + TopupMemberTrafficTrigger.trafficToBuy( + target.topupParameters.topupAmount, + trafficState.extraTrafficRemainder, + ) + override def pretty: Pretty[Task] = prettyOfClass[Task]( param("target", _.target), diff --git a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala index 7e06f67f5cc..a913c35f548 100644 --- a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala +++ b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala @@ -126,4 +126,37 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { unfunded shouldBe empty } } + + "TopupMemberTrafficTrigger.shortfall" should { + + "be what a balance below zero needs to reach zero" in { + TopupMemberTrafficTrigger.shortfall(-300L) shouldBe 300L + } + + "be zero for a balance at or above zero" in { + TopupMemberTrafficTrigger.shortfall(0L) shouldBe 0L + TopupMemberTrafficTrigger.shortfall(500L) shouldBe 0L + } + } + + "TopupMemberTrafficTrigger.trafficToBuy" should { + + "buy the shortfall together with the configured top-up" in { + TopupMemberTrafficTrigger.trafficToBuy( + topupAmount = 2_000L, + extraTrafficRemainder = -300L, + ) shouldBe 2_300L + } + + "buy only the configured top-up for a balance at or above zero" in { + TopupMemberTrafficTrigger.trafficToBuy( + topupAmount = 2_000L, + extraTrafficRemainder = 0L, + ) shouldBe 2_000L + TopupMemberTrafficTrigger.trafficToBuy( + topupAmount = 2_000L, + extraTrafficRemainder = 500L, + ) shouldBe 2_000L + } + } } diff --git a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala index a8c2b7c119c..25e2cc6008f 100644 --- a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala +++ b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala @@ -30,6 +30,7 @@ object TopupUtil { .map(_.payload.governanceParameters) .fold(BigDecimal(1))(p => BigDecimal(p.discountFactor)) + /** The cost of one top-up, plus `extraTraffic` bought with it. */ def minWalletBalanceForTopup( scanConnection: ScanConnection, validatorTopupConfig: ValidatorTopupConfig, @@ -37,6 +38,7 @@ object TopupUtil { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], + extraTraffic: Long = 0L, )(implicit tc: TraceContext, ec: ExecutionContext, mat: Materializer): Future[BigDecimal] = for { amuletRules <- scanConnection.getAmuletRulesWithState() synchronizerFeesConfig = AmuletConfigSchedule(amuletRules) @@ -50,16 +52,32 @@ object TopupUtil { validatorTopupConfig.topupTriggerPollingInterval, ) latestRound <- scanConnection.getLatestOpenMiningRound() - amuletPrice = latestRound.payload.amuletPrice - extraTrafficPrice = BigDecimal(synchronizerFeesConfig.extraTrafficPrice) - } yield SpliceUtil - .synchronizerFees( - topupParameters.topupAmount, - extraTrafficPrice, - amuletPrice, - discountFactor(registration), - ) - ._2 + } yield topupCost( + topupParameters, + extraTraffic, + BigDecimal(synchronizerFeesConfig.extraTrafficPrice), + latestRound.payload.amuletPrice, + registration, + ) + + /** What one top-up, plus `extraTraffic` bought with it, costs in amulet. */ + def topupCost( + topupParameters: ExtraTrafficTopupParameters, + extraTraffic: Long, + extraTrafficPrice: BigDecimal, + amuletPrice: BigDecimal, + registration: Option[ + ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] + ], + ): BigDecimal = + SpliceUtil + .synchronizerFees( + topupParameters.topupAmount + extraTraffic, + extraTrafficPrice, + amuletPrice, + discountFactor(registration), + ) + ._2 private def currentWalletBalance(scanConnection: ScanConnection, store: UserWalletStore)(implicit tc: TraceContext, diff --git a/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala b/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala new file mode 100644 index 00000000000..89906b9b08a --- /dev/null +++ b/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala @@ -0,0 +1,40 @@ +// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package org.lfdecentralizedtrust.splice.wallet.util + +import com.digitalasset.canton.BaseTestWordSpec +import com.digitalasset.canton.config.NonNegativeFiniteDuration + +class TopupUtilTest extends BaseTestWordSpec { + + // 2,000 bytes per top-up at $60 per MB is $0.12, or 24 amulets at $0.005 each. + private val topupParameters = + ExtraTrafficTopupParameters(2_000L, NonNegativeFiniteDuration.ofMinutes(1)) + private val extraTrafficPrice = BigDecimal(60) + private val amuletPrice = BigDecimal("0.005") + + "TopupUtil.topupCost" should { + + "price one top-up" in { + TopupUtil.topupCost( + topupParameters, + extraTraffic = 0L, + extraTrafficPrice, + amuletPrice, + registration = None, + ) shouldBe BigDecimal(24) + } + + "price a shortfall bought with the top-up as part of it" in { + // 5,000 bytes in all, so the funds check covers everything the purchase buys. + TopupUtil.topupCost( + topupParameters, + extraTraffic = 3_000L, + extraTrafficPrice, + amuletPrice, + registration = None, + ) shouldBe BigDecimal(60) + } + } +} diff --git a/test-full-class-names-non-integration.log b/test-full-class-names-non-integration.log index 483884d369b..9bf1b3fa39c 100644 --- a/test-full-class-names-non-integration.log +++ b/test-full-class-names-non-integration.log @@ -59,6 +59,7 @@ org.lfdecentralizedtrust.splice.sv.cometbft.CometBftRequestSignerTest org.lfdecentralizedtrust.splice.sv.onboarding.SequencerBftPeerReconcilerSpec org.lfdecentralizedtrust.splice.sv.util.SvOnboardingTokenTest org.lfdecentralizedtrust.splice.sv.util.WeightDistributionForSvTest +org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTriggerTest org.lfdecentralizedtrust.splice.unit.http.HttpClientProxyTest org.lfdecentralizedtrust.splice.unit.store.db.AcsJdbcTypesTest org.lfdecentralizedtrust.splice.util.AmuletConfigScheduleTest @@ -74,4 +75,5 @@ org.lfdecentralizedtrust.splice.validator.automation.TopupMemberTrafficTriggerTe org.lfdecentralizedtrust.splice.validator.store.DbValidatorConfigProviderTest org.lfdecentralizedtrust.splice.validator.store.DbValidatorStoreTest org.lfdecentralizedtrust.splice.wallet.util.ExtraTrafficTopupParametersTest +org.lfdecentralizedtrust.splice.wallet.util.TopupUtilTest org.lfdecentralizedtrust.splice.wart.ParTraverseTest From 308b1c450f7d4ed8e2633886951f9a37f3b411b0 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Tue, 6 Oct 2026 15:38:40 -0400 Subject: [PATCH 02/11] fix: tighten the outage traffic allowance after review - Read a member's purchased total again before its grant, after the traffic state, so a purchase granted since the sweep retrieved its tasks is in the target instead of taken back until the next poll. - When the sweep settles with the allowance set, warn once about members with a purchase on record but no traffic state on the sequencer, which it leaves out. - Word the purchase warning for the DSO's merges as well, and log it once the grant succeeds, so a retried grant warns once. - Warn about a limit that could not be set at most once per warning interval for each member, with the same text at info in between. - Require a positive allowance and warning interval: 0 counted as set, and a 0 interval ran the periodic warning in a tight loop. Signed-off-by: Timothy Wu --- .../SyncOperatorTrafficIntegrationTest.scala | 11 +- .../OutageTrafficAllowanceTrigger.scala | 116 ++++++++++++++---- ...OutageTrafficAllowanceWarningTrigger.scala | 4 +- ...cileDedicatedSequencerTrafficTrigger.scala | 34 ++--- .../SyncOperatorAutomationService.scala | 11 +- .../config/SyncOperatorAppConfig.scala | 11 +- .../OutageTrafficAllowanceTriggerTest.scala | 38 ++++++ 7 files changed, 172 insertions(+), 53 deletions(-) diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index f93440d63b5..5e66685dce7 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -6,7 +6,11 @@ package org.lfdecentralizedtrust.splice.integration.tests import com.digitalasset.canton.SynchronizerAlias import com.digitalasset.canton.config.CantonRequireTypes.InstanceName import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, NonNegativeNumeric} +import com.digitalasset.canton.config.RequireTypes.{ + NonNegativeLong, + NonNegativeNumeric, + PositiveLong, +} import com.digitalasset.canton.logging.SuppressionRule import com.digitalasset.canton.topology.{Member, SynchronizerId} import monocle.macros.syntax.lens.* @@ -140,7 +144,7 @@ class SyncOperatorTrafficIntegrationTest ) // Last, so it is a copy of syncOperator as every transform above left it. .addConfigTransform { (_, conf) => - val allowance = Some(NonNegativeLong.tryCreate(outageTrafficAllowance)) + val allowance = Some(PositiveLong.tryCreate(outageTrafficAllowance)) val syncOperator = conf.syncOperatorApps(InstanceName.tryCreate("syncOperator")) conf.copy(syncOperatorApps = conf.syncOperatorApps.updated( @@ -519,7 +523,8 @@ class SyncOperatorTrafficIntegrationTest private val allowanceSetWarning = s"The outage traffic allowance of $outageTrafficAllowance bytes is set, covering 2 members" - private val purchaseLandedWarning = "landed while the outage traffic allowance is set" + private val purchaseLandedWarning = + "was bought or merged on the global synchronizer while the outage traffic allowance is set" // The warnings suppressed so far in the enclosing log assertion, for waiting on one. private def recordedWarnings(text: String) = diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala index bbb394e11df..7c35139c253 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala @@ -4,8 +4,9 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import cats.syntax.apply.* -import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.config.{NonNegativeFiniteDuration, PositiveFiniteDuration} +import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLong} +import com.digitalasset.canton.data.CantonTimestamp import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} import com.digitalasset.canton.topology.{MediatorId, Member} import com.digitalasset.canton.tracing.TraceContext @@ -15,6 +16,7 @@ import org.apache.pekko.stream.Materializer import org.lfdecentralizedtrust.splice.automation.{ PollingParallelTaskExecutionTrigger, TaskFailed, + TaskNoop, TaskOutcome, TaskSuccess, TriggerContext, @@ -26,6 +28,7 @@ import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllo import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore import java.util.concurrent.atomic.AtomicBoolean +import scala.collection.concurrent.TrieMap import scala.concurrent.{ExecutionContext, Future} /** Sets each member's traffic limit on this operator's sequencer from the outage traffic allowance @@ -53,8 +56,9 @@ class OutageTrafficAllowanceTrigger( override protected val context: TriggerContext, store: SyncOperatorStore, synchronizerNodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], - allowance: Option[NonNegativeLong], + allowance: Option[PositiveLong], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, + warningInterval: PositiveFiniteDuration, )(implicit override val ec: ExecutionContext, mat: Materializer, @@ -64,6 +68,9 @@ class OutageTrafficAllowanceTrigger( // Set once a poll finds every member at its target, after which polls do nothing. private val settled = new AtomicBoolean(false) + // When each member was last warned about a limit that could not be set. + private val lastWarnedAt = TrieMap.empty[Member, CantonTimestamp] + override protected def retrieveTasks()(implicit tc: TraceContext): Future[Seq[Task]] = if (settled.get()) Future.successful(Seq.empty) else @@ -82,7 +89,19 @@ class OutageTrafficAllowanceTrigger( val target = allowance.fold("its purchased total")(a => s"its purchased total plus the outage traffic allowance of ${a.value} bytes" ) - logger.info(s"Every member with a purchase on record is at $target") + logger.info( + s"Every member with a purchase on record and a traffic state on the sequencer is at $target" + ) + val skipped = OutageTrafficAllowanceTrigger.withoutTrafficState(totals, limits) + allowance.foreach(a => + if (skipped.nonEmpty) + logger.warn( + s"These members have a purchase on record but no traffic state on this " + + s"synchronizer's sequencer, so they did not get the outage traffic allowance of " + + s"${a.value} bytes: ${skipped.mkString(", ")}. They only get it if this app is " + + "restarted once the sequencer has a traffic state for them." + ) + ) } tasks } @@ -95,32 +114,66 @@ class OutageTrafficAllowanceTrigger( connection.getSequencerTrafficControlState(task.member), connection.getSequencerSynchronizerState(TopologySnapshot.Effective), ).tupled + // And the total after it: a purchase granted since the task was retrieved is then either in + // the total, or its grant moved the serial and this one is refused and retried. + current <- currentTask(task) limit = trafficState.extraTrafficLimit.value - outcome <- connection - .setSequencerTrafficControlState( - trafficState, - sequencerState, - NonNegativeLong.tryCreate(task.target), - context.pollingClock, - trafficBalanceReconciliationDelay, - ) - .map[TaskOutcome](_ => - TaskSuccess(s"Set the traffic limit of ${task.member} from $limit to ${task.target}") - ) - .recover { - // The grant did not land, for example because too few of the synchronizer's operators - // sent the same one. The next poll tries again, as the targets are not all in place. - case ex: StatusRuntimeException - if ex.getStatus.getCode == Status.Code.DEADLINE_EXCEEDED => - TaskFailed(task.notApplied(limit)) - } + outcome <- + if (current.isDone(limit)) + Future.successful( + TaskSuccess(s"The traffic limit of ${task.member} is already at its target of $limit") + ) + else + connection + .setSequencerTrafficControlState( + trafficState, + sequencerState, + NonNegativeLong.tryCreate(current.target), + context.pollingClock, + trafficBalanceReconciliationDelay, + ) + .map[TaskOutcome](_ => + TaskSuccess( + s"Set the traffic limit of ${task.member} from $limit to ${current.target}" + ) + ) + .recover { + // The grant did not land, for example because too few of the synchronizer's + // operators sent the same one. The next poll tries again, as the targets are not + // all in place. + case ex: StatusRuntimeException + if ex.getStatus.getCode == Status.Code.DEADLINE_EXCEEDED => + notApplied(task.member, current.notApplied(limit)) + } } yield outcome override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = for { connection <- synchronizerNodeService.sequencerAdminConnection() state <- connection.lookupSequencerTrafficControlState(task.member) - } yield state.forall(state => task.isDone(state.extraTrafficLimit.value)) + current <- currentTask(task) + } yield state.forall(state => current.isDone(state.extraTrafficLimit.value)) + + // A warning at most once per warning interval for each member, and the same text in the info log + // in between: on a synchronizer whose operators set the allowance at different times, a limit + // can stay out of reach for many polls. + private def notApplied(member: Member, message: String)(implicit + tc: TraceContext + ): TaskOutcome = { + val now = context.clock.now + if (OutageTrafficAllowanceTrigger.warningDue(lastWarnedAt.get(member), now, warningInterval)) { + lastWarnedAt.update(member, now) + TaskFailed(message) + } else { + logger.info(message) + TaskNoop + } + } + + // The task with the member's purchased total as it is now. A target from the total the task was + // retrieved with would take back a purchase the reconcile trigger granted since. + private def currentTask(task: Task)(implicit tc: TraceContext): Future[Task] = + store.getTotalPurchasedMemberTraffic(task.member).map(total => task.copy(total = total)) } object OutageTrafficAllowanceTrigger { @@ -152,6 +205,23 @@ object OutageTrafficAllowanceTrigger { } } + /** The members with a purchase on record that the allowance applies to but the sequencer has no + * traffic state for, so a sweep leaves them out, in a fixed order. + */ + def withoutTrafficState(totals: Map[Member, Long], limits: Map[Member, Long]): Seq[Member] = + totals.keys + .filter(member => isCovered(member) && !limits.contains(member)) + .toSeq + .sortBy(_.toProtoPrimitive) + + /** Whether a warning last given at `lastWarned` is due again at `now`. */ + def warningDue( + lastWarned: Option[CantonTimestamp], + now: CantonTimestamp, + interval: PositiveFiniteDuration, + ): Boolean = + lastWarned.forall(last => !now.isBefore(last.plus(interval.asJava))) + final case class Task(member: Member, total: Long, allowance: Option[Long]) extends PrettyPrinting { diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala index 92a682bf7fa..50959a7bc72 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala @@ -4,7 +4,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.config.RequireTypes.PositiveLong import com.digitalasset.canton.tracing.TraceContext import io.opentelemetry.api.trace.Tracer import org.lfdecentralizedtrust.splice.automation.{ @@ -24,7 +24,7 @@ class OutageTrafficAllowanceWarningTrigger( interval: NonNegativeFiniteDuration, triggerContext: TriggerContext, store: SyncOperatorStore, - allowance: NonNegativeLong, + allowance: PositiveLong, )(implicit override val ec: ExecutionContext, override val tracer: Tracer, diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index ed251602ff5..f0da7bfc323 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -4,7 +4,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.config.RequireTypes.PositiveLong import com.digitalasset.canton.topology.Member import com.digitalasset.canton.tracing.TraceContext import io.opentelemetry.api.trace.Tracer @@ -29,7 +29,7 @@ class ReconcileDedicatedSequencerTrafficTrigger( store: SyncOperatorStore, synchronizerNodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, - outageTrafficAllowance: Option[NonNegativeLong], + outageTrafficAllowance: Option[PositiveLong], )(implicit ec: ExecutionContext, mat: Materializer, @@ -60,18 +60,22 @@ class ReconcileDedicatedSequencerTrafficTrigger( override def completeTask( memberTraffic: AssignedContract[MemberTraffic.ContractId, MemberTraffic] - )(implicit tc: TraceContext): Future[TaskOutcome] = { - // Purchases are only made on the global synchronizer, so one that lands means it is back. The - // grant below only raises a limit to the purchased total, so it pays down the member's credit. - if ( - outageTrafficAllowance.isDefined && - memberTraffic.contract.createdAt.isAfter(startedAt.toInstant) - ) - logger.warn( - s"A traffic purchase for ${memberTraffic.payload.memberId} landed while the outage " + - "traffic allowance is set, so the global synchronizer is reachable again. Remove " + - "outage-traffic-allowance from this app's config and restart it." + )(implicit tc: TraceContext): Future[TaskOutcome] = + // The grant only raises a limit to the purchased total, so while the allowance is set it pays + // down the member's credit. The warning follows it, so a grant that is retried warns once. + super.completeTask(memberTraffic).map { outcome => + // Purchases, and the DSO's merges of them, only happen on the global synchronizer, so a + // contract created since this app started means it is back. + if ( + outageTrafficAllowance.isDefined && + memberTraffic.contract.createdAt.isAfter(startedAt.toInstant) ) - super.completeTask(memberTraffic) - } + logger.warn( + s"Traffic for ${memberTraffic.payload.memberId} was bought or merged on the global " + + "synchronizer while the outage traffic allowance is set, so the global synchronizer " + + "is reachable again. Remove outage-traffic-allowance from this app's config and " + + "restart it." + ) + outcome + } } diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala index 33ec4a0b076..fe548397ea5 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala @@ -3,8 +3,8 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation -import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.NonNegativeLong +import com.digitalasset.canton.config.{NonNegativeFiniteDuration, PositiveFiniteDuration} +import com.digitalasset.canton.config.RequireTypes.PositiveLong import com.digitalasset.canton.logging.NamedLoggerFactory import com.digitalasset.canton.resource.DbStorage import com.digitalasset.canton.time.Clock @@ -49,8 +49,8 @@ class SyncOperatorAutomationService( lsuConfig: Option[SyncOperatorLsuConfig], lsuDumpPath: Option[Path], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, - outageTrafficAllowance: Option[NonNegativeLong], - outageTrafficAllowanceWarningInterval: NonNegativeFiniteDuration, + outageTrafficAllowance: Option[PositiveLong], + outageTrafficAllowanceWarningInterval: PositiveFiniteDuration, protected val loggerFactory: NamedLoggerFactory, packageVersionSupport: PackageVersionSupport, )(implicit @@ -114,13 +114,14 @@ class SyncOperatorAutomationService( synchronizerNodeService, outageTrafficAllowance, trafficBalanceReconciliationDelay, + outageTrafficAllowanceWarningInterval, ) ) outageTrafficAllowance.foreach(allowance => registerTrigger( new OutageTrafficAllowanceWarningTrigger( - outageTrafficAllowanceWarningInterval, + NonNegativeFiniteDuration(outageTrafficAllowanceWarningInterval.underlying), triggerContext, store, allowance, diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala index 2ccdd835c8c..deaf21f3c8f 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala @@ -9,7 +9,7 @@ import com.digitalasset.canton.admin.api.client.data.{ SynchronizerLimits, } import com.digitalasset.canton.config.* -import com.digitalasset.canton.config.RequireTypes.{NonNegativeInt, NonNegativeLong} +import com.digitalasset.canton.config.RequireTypes.{NonNegativeInt, PositiveLong} import com.digitalasset.canton.version.ProtocolVersion import org.lfdecentralizedtrust.splice.config.{ AutomationConfig, @@ -100,10 +100,11 @@ case class SyncOperatorAppBackendConfig( // operator sets it while the global synchronizer is unavailable, so nothing can be bought, and // removes it once it is back, each time followed by a restart: on start, every such member's // limit is set to its purchased total plus this, or back to its purchased total when unset. - outageTrafficAllowance: Option[NonNegativeLong] = None, - // How often this app warns that the outage traffic allowance is set. - outageTrafficAllowanceWarningInterval: NonNegativeFiniteDuration = - NonNegativeFiniteDuration.ofMinutes(5), + outageTrafficAllowance: Option[PositiveLong] = None, + // How often this app warns that the outage traffic allowance is set, and at most how often it + // warns about each member whose limit it could not set. + outageTrafficAllowanceWarningInterval: PositiveFiniteDuration = + PositiveFiniteDuration.ofMinutes(5), // Set to false to disable the DB-level exclusive lock that prevents two sync operator instances // from running concurrently against the same database. Only disable for migration scenarios // where intentional overlap is required. diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala index df86c738d22..47b277138c1 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala @@ -4,6 +4,8 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.digitalasset.canton.BaseTest +import com.digitalasset.canton.config.PositiveFiniteDuration +import com.digitalasset.canton.data.CantonTimestamp import com.digitalasset.canton.topology.{MediatorId, Member, ParticipantId, UniqueIdentifier} import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTrigger.Task import org.scalatest.wordspec.AnyWordSpec @@ -113,4 +115,40 @@ class OutageTrafficAllowanceTriggerTest extends AnyWordSpec with BaseTest { Task(alice, 100L, None).notApplied(80L) should include("20 bytes below") } } + + "OutageTrafficAllowanceTrigger.withoutTrafficState" should { + + "list the members with a purchase on record the sequencer has no traffic state for" in { + OutageTrafficAllowanceTrigger.withoutTrafficState( + totals = Map(carol -> 100L, alice -> 100L, bob -> 200L, mediator -> 300L), + limits = Map(alice -> 100L), + ) shouldBe Seq(bob, carol) + } + } + + "OutageTrafficAllowanceTrigger.warningDue" should { + + val interval = PositiveFiniteDuration.ofMinutes(5) + val warnedAt = CantonTimestamp.Epoch + + "warn about a member not warned about before" in { + OutageTrafficAllowanceTrigger.warningDue(None, warnedAt, interval) shouldBe true + } + + "not warn again within the interval" in { + OutageTrafficAllowanceTrigger.warningDue( + Some(warnedAt), + warnedAt.plusSeconds(299), + interval, + ) shouldBe false + } + + "warn again once the interval has passed" in { + OutageTrafficAllowanceTrigger.warningDue( + Some(warnedAt), + warnedAt.plusSeconds(300), + interval, + ) shouldBe true + } + } } From bbea08c43fee957ac25dc7e481bf3d4847817d4c Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Tue, 6 Oct 2026 15:40:33 -0400 Subject: [PATCH 03/11] fix: fall back to the plain top-up when the wallet does not cover the shortfall The top-up bought the configured amount plus the whole shortfall or nothing, so a wallet that covered the top-up alone left a member below zero for good, where before it paid the shortfall down one interval at a time. A member below zero now gets the top-up alone when the wallet does not cover both, with a warning that it stays below zero for now. The insufficient-funds warning fires only when even the top-up alone does not fit. Signed-off-by: Timothy Wu --- .../TopupMemberTrafficTrigger.scala | 120 +++++++++++------- .../TopupMemberTrafficTriggerTest.scala | 26 +++- 2 files changed, 102 insertions(+), 44 deletions(-) diff --git a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala index 43d797918e7..691ddc064c4 100644 --- a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala +++ b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala @@ -116,16 +116,19 @@ class TopupMemberTrafficTrigger( ) validatorWallet <- ValidatorUtil.getValidatorWallet(store, walletManager) budget <- TopupUtil.topupBudget(scanConnection, validatorWallet.store) - candidates <- MonadUtil.sequentialTraverseFilter(targets)(target => + candidates <- MonadUtil.sequentialTraverse(targets)(target => // The traversal is sequential, so a failure would otherwise cost every remaining target // its top-up for this poll. Info, not warn: a synchronizer is legitimately unreachable // for a stretch of polls while it is being upgraded or recovered. retrieveTaskFor(target, activeSynchronizerId).recover { case ex => logger.info(s"Skipping the top-up for ${target.alias} in this poll", ex) - None + Seq.empty } ) - (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks(candidates, budget) + (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks( + candidates.filter(_.nonEmpty), + budget, + ) // we do not even submit the topup tx if the validator does not have sufficient funds because we know // the tx would fail but it would still drain synchronizer traffic which we would like to avoid (see #11915). _ = unfunded.foreach(task => @@ -134,16 +137,28 @@ class TopupMemberTrafficTrigger( s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." ) ) + _ = funded + .filterNot(_.withShortfall) + .foreach(task => + logger.warn( + s"Insufficient funds to buy the ${BigDecimal(task.shortfall) / 1e6} MB of traffic this member is below zero on ${task.target.alias} " + + "together with the configured top-up, so only the top-up is bought, and the member cannot submit there until its top-ups have covered it. " + + "Please ensure that the validator's wallet has enough amulets to buy both." + ) + ) } yield funded } - /** The task this target is due, paired with what it would cost the wallet. */ + /** The purchases this target is due, preferred first, each paired with what it would cost the + * wallet: for a member below zero, its shortfall together with the configured top-up, and the + * top-up alone in case the wallet does not cover both. + */ private def retrieveTaskFor( target: TopupMemberTrafficTrigger.Target, submissionSynchronizerId: SynchronizerId, )(implicit tc: TraceContext - ): Future[Option[(TopupMemberTrafficTrigger.Task, BigDecimal)]] = + ): Future[Seq[(TopupMemberTrafficTrigger.Task, BigDecimal)]] = for { currentTrafficState <- participantAdminConnection.getParticipantTrafficState( target.synchronizerId @@ -151,41 +166,48 @@ class TopupMemberTrafficTrigger( registration <- if (!target.needsRegistration) Future.successful(None) else scanConnection.lookupSynchronizerRegistration(target.synchronizerId.toProtoPrimitive) - task <- + purchases <- if (target.needsRegistration && registration.isEmpty) { // A buy without it is rejected on-ledger, and lastPurchasedAt never advances, so // nothing would stop the retry. logger.info( s"Not topping up ${target.alias}: Scan serves no registration for ${target.synchronizerId}" ) - Future.successful(None) + Future.successful(Seq.empty) } else for { topupState <- getOrCreateValidatorTopupState(target, submissionSynchronizerId) - cost <- - if (!wantsTopup(target, currentTrafficState, topupState)) Future.successful(None) - else - TopupUtil - // The cost is derived from this target's throughput and interval, plus any - // shortfall the top-up also buys. - .minWalletBalanceForTopup( - scanConnection, - target.topupConfig, - clock, - registration, - extraTraffic = TopupMemberTrafficTrigger - .shortfall(currentTrafficState.extraTrafficRemainder), - ) - .map(Some(_)) - } yield cost.map( - TopupMemberTrafficTrigger.Task( - target, - topupState, - currentTrafficState, - registration, - ) -> _ - ) - } yield task + tasks <- + if (!wantsTopup(target, currentTrafficState, topupState)) Future.successful(Seq.empty) + else { + val shortfall = + TopupMemberTrafficTrigger.shortfall(currentTrafficState.extraTrafficRemainder) + // The top-up alone still pays a shortfall down, one interval at a time. + val withShortfallOptions = if (shortfall > 0) Seq(true, false) else Seq(true) + Future.traverse(withShortfallOptions)(withShortfall => + TopupUtil + // The cost is derived from this target's throughput and interval, plus the + // shortfall where the purchase covers it. + .minWalletBalanceForTopup( + scanConnection, + target.topupConfig, + clock, + registration, + extraTraffic = if (withShortfall) shortfall else 0L, + ) + .map( + TopupMemberTrafficTrigger.Task( + target, + topupState, + currentTrafficState, + registration, + withShortfall, + ) -> _ + ) + ) + } + } yield tasks + } yield purchases override protected def completeTask( task: TopupMemberTrafficTrigger.Task @@ -357,21 +379,25 @@ object TopupMemberTrafficTrigger { ) } - /** Splits candidates into the ones the balance covers and the ones it does not, in target order. - * Every buy draws on the same wallet, so a target is only funded once the ones before it are. - * A `None` budget does not bound the purchases. + /** Splits candidates into the purchases the balance covers and the candidates it does not, in + * target order. Each candidate lists its purchases preferred first and gets the first one the + * balance covers; one it covers none of is reported by its last, cheapest, purchase. Every buy + * draws on the same wallet, so a target is only funded once the ones before it are. A `None` + * budget does not bound the purchases. */ private[automation] def fundedTasks[A]( - candidates: Seq[(A, BigDecimal)], + candidates: Seq[Seq[(A, BigDecimal)]], budget: Option[BigDecimal], ): (Seq[A], Seq[A]) = budget match { - case None => (candidates.map(_._1), Seq.empty) + case None => (candidates.flatMap(_.headOption.map(_._1)), Seq.empty) case Some(balance) => val (funded, unfunded, _) = candidates.foldLeft((Seq.empty[A], Seq.empty[A], balance)) { - case ((funded, unfunded, remaining), (task, cost)) => - if (cost <= remaining) (funded :+ task, unfunded, remaining - cost) - else (funded, unfunded :+ task, remaining) + case ((funded, unfunded, remaining), purchases) => + purchases.find { case (_, cost) => cost <= remaining } match { + case Some((task, cost)) => (funded :+ task, unfunded, remaining - cost) + case None => (funded, unfunded ++ purchases.lastOption.map(_._1), remaining) + } } (funded, unfunded) } @@ -443,19 +469,27 @@ object TopupMemberTrafficTrigger { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], + // Whether the purchase covers the member's shortfall as well as the configured top-up. False + // only where the wallet does not cover both, so the top-up alone pays the shortfall down. + withShortfall: Boolean, ) extends PrettyPrinting { + def shortfall: Long = TopupMemberTrafficTrigger.shortfall(trafficState.extraTrafficRemainder) + def trafficToBuy: Long = - TopupMemberTrafficTrigger.trafficToBuy( - target.topupParameters.topupAmount, - trafficState.extraTrafficRemainder, - ) + if (withShortfall) + TopupMemberTrafficTrigger.trafficToBuy( + target.topupParameters.topupAmount, + trafficState.extraTrafficRemainder, + ) + else target.topupParameters.topupAmount override def pretty: Pretty[Task] = prettyOfClass[Task]( param("target", _.target), param("topupState", _.topupState), param("trafficState", _.trafficState), + param("withShortfall", _.withShortfall), ) } } diff --git a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala index a913c35f548..8f687b66362 100644 --- a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala +++ b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala @@ -110,7 +110,8 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { "TopupMemberTrafficTrigger.fundedTasks" should { // Costs only; what they are attached to does not affect the allocation. - val candidates = Seq(globalAlias -> BigDecimal(100), dedicatedAlias -> BigDecimal(100)) + val candidates = + Seq(Seq(globalAlias -> BigDecimal(100)), Seq(dedicatedAlias -> BigDecimal(100))) "fund only what the balance covers, in order" in { // Enough for either one alone, not for both, so the second one misses out. @@ -125,6 +126,29 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { funded shouldBe Seq(globalAlias, dedicatedAlias) unfunded shouldBe empty } + + // A member below zero: its shortfall with the top-up, or the top-up alone. + val withShortfall = Seq("top-up and shortfall" -> BigDecimal(300), "top-up" -> BigDecimal(100)) + + "buy the shortfall with the top-up where the balance covers both" in { + TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(300))) shouldBe + (Seq("top-up and shortfall"), Seq.empty) + } + + "fall back to the top-up alone where the balance does not cover the shortfall too" in { + TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(150))) shouldBe + (Seq("top-up"), Seq.empty) + } + + "report the top-up alone where the balance does not cover even that" in { + TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(50))) shouldBe + (Seq.empty, Seq("top-up")) + } + + "buy the shortfall with the top-up where the balance does not bound the purchases" in { + TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), None) shouldBe + (Seq("top-up and shortfall"), Seq.empty) + } } "TopupMemberTrafficTrigger.shortfall" should { From 89b0dfc724f8f52eecd2942cb219e549f456ab64 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Tue, 6 Oct 2026 15:42:13 -0400 Subject: [PATCH 04/11] test: have alice transact during the outage, and reuse the splitwell helper [ci] Alice now pings during the outage before the check that she ran on her own traffic, so it covers a member transacting rather than only her background traffic. The helper that drops the splitwell apps takes the validators to disconnect from splitwell, and this test uses it instead of repeating it with bob left connected. Signed-off-by: Timothy Wu --- .../integration/EnvironmentDefinition.scala | 22 +++++++++++-------- .../SyncOperatorTrafficIntegrationTest.scala | 15 +++++-------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala index 34a34251a58..7483ff6b657 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala @@ -523,22 +523,26 @@ case class EnvironmentDefinition( /** Drops the splitwell apps and leaves alice's validator as the only one connected to the * splitwell synchronizer. */ - def withOnlyAliceValidatorConnectingToSplitwell: EnvironmentDefinition = { + def withOnlyAliceValidatorConnectingToSplitwell: EnvironmentDefinition = + withValidatorsDisconnectedFromSplitwell("bobValidator", "splitwellValidator") + + /** Drops the splitwell apps and disconnects the given validators from the splitwell + * synchronizer. + */ + def withValidatorsDisconnectedFromSplitwell(validators: String*): EnvironmentDefinition = this .addConfigTransform((_, conf) => conf.copy( splitwellApps = Map.empty, - validatorApps = - Seq("bobValidator", "splitwellValidator").foldLeft(conf.validatorApps) { (apps, name) => - apps.updatedWith(InstanceName.tryCreate(name)) { - _.map { validatorConfig => - validatorConfig.copy(domains = validatorConfig.domains.copy(extra = Seq.empty)) - } + validatorApps = validators.foldLeft(conf.validatorApps) { (apps, name) => + apps.updatedWith(InstanceName.tryCreate(name)) { + _.map { validatorConfig => + validatorConfig.copy(domains = validatorConfig.domains.copy(extra = Seq.empty)) } - }, + } + }, ) ) - } def clearConfigTransforms(): EnvironmentDefinition = copy(configTransformsWithContext = _ => Seq()) diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index 5e66685dce7..ad823469987 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -79,15 +79,7 @@ class SyncOperatorTrafficIntegrationTest ) // Only alice and bob connect to splitwell: alice for the purchases, bob for the outage test, // which needs a member whose balance starts small. - .addConfigTransform((_, conf) => - conf.copy( - splitwellApps = Map.empty, - validatorApps = - conf.validatorApps.updatedWith(InstanceName.tryCreate("splitwellValidator")) { - _.map(c => c.copy(domains = c.domains.copy(extra = Seq.empty))) - }, - ) - ) + .withValidatorsDisconnectedFromSplitwell("splitwellValidator") .withStandardSetup // Down from 200 KB, so bob's small purchases are allowed. .addConfigTransform((_, conf) => @@ -438,6 +430,11 @@ class SyncOperatorTrafficIntegrationTest } clue("alice runs on what was bought for her, without the allowance") { + val consumedBefore = consumed(alice) + ping(aliceParticipant, synchronizerId) + eventually() { + consumed(alice) should be > consumedBefore + } consumed(alice) should be <= totals(alice) } }, From 9feb7ff082ac3c3fc58abd1c847b6adef9db459c Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Tue, 6 Oct 2026 16:32:22 -0400 Subject: [PATCH 05/11] fix: address the re-review of the outage traffic allowance [ci] - When even the top-up alone does not fit, the insufficient-funds warning also names the top-up and the shortfall together for a member below zero, so the wallet is funded in one round. - Both purchases are priced from one read of the rules and the open round, and minWalletBalanceForTopup is back to its signature on main. - A task whose target a grant has already met returns TaskNoop. - The warnings for a limit that could not be set say that operators may also not all have seen the same purchases yet. Signed-off-by: Timothy Wu --- .../OutageTrafficAllowanceTrigger.scala | 16 ++++----- .../OutageTrafficAllowanceTriggerTest.scala | 6 ++++ .../TopupMemberTrafficTrigger.scala | 28 +++++++-------- .../splice/wallet/util/TopupUtil.scala | 36 +++++++++++++------ 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala index 7c35139c253..6e57e2880fa 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala @@ -119,10 +119,8 @@ class OutageTrafficAllowanceTrigger( current <- currentTask(task) limit = trafficState.extraTrafficLimit.value outcome <- - if (current.isDone(limit)) - Future.successful( - TaskSuccess(s"The traffic limit of ${task.member} is already at its target of $limit") - ) + // A grant since the task was retrieved has already brought the limit to its target. + if (current.isDone(limit)) Future.successful(TaskNoop) else connection .setSequencerTrafficControlState( @@ -242,15 +240,17 @@ object OutageTrafficAllowanceTrigger { case Some(a) => s"Could not set the traffic limit of $member to its purchased total of $total plus the " + s"outage traffic allowance of $a bytes: it is still $limit. On a synchronizer whose " + - "sequencers several operators run, it changes only once enough of them set the same " + - "allowance." + "sequencers several operators run, it changes only once enough of them send the same " + + "limit: either too few of them have set the same allowance yet, or they have not all " + + "seen the same purchases yet." case None => val gap = limit - total val side = if (gap > 0) "above" else "below" s"Could not bring the traffic limit of $member back to its purchased total of $total: " + s"it is still $limit, ${math.abs(gap)} bytes $side it. On a synchronizer whose " + - "sequencers several operators run, it changes only once enough of them have removed " + - "the outage traffic allowance." + "sequencers several operators run, it changes only once enough of them send the same " + + "limit: either too few of them have removed the outage traffic allowance yet, or they " + + "have not all seen the same purchases yet." } override def pretty: Pretty[Task] = prettyOfClass( diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala index 47b277138c1..53a4707e069 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala @@ -114,6 +114,12 @@ class OutageTrafficAllowanceTriggerTest extends AnyWordSpec with BaseTest { Task(alice, 100L, None).notApplied(150L) should include("50 bytes above") Task(alice, 100L, None).notApplied(80L) should include("20 bytes below") } + + "tell the operator that the other operators may not have seen the same purchases" in { + forAll(Seq(Task(alice, 100L, None), Task(alice, 100L, allowance))) { task => + task.notApplied(150L) should include("not all seen the same purchases") + } + } } "OutageTrafficAllowanceTrigger.withoutTrafficState" should { diff --git a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala index 691ddc064c4..f840e3667a8 100644 --- a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala +++ b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala @@ -134,7 +134,11 @@ class TopupMemberTrafficTrigger( _ = unfunded.foreach(task => logger.warn( s"Insufficient funds to buy configured traffic amount. Please ensure that the validator's wallet has enough amulets to purchase " + - s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." + s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." + + (if (task.shortfall > 0) + s" This member is also ${BigDecimal(task.shortfall) / 1e6} MB below zero there: for one purchase to get it submitting again, the wallet needs enough amulets for " + + s"${BigDecimal(task.target.topupParameters.topupAmount + task.shortfall) / 1e6} MB, the top-up and the shortfall together." + else "") ) ) _ = funded @@ -184,27 +188,21 @@ class TopupMemberTrafficTrigger( TopupMemberTrafficTrigger.shortfall(currentTrafficState.extraTrafficRemainder) // The top-up alone still pays a shortfall down, one interval at a time. val withShortfallOptions = if (shortfall > 0) Seq(true, false) else Seq(true) - Future.traverse(withShortfallOptions)(withShortfall => - TopupUtil - // The cost is derived from this target's throughput and interval, plus the - // shortfall where the purchase covers it. - .minWalletBalanceForTopup( - scanConnection, - target.topupConfig, - clock, - registration, - extraTraffic = if (withShortfall) shortfall else 0L, - ) - .map( + TopupUtil + // The cost is derived from this target's throughput and interval, plus the + // shortfall where the purchase covers it. + .topupCosts(scanConnection, target.topupConfig, clock, registration) + .map(costOf => + withShortfallOptions.map(withShortfall => TopupMemberTrafficTrigger.Task( target, topupState, currentTrafficState, registration, withShortfall, - ) -> _ + ) -> costOf(if (withShortfall) shortfall else 0L) ) - ) + ) } } yield tasks } yield purchases diff --git a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala index 25e2cc6008f..c90de02a130 100644 --- a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala +++ b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala @@ -30,7 +30,6 @@ object TopupUtil { .map(_.payload.governanceParameters) .fold(BigDecimal(1))(p => BigDecimal(p.discountFactor)) - /** The cost of one top-up, plus `extraTraffic` bought with it. */ def minWalletBalanceForTopup( scanConnection: ScanConnection, validatorTopupConfig: ValidatorTopupConfig, @@ -38,8 +37,24 @@ object TopupUtil { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], - extraTraffic: Long = 0L, - )(implicit tc: TraceContext, ec: ExecutionContext, mat: Materializer): Future[BigDecimal] = for { + )(implicit tc: TraceContext, ec: ExecutionContext, mat: Materializer): Future[BigDecimal] = + topupCosts(scanConnection, validatorTopupConfig, clock, registration).map(costOf => costOf(0L)) + + /** What one top-up, plus the extra traffic bought with it, costs in amulet, for any amount of + * extra traffic, from one read of the rules and the open round. + */ + def topupCosts( + scanConnection: ScanConnection, + validatorTopupConfig: ValidatorTopupConfig, + clock: Clock, + registration: Option[ + ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] + ], + )(implicit + tc: TraceContext, + ec: ExecutionContext, + mat: Materializer, + ): Future[Long => BigDecimal] = for { amuletRules <- scanConnection.getAmuletRulesWithState() synchronizerFeesConfig = AmuletConfigSchedule(amuletRules) .getConfigAsOf(clock.now) @@ -52,13 +67,14 @@ object TopupUtil { validatorTopupConfig.topupTriggerPollingInterval, ) latestRound <- scanConnection.getLatestOpenMiningRound() - } yield topupCost( - topupParameters, - extraTraffic, - BigDecimal(synchronizerFeesConfig.extraTrafficPrice), - latestRound.payload.amuletPrice, - registration, - ) + } yield (extraTraffic: Long) => + topupCost( + topupParameters, + extraTraffic, + BigDecimal(synchronizerFeesConfig.extraTrafficPrice), + latestRound.payload.amuletPrice, + registration, + ) /** What one top-up, plus `extraTraffic` bought with it, costs in amulet. */ def topupCost( From 6a6bb91d20e5b0fd7b4a12d5c77fa068d7b1be1e Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Mon, 12 Oct 2026 13:38:49 -0400 Subject: [PATCH 06/11] fix: reconcile dedicated-synchronizer traffic by polling, and drop the custom warnings [ci] Per review: a member that joined during an outage got nothing, since the startup pass only covered members with a purchase on record and stopped once it had settled. The event-driven reconcile trigger and the startup pass are replaced by one polling trigger. Each poll reads the traffic state of every member on the sequencer and every purchased total, and sets each participant's limit to its purchased total, or 0, plus the allowance while it is set. Mediators and sequencers are left alone. The periodic allowance warning, the purchase warning and the other operator-side logs this PR added are removed, along with the warning-interval setting. Signed-off-by: Timothy Wu --- .../SyncOperatorTrafficIntegrationTest.scala | 184 ++++++------ .../splice/syncoperator/SyncOperatorApp.scala | 1 - .../OutageTrafficAllowanceTrigger.scala | 263 ------------------ ...OutageTrafficAllowanceWarningTrigger.scala | 47 ---- ...cileDedicatedSequencerTrafficTrigger.scala | 217 +++++++++++---- .../SyncOperatorAutomationService.scala | 28 +- .../config/SyncOperatorAppConfig.scala | 11 +- .../OutageTrafficAllowanceTriggerTest.scala | 160 ----------- ...DedicatedSequencerTrafficTriggerTest.scala | 164 +++++++++++ test-full-class-names-non-integration.log | 2 +- 10 files changed, 432 insertions(+), 645 deletions(-) delete mode 100644 apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala delete mode 100644 apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala delete mode 100644 apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala create mode 100644 apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index ad823469987..e6f2fbb703c 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -78,7 +78,8 @@ class SyncOperatorTrafficIntegrationTest this.getClass.getSimpleName, ) // Only alice and bob connect to splitwell: alice for the purchases, bob for the outage test, - // which needs a member whose balance starts small. + // which needs a member whose balance starts small. The operator's participant joins during + // the outage test, as a member with no purchase on record. .withValidatorsDisconnectedFromSplitwell("splitwellValidator") .withStandardSetup // Down from 200 KB, so bob's small purchases are allowed. @@ -360,6 +361,8 @@ class SyncOperatorTrafficIntegrationTest // The operator app sees purchases, which are made on the global synchronizer, through // this participant. val operatorParticipant = splitwellValidatorBackend.participantClientWithAdminToken + // Not connected to splitwell until it joins during the outage. + val operator = operatorParticipant.id val syncOperatorWithAllowance = syncop(syncOperatorWithAllowanceName) def purchased() = runningSyncOperatorBackend.appState.store.listTotalPurchasedMemberTraffic().futureValue @@ -388,95 +391,101 @@ class SyncOperatorTrafficIntegrationTest try { // The other apps on the operator's participant warn about the submissions that fail // while it is cut off from the global synchronizer, so this tolerates their warnings. - loggerFactory.assertLogsSeq(SuppressionRule.LevelAndAbove(Level.WARN))( - { - // Cut off from the global synchronizer, the operator's participant sees no - // purchases, which is what an outage looks like to the operator. - operatorParticipant.synchronizers.disconnect(globalAlias) - - actAndCheck( - "the operator sets the allowance and restarts its app", { - syncOperatorBackend.stop() - syncOperatorWithAllowance.startSync() - }, - )( - "each member's limit rises to its purchased total plus the allowance, and the app " + - "warns that the allowance is set", - _ => { - limitsAreTotalsPlus(outageTrafficAllowance, totals) - recordedWarnings(allowanceSetWarning) should not be empty - }, - ) - - clue("a restart with the allowance still set moves no limit") { - syncOperatorWithAllowance.stop() + loggerFactory.suppress(SuppressionRule.LevelAndAbove(Level.WARN)) { + // Cut off from the global synchronizer, the operator's participant sees no purchases, + // which is what an outage looks like to the operator. + operatorParticipant.synchronizers.disconnect(globalAlias) + + actAndCheck( + "the operator sets the allowance and restarts its app", { + syncOperatorBackend.stop() syncOperatorWithAllowance.startSync() - always(durationOfSuccess = 10.seconds, pollIntervalMs = 1000) { - limitsAreTotalsPlus(outageTrafficAllowance, totals) - } - } + }, + )( + "each member's limit rises to its purchased total plus the allowance", + _ => limitsAreTotalsPlus(outageTrafficAllowance, totals), + ) - clue("bob keeps transacting past what was bought for him") { - pingWhile(bobParticipant, synchronizerId)(consumed(bob) <= bobPurchase) - consumed(bob) should be > bobPurchase + clue("a restart with the allowance still set moves no limit") { + syncOperatorWithAllowance.stop() + syncOperatorWithAllowance.startSync() + always(durationOfSuccess = 10.seconds, pollIntervalMs = 1000) { + limitsAreTotalsPlus(outageTrafficAllowance, totals) } + } - clue("and is refused once he has used the allowance") { - Iterator - .continually(Try(ping(bobParticipant, synchronizerId))) - .take(maxPings) - .exists(_.isFailure) shouldBe true - consumed(bob) should be <= bobPurchase + outageTrafficAllowance - } + clue("bob keeps transacting past what was bought for him") { + pingWhile(bobParticipant, synchronizerId)(consumed(bob) <= bobPurchase) + consumed(bob) should be > bobPurchase + } + + clue("and is refused once he has used the allowance") { + Iterator + .continually(Try(ping(bobParticipant, synchronizerId))) + .take(maxPings) + .exists(_.isFailure) shouldBe true + consumed(bob) should be <= bobPurchase + outageTrafficAllowance + } - clue("alice runs on what was bought for her, without the allowance") { - val consumedBefore = consumed(alice) - ping(aliceParticipant, synchronizerId) - eventually() { - consumed(alice) should be > consumedBefore - } - consumed(alice) should be <= totals(alice) + clue("alice runs on what was bought for her, without the allowance") { + val consumedBefore = consumed(alice) + ping(aliceParticipant, synchronizerId) + eventually() { + consumed(alice) should be > consumedBefore } - }, - // Once per start of the app. - logs => forAtLeast(2, logs)(_.warningMessage should include(allowanceSetWarning)), - ) + consumed(alice) should be <= totals(alice) + } - loggerFactory.assertLogsSeq(SuppressionRule.LevelAndAbove(Level.WARN))( - { - operatorParticipant.synchronizers.reconnect(globalAlias) - - actAndCheck( - "alice buys more traffic for bob once the global synchronizer is back", - buyTraffic( - aliceParty, - bob, - synchronizerId, - Some(registration), - dsoParty, - bobPurchaseWithAllowanceSet, - ), - )( - "the operator warns that the allowance is still set", - _ => recordedWarnings(purchaseLandedWarning) should not be empty, - ) + actAndCheck(timeUntilSuccess = 1.minute)( + "the operator's participant joins splitwell, with no purchase on record", + operatorParticipant.synchronizers.connect_by_config( + aliceParticipant.synchronizers + .config(splitwellAlias) + .value + // So that nothing reconnects it once it has left again. + .copy(manualConnect = true), + // Its topology transactions only go through once it has traffic. + synchronize = None, + ), + )( + "its limit is the allowance alone", + _ => { + purchased().keySet should not contain operator + extraTrafficLimit(operator) shouldBe outageTrafficAllowance + }, + ) + // It only joined for that check. + operatorParticipant.synchronizers.disconnect(splitwellAlias) + + operatorParticipant.synchronizers.reconnect(globalAlias) - clue("the purchase pays down bob's credit instead of raising his limit") { + actAndCheck( + "alice buys more traffic for bob once the global synchronizer is back", + buyTraffic( + aliceParty, + bob, + synchronizerId, + Some(registration), + dsoParty, + bobPurchaseWithAllowanceSet, + ), + )( + "bob's limit rises to his new purchased total plus the allowance", + _ => { purchased()(bob) shouldBe bobPurchase + bobPurchaseWithAllowanceSet - extraTrafficLimit(bob) shouldBe bobPurchase + outageTrafficAllowance - } + extraTrafficLimit(bob) shouldBe + bobPurchase + bobPurchaseWithAllowanceSet + outageTrafficAllowance + }, + ) - // In here, as the app warns that the allowance is set until it has stopped. - syncOperatorWithAllowance.stop() - }, - logs => forAtLeast(1, logs)(_.warningMessage should include(purchaseLandedWarning)), - ) + syncOperatorWithAllowance.stop() + } } finally { - if ( - !operatorParticipant.synchronizers - .list_connected() - .exists(_.synchronizerAlias == globalAlias) - ) + def connectedTo(alias: SynchronizerAlias) = + operatorParticipant.synchronizers.list_connected().exists(_.synchronizerAlias == alias) + if (connectedTo(splitwellAlias)) + operatorParticipant.synchronizers.disconnect(splitwellAlias) + if (!connectedTo(globalAlias)) operatorParticipant.synchronizers.reconnect(globalAlias) } @@ -486,8 +495,12 @@ class SyncOperatorTrafficIntegrationTest purchased() }, )( - "each member's limit goes back down to its purchased total", - totals => limitsAreTotalsPlus(0L, totals), + "each member's limit goes back to its purchased total, which is zero for the participant " + + "that joined during the outage", + totals => { + limitsAreTotalsPlus(0L, totals) + extraTrafficLimit(operator) shouldBe 0L + }, )._1 clue("alice, who did not use the allowance, keeps transacting") { @@ -518,15 +531,6 @@ class SyncOperatorTrafficIntegrationTest } } - private val allowanceSetWarning = - s"The outage traffic allowance of $outageTrafficAllowance bytes is set, covering 2 members" - private val purchaseLandedWarning = - "was bought or merged on the global synchronizer while the outage traffic allowance is set" - - // The warnings suppressed so far in the enclosing log assertion, for waiting on one. - private def recordedWarnings(text: String) = - loggerFactory.fetchRecordedLogEntries.filter(_.message.contains(text)) - // Bounds the ping loops, so a ping that costs less than expected fails the test instead of // running forever. private val maxPings = 300 diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala index 4af8fa9d3e8..7ebd94956db 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/SyncOperatorApp.scala @@ -174,7 +174,6 @@ class SyncOperatorApp( config.lsuDumpPath, config.trafficBalanceReconciliationDelay, config.outageTrafficAllowance, - config.outageTrafficAllowanceWarningInterval, loggerFactory, packageVersionSupport, ) diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala deleted file mode 100644 index 6e57e2880fa..00000000000 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTrigger.scala +++ /dev/null @@ -1,263 +0,0 @@ -// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package org.lfdecentralizedtrust.splice.syncoperator.automation - -import cats.syntax.apply.* -import com.digitalasset.canton.config.{NonNegativeFiniteDuration, PositiveFiniteDuration} -import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLong} -import com.digitalasset.canton.data.CantonTimestamp -import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} -import com.digitalasset.canton.topology.{MediatorId, Member} -import com.digitalasset.canton.tracing.TraceContext -import io.grpc.{Status, StatusRuntimeException} -import io.opentelemetry.api.trace.Tracer -import org.apache.pekko.stream.Materializer -import org.lfdecentralizedtrust.splice.automation.{ - PollingParallelTaskExecutionTrigger, - TaskFailed, - TaskNoop, - TaskOutcome, - TaskSuccess, - TriggerContext, -} -import org.lfdecentralizedtrust.splice.environment.SynchronizerNodeService -import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.TopologySnapshot -import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode -import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTrigger.Task -import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore - -import java.util.concurrent.atomic.AtomicBoolean -import scala.collection.concurrent.TrieMap -import scala.concurrent.{ExecutionContext, Future} - -/** Sets each member's traffic limit on this operator's sequencer from the outage traffic allowance - * in this app's config, when the app starts. - * - * While the global synchronizer is unavailable, nothing can be bought for this synchronizer, so - * members run only on what was bought for them before. The operator then sets - * `outage-traffic-allowance` and restarts this app: each member with a purchase on record gets a - * limit of exactly its purchased total plus the allowance, so it can run that many bytes past what - * was bought for it. Once the global synchronizer is back, the operator removes the setting and - * restarts again: each limit is set back to the purchased total, so a member that used the - * allowance is below zero until its purchases cover what it consumed. - * - * The targets only move with the config, which only changes on a restart, so once a poll finds - * every member at its target this does no more work per member. Purchases that land in the - * meantime are granted by [[ReconcileDedicatedSequencerTrafficTrigger]], which only raises a limit - * to the purchased total: while the allowance is set, they pay down the credit instead of adding - * to it. - * - * On a synchronizer whose sequencers several operators run, a limit changes only once the - * sequencer group's threshold of operators set the same one, so they set and remove the allowance - * together. - */ -class OutageTrafficAllowanceTrigger( - override protected val context: TriggerContext, - store: SyncOperatorStore, - synchronizerNodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], - allowance: Option[PositiveLong], - trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, - warningInterval: PositiveFiniteDuration, -)(implicit - override val ec: ExecutionContext, - mat: Materializer, - override val tracer: Tracer, -) extends PollingParallelTaskExecutionTrigger[Task] { - - // Set once a poll finds every member at its target, after which polls do nothing. - private val settled = new AtomicBoolean(false) - - // When each member was last warned about a limit that could not be set. - private val lastWarnedAt = TrieMap.empty[Member, CantonTimestamp] - - override protected def retrieveTasks()(implicit tc: TraceContext): Future[Seq[Task]] = - if (settled.get()) Future.successful(Seq.empty) - else - for { - connection <- synchronizerNodeService.sequencerAdminConnection() - totals <- store.listTotalPurchasedMemberTraffic() - // An empty filter would list every member of the synchronizer. - states <- - if (totals.isEmpty) Future.successful(Seq.empty) - else connection.listSequencerTrafficControlState(totals.keys.toSeq) - } yield { - val limits = states.map(state => state.member -> state.extraTrafficLimit.value).toMap - val tasks = - OutageTrafficAllowanceTrigger.tasks(totals, limits, allowance.map(_.value)) - if (tasks.isEmpty && !settled.getAndSet(true)) { - val target = allowance.fold("its purchased total")(a => - s"its purchased total plus the outage traffic allowance of ${a.value} bytes" - ) - logger.info( - s"Every member with a purchase on record and a traffic state on the sequencer is at $target" - ) - val skipped = OutageTrafficAllowanceTrigger.withoutTrafficState(totals, limits) - allowance.foreach(a => - if (skipped.nonEmpty) - logger.warn( - s"These members have a purchase on record but no traffic state on this " + - s"synchronizer's sequencer, so they did not get the outage traffic allowance of " + - s"${a.value} bytes: ${skipped.mkString(", ")}. They only get it if this app is " + - "restarted once the sequencer has a traffic state for them." - ) - ) - } - tasks - } - - override protected def completeTask(task: Task)(implicit tc: TraceContext): Future[TaskOutcome] = - for { - connection <- synchronizerNodeService.sequencerAdminConnection() - // Read again, so the grant carries the serial the sequencer expects next. - (trafficState, sequencerState) <- ( - connection.getSequencerTrafficControlState(task.member), - connection.getSequencerSynchronizerState(TopologySnapshot.Effective), - ).tupled - // And the total after it: a purchase granted since the task was retrieved is then either in - // the total, or its grant moved the serial and this one is refused and retried. - current <- currentTask(task) - limit = trafficState.extraTrafficLimit.value - outcome <- - // A grant since the task was retrieved has already brought the limit to its target. - if (current.isDone(limit)) Future.successful(TaskNoop) - else - connection - .setSequencerTrafficControlState( - trafficState, - sequencerState, - NonNegativeLong.tryCreate(current.target), - context.pollingClock, - trafficBalanceReconciliationDelay, - ) - .map[TaskOutcome](_ => - TaskSuccess( - s"Set the traffic limit of ${task.member} from $limit to ${current.target}" - ) - ) - .recover { - // The grant did not land, for example because too few of the synchronizer's - // operators sent the same one. The next poll tries again, as the targets are not - // all in place. - case ex: StatusRuntimeException - if ex.getStatus.getCode == Status.Code.DEADLINE_EXCEEDED => - notApplied(task.member, current.notApplied(limit)) - } - } yield outcome - - override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = - for { - connection <- synchronizerNodeService.sequencerAdminConnection() - state <- connection.lookupSequencerTrafficControlState(task.member) - current <- currentTask(task) - } yield state.forall(state => current.isDone(state.extraTrafficLimit.value)) - - // A warning at most once per warning interval for each member, and the same text in the info log - // in between: on a synchronizer whose operators set the allowance at different times, a limit - // can stay out of reach for many polls. - private def notApplied(member: Member, message: String)(implicit - tc: TraceContext - ): TaskOutcome = { - val now = context.clock.now - if (OutageTrafficAllowanceTrigger.warningDue(lastWarnedAt.get(member), now, warningInterval)) { - lastWarnedAt.update(member, now) - TaskFailed(message) - } else { - logger.info(message) - TaskNoop - } - } - - // The task with the member's purchased total as it is now. A target from the total the task was - // retrieved with would take back a purchase the reconcile trigger granted since. - private def currentTask(task: Task)(implicit tc: TraceContext): Future[Task] = - store.getTotalPurchasedMemberTraffic(task.member).map(total => task.copy(total = total)) -} - -object OutageTrafficAllowanceTrigger { - - /** Whether the allowance applies to a member with a purchase on record. Not to a mediator: with - * no purchase path, it is granted unlimited traffic by [[MediatorUnlimitedTrafficTrigger]], - * which a purchase someone made for it must not lower. - */ - def isCovered(member: Member): Boolean = member match { - case _: MediatorId => false - case _ => true - } - - /** The grants that bring each member with a purchase on record to its target, in a fixed order. - * Members without a traffic state on the sequencer are left out, as there is nothing to set yet. - */ - def tasks( - totals: Map[Member, Long], - limits: Map[Member, Long], - allowance: Option[Long], - ): Seq[Task] = - totals.toSeq - .filter { case (member, _) => isCovered(member) } - .sortBy { case (member, _) => member.toProtoPrimitive } - .flatMap { case (member, total) => - limits.get(member).flatMap { limit => - val task = Task(member, total, allowance) - Option.when(!task.isDone(limit))(task) - } - } - - /** The members with a purchase on record that the allowance applies to but the sequencer has no - * traffic state for, so a sweep leaves them out, in a fixed order. - */ - def withoutTrafficState(totals: Map[Member, Long], limits: Map[Member, Long]): Seq[Member] = - totals.keys - .filter(member => isCovered(member) && !limits.contains(member)) - .toSeq - .sortBy(_.toProtoPrimitive) - - /** Whether a warning last given at `lastWarned` is due again at `now`. */ - def warningDue( - lastWarned: Option[CantonTimestamp], - now: CantonTimestamp, - interval: PositiveFiniteDuration, - ): Boolean = - lastWarned.forall(last => !now.isBefore(last.plus(interval.asJava))) - - final case class Task(member: Member, total: Long, allowance: Option[Long]) - extends PrettyPrinting { - - /** The purchased total, plus the allowance while it is set, capped at the largest limit there - * is. - */ - def target: Long = - allowance.fold(total)(a => if (a > Long.MaxValue - total) Long.MaxValue else total + a) - - /** Whether `limit` leaves nothing to do. Exact both ways: setting the allowance raises a limit - * and setting a smaller one lowers it again, while removing it lowers a limit the allowance - * raised and raises one a purchase ingested during the removal left behind. - */ - def isDone(limit: Long): Boolean = limit == target - - /** Why a limit still at `limit` matters to the operator. */ - def notApplied(limit: Long): String = allowance match { - case Some(a) => - s"Could not set the traffic limit of $member to its purchased total of $total plus the " + - s"outage traffic allowance of $a bytes: it is still $limit. On a synchronizer whose " + - "sequencers several operators run, it changes only once enough of them send the same " + - "limit: either too few of them have set the same allowance yet, or they have not all " + - "seen the same purchases yet." - case None => - val gap = limit - total - val side = if (gap > 0) "above" else "below" - s"Could not bring the traffic limit of $member back to its purchased total of $total: " + - s"it is still $limit, ${math.abs(gap)} bytes $side it. On a synchronizer whose " + - "sequencers several operators run, it changes only once enough of them send the same " + - "limit: either too few of them have removed the outage traffic allowance yet, or they " + - "have not all seen the same purchases yet." - } - - override def pretty: Pretty[Task] = prettyOfClass( - param("member", _.member), - param("total", _.total), - paramIfDefined("allowance", _.allowance), - param("target", _.target), - ) - } -} diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala deleted file mode 100644 index 50959a7bc72..00000000000 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceWarningTrigger.scala +++ /dev/null @@ -1,47 +0,0 @@ -// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package org.lfdecentralizedtrust.splice.syncoperator.automation - -import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.PositiveLong -import com.digitalasset.canton.tracing.TraceContext -import io.opentelemetry.api.trace.Tracer -import org.lfdecentralizedtrust.splice.automation.{ - PeriodicTaskTrigger, - TaskOutcome, - TaskSuccess, - TriggerContext, -} -import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore - -import scala.concurrent.{ExecutionContext, Future} - -/** Warns, from startup and then every `interval`, that the outage traffic allowance is set, so the - * operator removes it once the global synchronizer is back. Only registered while it is set. - */ -class OutageTrafficAllowanceWarningTrigger( - interval: NonNegativeFiniteDuration, - triggerContext: TriggerContext, - store: SyncOperatorStore, - allowance: PositiveLong, -)(implicit - override val ec: ExecutionContext, - override val tracer: Tracer, -) extends PeriodicTaskTrigger(interval, triggerContext) { - - override def completeTask( - task: PeriodicTaskTrigger.PeriodicTask - )(implicit tc: TraceContext): Future[TaskOutcome] = - store.listTotalPurchasedMemberTraffic().map { totals => - val members = totals.keys.count(OutageTrafficAllowanceTrigger.isCovered) - val credit = BigInt(allowance.value) * members - logger.warn( - s"The outage traffic allowance of ${allowance.value} bytes is set, covering $members " + - s"members with a purchase on record, for a total credit outstanding of up to $credit " + - "bytes. Once the global synchronizer is reachable again, remove " + - "outage-traffic-allowance from this app's config and restart it." - ) - TaskSuccess(s"Warned that the outage traffic allowance covers $members members") - } -} diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index f0da7bfc323..18b6ca34aa3 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -3,27 +3,53 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation +import cats.syntax.apply.* import com.digitalasset.canton.config.NonNegativeFiniteDuration -import com.digitalasset.canton.config.RequireTypes.PositiveLong -import com.digitalasset.canton.topology.Member +import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLong} +import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} +import com.digitalasset.canton.topology.{Member, ParticipantId} import com.digitalasset.canton.tracing.TraceContext +import io.grpc.Status import io.opentelemetry.api.trace.Tracer import org.apache.pekko.stream.Materializer import org.lfdecentralizedtrust.splice.automation.{ - ReconcileSequencerLimitWithMemberTrafficTriggerBase, + PollingParallelTaskExecutionTrigger, + TaskNoop, TaskOutcome, + TaskSuccess, TriggerContext, } -import org.lfdecentralizedtrust.splice.codegen.java.splice.decentralizedsynchronizer.MemberTraffic -import org.lfdecentralizedtrust.splice.environment.SequencerAdminConnection -import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore -import org.lfdecentralizedtrust.splice.environment.SynchronizerNodeService +import org.lfdecentralizedtrust.splice.environment.{ + SequencerAdminConnection, + SynchronizerNodeService, +} +import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.TopologySnapshot import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode -import org.lfdecentralizedtrust.splice.util.AssignedContract +import org.lfdecentralizedtrust.splice.syncoperator.automation.ReconcileDedicatedSequencerTrafficTrigger.Task +import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore import scala.concurrent.{ExecutionContext, Future} -/** Reconciles the traffic purchased for this operator's synchronizer with its sequencer */ +/** Keeps the traffic limit of every participant on this operator's sequencer at the traffic + * purchased for it on this synchronizer, plus the outage traffic allowance while that is set. + * + * Each poll reads the traffic state of every member the sequencer knows and the purchased total of + * every member, so a participant that joins later, also during an outage, gets its limit on the + * next poll. A participant with no purchase on record has bought nothing, so it gets the allowance + * alone. Mediators are left to [[MediatorUnlimitedTrafficTrigger]], and sequencers are not charged + * traffic. + * + * Limits are set exactly, both ways: setting the allowance raises them, and removing it lowers them + * back to the purchased totals, so a participant that used the allowance is below zero until its + * purchases cover what it consumed. + * + * Reading every member on each poll is fine for the few members of a dedicated synchronizer, but + * would not scale to the global synchronizer. + * + * On a synchronizer whose sequencers several operators run, a limit changes only once the + * sequencer group's threshold of operators set the same one, so they set and remove the allowance + * together. + */ class ReconcileDedicatedSequencerTrafficTrigger( override protected val context: TriggerContext, store: SyncOperatorStore, @@ -31,51 +57,144 @@ class ReconcileDedicatedSequencerTrafficTrigger( trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, outageTrafficAllowance: Option[PositiveLong], )(implicit - ec: ExecutionContext, + override val ec: ExecutionContext, mat: Materializer, - tracer: Tracer, -) extends ReconcileSequencerLimitWithMemberTrafficTriggerBase( - store, - trafficBalanceReconciliationDelay, - ) { + override val tracer: Tracer, +) extends PollingParallelTaskExecutionTrigger[Task] { - // The purchases on record when this app started are the ones the allowance was set over. - private val startedAt = context.clock.now + private val allowance = outageTrafficAllowance.map(_.value) - override protected def sequencerAdminConnection()(implicit - tc: TraceContext - ): Future[SequencerAdminConnection] = - synchronizerNodeService.sequencerAdminConnection() + override protected def retrieveTasks()(implicit tc: TraceContext): Future[Seq[Task]] = + for { + connection <- sequencerAdminConnection() + (states, totals) <- ( + // An empty filter lists every member the sequencer knows. + connection.listSequencerTrafficControlState(), + store.listTotalPurchasedMemberTraffic(), + ).tupled + } yield ReconcileDedicatedSequencerTrafficTrigger.tasks( + states.map(state => state.member -> state.extraTrafficLimit.value).toMap, + totals, + allowance, + ) - override protected def getTotalPurchasedMemberTraffic(memberId: Member)(implicit - tc: TraceContext - ): Future[Long] = - store.getTotalPurchasedMemberTraffic(memberId) + override protected def completeTask(task: Task)(implicit tc: TraceContext): Future[TaskOutcome] = + for { + connection <- sequencerAdminConnection() + // Read again, so the grant carries the serial the sequencer expects next. + (trafficState, sequencerState) <- ( + connection.getSequencerTrafficControlState(task.member), + connection.getSequencerSynchronizerState(TopologySnapshot.Effective), + ).tupled + // And the total after it: a purchase granted since the task was retrieved is then either in + // the total, or its grant moved the serial and this one is refused and retried. + current <- currentTask(task) + limit = trafficState.extraTrafficLimit.value + outcome <- + if (current.isDone(limit)) Future.successful(TaskNoop) + else + connection + .setSequencerTrafficControlState( + trafficState, + sequencerState, + NonNegativeLong.tryCreate(current.target), + context.pollingClock, + trafficBalanceReconciliationDelay, + ) + .map[TaskOutcome](_ => + TaskSuccess( + s"Set the traffic limit of ${task.member} from $limit to ${current.target}" + ) + ) + } yield outcome + + override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = + for { + connection <- sequencerAdminConnection() + state <- connection.lookupSequencerTrafficControlState(task.member) + current <- currentTask(task) + } yield state.forall(state => current.isDone(state.extraTrafficLimit.value)) - override protected def trafficLimitOffset(memberId: Member)(implicit + // The task with the member's purchased total as it is now, so a purchase ingested since the task + // was retrieved is not taken back. + private def currentTask(task: Task)(implicit tc: TraceContext): Future[Task] = + store.getTotalPurchasedMemberTraffic(task.member).map(total => task.copy(total = total)) + + // The purchased totals are this operator's synchronizer's, so limits are only set on a sequencer + // that serves it: the one this app started with, or its successor once an upgrade switched over. + private def sequencerAdminConnection()(implicit tc: TraceContext - ): Future[Either[String, Long]] = - // No prior consumption to carry. - Future.successful(Right(0L)) - - override def completeTask( - memberTraffic: AssignedContract[MemberTraffic.ContractId, MemberTraffic] - )(implicit tc: TraceContext): Future[TaskOutcome] = - // The grant only raises a limit to the purchased total, so while the allowance is set it pays - // down the member's credit. The warning follows it, so a grant that is retried warns once. - super.completeTask(memberTraffic).map { outcome => - // Purchases, and the DSO's merges of them, only happen on the global synchronizer, so a - // contract created since this app started means it is back. - if ( - outageTrafficAllowance.isDefined && - memberTraffic.contract.createdAt.isAfter(startedAt.toInstant) - ) - logger.warn( - s"Traffic for ${memberTraffic.payload.memberId} was bought or merged on the global " + - "synchronizer while the outage traffic allowance is set, so the global synchronizer " + - "is reachable again. Remove outage-traffic-allowance from this app's config and " + - "restart it." - ) - outcome + ): Future[SequencerAdminConnection] = + for { + connection <- synchronizerNodeService.sequencerAdminConnection() + served <- connection.getStatus.map(_.successOption.map(_.synchronizerId.logical)) + } yield served match { + case Some(synchronizerId) if synchronizerId == store.key.synchronizerId => connection + case Some(synchronizerId) => + throw Status.FAILED_PRECONDITION + .withDescription( + s"The sequencer serves $synchronizerId, not this operator's synchronizer " + + s"${store.key.synchronizerId}" + ) + .asRuntimeException() + case None => + throw Status.FAILED_PRECONDITION + .withDescription("Sequencer is not yet initialized") + .asRuntimeException() } } + +object ReconcileDedicatedSequencerTrafficTrigger { + + /** Whether this trigger sets the member's limit. Only participants: a mediator has no purchase + * path and is granted unlimited traffic by [[MediatorUnlimitedTrafficTrigger]], which a purchase + * someone made for it must not lower, and a sequencer is not charged traffic. + */ + def isCovered(member: Member): Boolean = member match { + case _: ParticipantId => true + case _ => false + } + + /** The grants that bring each participant on the sequencer to its target, in a fixed order. + * + * @param limits + * the current limit of every member the sequencer has a traffic state for + * @param totals + * the purchased total of every member with a purchase on record for this synchronizer. A + * purchase for a member without a traffic state waits until the sequencer has one. + */ + def tasks( + limits: Map[Member, Long], + totals: Map[Member, Long], + allowance: Option[Long], + ): Seq[Task] = + limits.toSeq + .filter { case (member, _) => isCovered(member) } + .sortBy { case (member, _) => member.toProtoPrimitive } + .flatMap { case (member, limit) => + val task = Task(member, totals.getOrElse(member, 0L), allowance) + Option.when(!task.isDone(limit))(task) + } + + final case class Task(member: Member, total: Long, allowance: Option[Long]) + extends PrettyPrinting { + + /** The purchased total, plus the allowance while it is set, capped at the largest limit there + * is. + */ + def target: Long = + allowance.fold(total)(a => if (a > Long.MaxValue - total) Long.MaxValue else total + a) + + /** Whether `limit` leaves nothing to do. Exact both ways, so that removing the allowance lowers + * a limit it raised. + */ + def isDone(limit: Long): Boolean = limit == target + + override def pretty: Pretty[Task] = prettyOfClass( + param("member", _.member), + param("total", _.total), + paramIfDefined("allowance", _.allowance), + param("target", _.target), + ) + } +} diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala index fe548397ea5..317fe8032ea 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/SyncOperatorAutomationService.scala @@ -3,7 +3,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation -import com.digitalasset.canton.config.{NonNegativeFiniteDuration, PositiveFiniteDuration} +import com.digitalasset.canton.config.NonNegativeFiniteDuration import com.digitalasset.canton.config.RequireTypes.PositiveLong import com.digitalasset.canton.logging.NamedLoggerFactory import com.digitalasset.canton.resource.DbStorage @@ -50,7 +50,6 @@ class SyncOperatorAutomationService( lsuDumpPath: Option[Path], trafficBalanceReconciliationDelay: NonNegativeFiniteDuration, outageTrafficAllowance: Option[PositiveLong], - outageTrafficAllowanceWarningInterval: PositiveFiniteDuration, protected val loggerFactory: NamedLoggerFactory, packageVersionSupport: PackageVersionSupport, )(implicit @@ -106,29 +105,6 @@ class SyncOperatorAutomationService( ) ) - // Also without an allowance, so that a restart after removing one sets the limits back. - registerTrigger( - new OutageTrafficAllowanceTrigger( - triggerContext, - store, - synchronizerNodeService, - outageTrafficAllowance, - trafficBalanceReconciliationDelay, - outageTrafficAllowanceWarningInterval, - ) - ) - - outageTrafficAllowance.foreach(allowance => - registerTrigger( - new OutageTrafficAllowanceWarningTrigger( - NonNegativeFiniteDuration(outageTrafficAllowanceWarningInterval.underlying), - triggerContext, - store, - allowance, - ) - ) - ) - registerLsuTriggers() /** The upgrade path runs only once a successor node is configured. */ @@ -182,8 +158,6 @@ object SyncOperatorAutomationService extends AutomationServiceCompanion { aTrigger[ReconcileDedicatedSynchronizerParametersTrigger], aTrigger[MediatorUnlimitedTrafficTrigger], aTrigger[ReconcileDedicatedSequencerTrafficTrigger], - aTrigger[OutageTrafficAllowanceTrigger], - aTrigger[OutageTrafficAllowanceWarningTrigger], aTrigger[DedicatedLsuAnnouncementTrigger], aTrigger[DedicatedLsuTrigger], aTrigger[LsuTransferTrafficTrigger], diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala index deaf21f3c8f..8d4e24a6408 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala @@ -96,15 +96,12 @@ case class SyncOperatorAppBackendConfig( parameters: SpliceParametersConfig = SpliceParametersConfig(batching = BatchingConfig()), trafficBalanceReconciliationDelay: NonNegativeFiniteDuration = NonNegativeFiniteDuration.ofSeconds(10), - // Bytes each member with a purchase on record may use past what has been bought for it. The + // Bytes each participant on this synchronizer may use past what has been bought for it. The // operator sets it while the global synchronizer is unavailable, so nothing can be bought, and - // removes it once it is back, each time followed by a restart: on start, every such member's - // limit is set to its purchased total plus this, or back to its purchased total when unset. + // removes it once it is back, each time followed by a restart. While it is set, this app keeps + // every participant's limit at its purchased total plus this, including participants with no + // purchase on record and ones that join meanwhile; while it is unset, at its purchased total. outageTrafficAllowance: Option[PositiveLong] = None, - // How often this app warns that the outage traffic allowance is set, and at most how often it - // warns about each member whose limit it could not set. - outageTrafficAllowanceWarningInterval: PositiveFiniteDuration = - PositiveFiniteDuration.ofMinutes(5), // Set to false to disable the DB-level exclusive lock that prevents two sync operator instances // from running concurrently against the same database. Only disable for migration scenarios // where intentional overlap is required. diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala deleted file mode 100644 index 53a4707e069..00000000000 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/OutageTrafficAllowanceTriggerTest.scala +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package org.lfdecentralizedtrust.splice.syncoperator.automation - -import com.digitalasset.canton.BaseTest -import com.digitalasset.canton.config.PositiveFiniteDuration -import com.digitalasset.canton.data.CantonTimestamp -import com.digitalasset.canton.topology.{MediatorId, Member, ParticipantId, UniqueIdentifier} -import org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTrigger.Task -import org.scalatest.wordspec.AnyWordSpec - -class OutageTrafficAllowanceTriggerTest extends AnyWordSpec with BaseTest { - - private val alice: Member = ParticipantId.tryFromProtoPrimitive("PAR::alice::default") - private val bob: Member = ParticipantId.tryFromProtoPrimitive("PAR::bob::default") - private val carol: Member = ParticipantId.tryFromProtoPrimitive("PAR::carol::default") - private val mediator: Member = MediatorId(UniqueIdentifier.tryCreate("mediator", "default")) - - private val allowance = Some(50L) - - "OutageTrafficAllowanceTrigger.tasks" should { - - "raise each member to its purchased total plus the allowance" in { - OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L, bob -> 200L), - limits = Map(alice -> 100L, bob -> 200L), - allowance, - ) shouldBe Seq(Task(alice, 100L, allowance), Task(bob, 200L, allowance)) - } - - "lower a limit a larger allowance raised to the total plus a smaller one" in { - val tasks = OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L), - limits = Map(alice -> 200L), - allowance, - ) - tasks shouldBe Seq(Task(alice, 100L, allowance)) - tasks.map(_.target) shouldBe Seq(150L) - } - - "leave a member already at its total plus the allowance alone, so a restart adds nothing" in { - OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L), - limits = Map(alice -> 150L), - allowance, - ) shouldBe empty - } - - "set each limit back to the purchased total once the allowance is removed" in { - val tasks = OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L, bob -> 200L), - limits = Map(alice -> 150L, bob -> 200L), - allowance = None, - ) - tasks shouldBe Seq(Task(alice, 100L, None)) - tasks.map(_.target) shouldBe Seq(100L) - } - - "raise a limit left below its total by a purchase ingested while the allowance was removed" in { - OutageTrafficAllowanceTrigger - .tasks( - totals = Map(alice -> 300L), - limits = Map(alice -> 100L), - allowance = None, - ) - .map(_.target) shouldBe Seq(300L) - } - - "skip a member the sequencer has no traffic state for" in { - OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L, carol -> 10L), - limits = Map(alice -> 100L), - allowance, - ) shouldBe Seq(Task(alice, 100L, allowance)) - } - - "leave a member with no purchase on record alone" in { - OutageTrafficAllowanceTrigger.tasks( - totals = Map(alice -> 100L), - limits = Map(alice -> 100L, carol -> 0L), - allowance, - ) shouldBe Seq(Task(alice, 100L, allowance)) - } - - "leave a mediator's unlimited traffic alone, even with a purchase on record" in { - forEvery(Seq(allowance, None)) { allowance => - OutageTrafficAllowanceTrigger.tasks( - totals = Map(mediator -> 10L), - limits = Map(mediator -> Long.MaxValue), - allowance, - ) shouldBe empty - } - } - - "list the members in a fixed order" in { - OutageTrafficAllowanceTrigger - .tasks( - totals = Map(carol -> 1L, alice -> 1L, bob -> 1L), - limits = Map(carol -> 1L, alice -> 1L, bob -> 1L), - allowance, - ) - .map(_.member) shouldBe Seq(alice, bob, carol) - } - } - - "OutageTrafficAllowanceTrigger.Task" should { - - "cap a target the allowance would take past the largest limit" in { - Task(alice, 100L, Some(Long.MaxValue)).target shouldBe Long.MaxValue - } - - "tell the operator the gap of a limit it could not bring back to the purchased total" in { - Task(alice, 100L, None).notApplied(150L) should include("50 bytes above") - Task(alice, 100L, None).notApplied(80L) should include("20 bytes below") - } - - "tell the operator that the other operators may not have seen the same purchases" in { - forAll(Seq(Task(alice, 100L, None), Task(alice, 100L, allowance))) { task => - task.notApplied(150L) should include("not all seen the same purchases") - } - } - } - - "OutageTrafficAllowanceTrigger.withoutTrafficState" should { - - "list the members with a purchase on record the sequencer has no traffic state for" in { - OutageTrafficAllowanceTrigger.withoutTrafficState( - totals = Map(carol -> 100L, alice -> 100L, bob -> 200L, mediator -> 300L), - limits = Map(alice -> 100L), - ) shouldBe Seq(bob, carol) - } - } - - "OutageTrafficAllowanceTrigger.warningDue" should { - - val interval = PositiveFiniteDuration.ofMinutes(5) - val warnedAt = CantonTimestamp.Epoch - - "warn about a member not warned about before" in { - OutageTrafficAllowanceTrigger.warningDue(None, warnedAt, interval) shouldBe true - } - - "not warn again within the interval" in { - OutageTrafficAllowanceTrigger.warningDue( - Some(warnedAt), - warnedAt.plusSeconds(299), - interval, - ) shouldBe false - } - - "warn again once the interval has passed" in { - OutageTrafficAllowanceTrigger.warningDue( - Some(warnedAt), - warnedAt.plusSeconds(300), - interval, - ) shouldBe true - } - } -} diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala new file mode 100644 index 00000000000..fc5e4f47c34 --- /dev/null +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala @@ -0,0 +1,164 @@ +// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package org.lfdecentralizedtrust.splice.syncoperator.automation + +import com.digitalasset.canton.BaseTest +import com.digitalasset.canton.topology.{ + MediatorId, + Member, + ParticipantId, + SequencerId, + UniqueIdentifier, +} +import org.lfdecentralizedtrust.splice.syncoperator.automation.ReconcileDedicatedSequencerTrafficTrigger.{ + Task, + tasks, +} +import org.scalatest.wordspec.AnyWordSpec + +class ReconcileDedicatedSequencerTrafficTriggerTest extends AnyWordSpec with BaseTest { + + private val alice: Member = ParticipantId.tryFromProtoPrimitive("PAR::alice::default") + private val bob: Member = ParticipantId.tryFromProtoPrimitive("PAR::bob::default") + private val carol: Member = ParticipantId.tryFromProtoPrimitive("PAR::carol::default") + private val mediator: Member = MediatorId(UniqueIdentifier.tryCreate("mediator", "default")) + private val sequencer: Member = SequencerId(UniqueIdentifier.tryCreate("sequencer", "default")) + + private val allowance = Some(50L) + + "ReconcileDedicatedSequencerTrafficTrigger.tasks" should { + + "grant each participant its purchased total" in { + val result = tasks( + limits = Map(alice -> 0L, bob -> 100L), + totals = Map(alice -> 100L, bob -> 300L), + allowance = None, + ) + result shouldBe Seq(Task(alice, 100L, None), Task(bob, 300L, None)) + result.map(_.target) shouldBe Seq(100L, 300L) + } + + "leave a participant already at its purchased total alone" in { + tasks( + limits = Map(alice -> 100L), + totals = Map(alice -> 100L), + allowance = None, + ) shouldBe empty + } + + "raise each participant to its purchased total plus the allowance" in { + val result = tasks( + limits = Map(alice -> 100L, bob -> 200L), + totals = Map(alice -> 100L, bob -> 200L), + allowance, + ) + result shouldBe Seq(Task(alice, 100L, allowance), Task(bob, 200L, allowance)) + result.map(_.target) shouldBe Seq(150L, 250L) + } + + "leave a participant already at its purchased total plus the allowance alone" in { + tasks( + limits = Map(alice -> 150L), + totals = Map(alice -> 100L), + allowance, + ) shouldBe empty + } + + "lower a limit a larger allowance raised to the purchased total plus a smaller one" in { + tasks( + limits = Map(alice -> 200L), + totals = Map(alice -> 100L), + allowance, + ).map(_.target) shouldBe Seq(150L) + } + + "lower each limit back to the purchased total once the allowance is removed" in { + val result = tasks( + limits = Map(alice -> 150L, bob -> 200L), + totals = Map(alice -> 100L, bob -> 200L), + allowance = None, + ) + result shouldBe Seq(Task(alice, 100L, None)) + result.map(_.target) shouldBe Seq(100L) + } + + "give a participant with no purchase on record the allowance alone" in { + val result = tasks( + limits = Map(carol -> 0L), + totals = Map.empty, + allowance, + ) + result shouldBe Seq(Task(carol, 0L, allowance)) + result.map(_.target) shouldBe Seq(50L) + } + + "leave a participant with no purchase on record at zero without an allowance" in { + tasks( + limits = Map(carol -> 0L), + totals = Map.empty, + allowance = None, + ) shouldBe empty + } + + "take back the allowance from a participant with no purchase on record once it is removed" in { + tasks( + limits = Map(carol -> 50L), + totals = Map.empty, + allowance = None, + ).map(_.target) shouldBe Seq(0L) + } + + "leave a mediator's unlimited traffic alone, even with a purchase on record" in { + forEvery(Seq(allowance, None)) { allowance => + forEvery(Seq(0L, Long.MaxValue)) { limit => + tasks( + limits = Map(mediator -> limit), + totals = Map(mediator -> 10L), + allowance, + ) shouldBe empty + } + } + } + + "leave a sequencer alone" in { + tasks( + limits = Map(sequencer -> 0L), + totals = Map(sequencer -> 10L), + allowance, + ) shouldBe empty + } + + "wait with a purchase for a member the sequencer has no traffic state for" in { + tasks( + limits = Map(alice -> 100L), + totals = Map(alice -> 100L, carol -> 10L), + allowance = None, + ) shouldBe empty + } + + "list the participants in a fixed order" in { + tasks( + limits = Map(carol -> 0L, mediator -> 0L, alice -> 0L, bob -> 0L), + totals = Map(carol -> 1L, alice -> 1L, bob -> 1L), + allowance, + ).map(_.member) shouldBe Seq(alice, bob, carol) + } + } + + "ReconcileDedicatedSequencerTrafficTrigger.Task" should { + + "cap a target the allowance would take past the largest limit" in { + Task(alice, 100L, Some(Long.MaxValue)).target shouldBe Long.MaxValue + Task(alice, Long.MaxValue, allowance).target shouldBe Long.MaxValue + } + + "leave a participant at the largest limit alone once its target is capped" in { + tasks( + limits = Map(alice -> Long.MaxValue), + totals = Map(alice -> 100L), + allowance = Some(Long.MaxValue), + ) shouldBe empty + } + } +} diff --git a/test-full-class-names-non-integration.log b/test-full-class-names-non-integration.log index 9bf1b3fa39c..d199d50ee60 100644 --- a/test-full-class-names-non-integration.log +++ b/test-full-class-names-non-integration.log @@ -59,7 +59,7 @@ org.lfdecentralizedtrust.splice.sv.cometbft.CometBftRequestSignerTest org.lfdecentralizedtrust.splice.sv.onboarding.SequencerBftPeerReconcilerSpec org.lfdecentralizedtrust.splice.sv.util.SvOnboardingTokenTest org.lfdecentralizedtrust.splice.sv.util.WeightDistributionForSvTest -org.lfdecentralizedtrust.splice.syncoperator.automation.OutageTrafficAllowanceTriggerTest +org.lfdecentralizedtrust.splice.syncoperator.automation.ReconcileDedicatedSequencerTrafficTriggerTest org.lfdecentralizedtrust.splice.unit.http.HttpClientProxyTest org.lfdecentralizedtrust.splice.unit.store.db.AcsJdbcTypesTest org.lfdecentralizedtrust.splice.util.AmuletConfigScheduleTest From c78ddd0649bec5c1cd72abaff92954ca85bb45b8 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Mon, 12 Oct 2026 20:52:53 -0400 Subject: [PATCH 07/11] fix: address the review of the polling trigger and the shortfall top-up [ci] - Where a grant needs more than one sequencer, one that times out while the operators disagree is logged at info and sent again on the next poll, rather than retried until it logs an error. With a threshold of one, a timeout still fails as before. - The top-up budget is allocated in two passes: first purchases that leave their member able to submit, then fallbacks with what is left, so a fallback that leaves its member blocked cannot starve a later target. A task carries the extra traffic it buys, so what is priced and what is bought derive from one value. - The fallback warning only says the member cannot submit when the top-up leaves it at or below zero. - The integration test checks bob is refused only once he has less than one ping's worth of the allowance left, that one top-up buys exactly his shortfall with it, and that a restart sends no grant. - isCovered is private, and the base class's doc says only the SV trigger extends it now. Signed-off-by: Timothy Wu --- .../SyncOperatorTrafficIntegrationTest.scala | 57 +++++++-- ...cerLimitWithMemberTrafficTriggerBase.scala | 7 +- ...cileDedicatedSequencerTrafficTrigger.scala | 22 +++- .../TopupMemberTrafficTrigger.scala | 92 +++++++------ .../TopupMemberTrafficTriggerTest.scala | 121 +++++++++++++----- 5 files changed, 214 insertions(+), 85 deletions(-) diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index e6f2fbb703c..7963a9dc6bf 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -407,24 +407,42 @@ class SyncOperatorTrafficIntegrationTest ) clue("a restart with the allowance still set moves no limit") { + // A grant moves the serial even where it sets the limit the member already has. + def serials() = totals.keys.map(member => member -> serial(member)).toMap + val serialsBefore = serials() syncOperatorWithAllowance.stop() syncOperatorWithAllowance.startSync() - always(durationOfSuccess = 10.seconds, pollIntervalMs = 1000) { + // Long enough for several of the restarted app's polls. + val window = 10.seconds + val pollingInterval = syncOperatorWithAllowance.config.automation.pollingInterval + pollingInterval.asFiniteApproximation * 3 should be < window + always(durationOfSuccess = window, pollIntervalMs = 1000) { limitsAreTotalsPlus(outageTrafficAllowance, totals) + serials() shouldBe serialsBefore } } - clue("bob keeps transacting past what was bought for him") { - pingWhile(bobParticipant, synchronizerId)(consumed(bob) <= bobPurchase) + // The most one of bob's pings took his consumed traffic up by. + val pingCost = clue("bob keeps transacting past what was bought for him") { + val costs = pingWhile(bobParticipant, synchronizerId)(consumed(bob) <= bobPurchase) consumed(bob) should be > bobPurchase + costs.maxOption.getOrElse(fail("bob never pinged")) } clue("and is refused once he has used the allowance") { - Iterator + def headroom = bobPurchase + outageTrafficAllowance - consumed(bob) + // A ping can fail for other reasons too, so only a failure that leaves bob less than + // one ping's worth of traffic shows that he has used the allowance up. A few other + // failures are tolerated, rather than up to maxPings that can each take a timeout. + val refused = Iterator .continually(Try(ping(bobParticipant, synchronizerId))) .take(maxPings) - .exists(_.isFailure) shouldBe true - consumed(bob) should be <= bobPurchase + outageTrafficAllowance + .filter(_.isFailure) + .take(maxRefusals) + .exists(_ => headroom < pingCost) + withClue(s"bob has $headroom left, and one ping took up to $pingCost:") { + refused shouldBe true + } } clue("alice runs on what was bought for her, without the allowance") { @@ -515,16 +533,19 @@ class SyncOperatorTrafficIntegrationTest ) } + // Refused, bob consumes nothing more, so this is what his top-up finds. + val shortfall = consumed(bob) - totalsWithoutAllowance(bob) val topupTrigger = bobValidatorBackend.appState.automation.trigger[TopupMemberTrafficTrigger] setTriggersWithin(triggersToResumeAtStart = Seq(topupTrigger)) { clue("one top-up buys bob's shortfall with it, and he transacts again") { - // Inside bob's two-minute top-up interval, so a single purchase covered it. + // Inside bob's two-minute top-up interval, so no second top-up is bought meanwhile. eventually(100.seconds) { extraTrafficLimit(bob) should be > consumed(bob) } - // More than the top-up alone, which is less than the shortfall. - purchased()(bob) - totalsWithoutAllowance(bob) should be > bobTopupAmount + // One purchase of the top-up and the shortfall together, not the top-up alone or two + // top-ups. + purchased()(bob) - totalsWithoutAllowance(bob) shouldBe bobTopupAmount + shortfall ping(bobParticipant, synchronizerId) } } @@ -535,21 +556,35 @@ class SyncOperatorTrafficIntegrationTest // running forever. private val maxPings = 300 + // Bounds the failed pings the refusal check looks at. + private val maxRefusals = 5 + private def ping(participant: ParticipantClientReference, synchronizerId: SynchronizerId) = participant.health.ping(participant.id, synchronizerId = Some(synchronizerId)) + /** Pings while `condition` holds, and returns how much each ping took the participant's + * consumed traffic up by. + */ private def pingWhile(participant: ParticipantClientReference, synchronizerId: SynchronizerId)( condition: => Boolean - ): Unit = + )(implicit env: SpliceTestConsoleEnvironment): Seq[Long] = Iterator .continually(()) .take(maxPings) .takeWhile(_ => condition) - .foreach(_ => ping(participant, synchronizerId)) + .map { _ => + val before = consumed(participant.id) + ping(participant, synchronizerId) + consumed(participant.id) - before + } + .toSeq private def consumed(member: Member)(implicit env: SpliceTestConsoleEnvironment): Long = trafficState(member).fold(0L)(_.extraTrafficConsumed.value) + private def serial(member: Member)(implicit env: SpliceTestConsoleEnvironment) = + trafficState(member).flatMap(_.state.serial) + // The sync-operator CI job bootstraps splitwell with traffic control and a zero base rate; // see start-canton.sh -t. private def synchronizerParameters(implicit env: SpliceTestConsoleEnvironment) = diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/automation/ReconcileSequencerLimitWithMemberTrafficTriggerBase.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/automation/ReconcileSequencerLimitWithMemberTrafficTriggerBase.scala index f35e5142205..f1af562c39b 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/automation/ReconcileSequencerLimitWithMemberTrafficTriggerBase.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/automation/ReconcileSequencerLimitWithMemberTrafficTriggerBase.scala @@ -19,10 +19,11 @@ import org.lfdecentralizedtrust.splice.util.AssignedContract import scala.concurrent.{ExecutionContext, Future} /** Grants the traffic purchased via `MemberTraffic` contracts on the sequencer this trigger's - * connection serves, skipping contracts that name another synchronizer. One instance per - * sequencer, so a BFT synchronizer runs one operator app per sequencer. + * connection serves, skipping contracts that name another synchronizer. Only the SV app's + * `ReconcileSequencerLimitWithMemberTrafficTrigger` extends it: each SV runs one, which grants on + * that SV's own sequencer of the decentralized synchronizer. * - * This trigger currently relies on enough nodes working on the same set traffic balance request + * This trigger currently relies on enough SVs working on the same set traffic balance request * around the same time. It also depends on the sorting of tasks done in OnAssignedContractTrigger * to make this more likely to succeed. * diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index 18b6ca34aa3..5ea02e3fca6 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -4,6 +4,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import cats.syntax.apply.* +import com.daml.grpc.{GrpcException, GrpcStatus} import com.digitalasset.canton.config.NonNegativeFiniteDuration import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLong} import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} @@ -48,7 +49,8 @@ import scala.concurrent.{ExecutionContext, Future} * * On a synchronizer whose sequencers several operators run, a limit changes only once the * sequencer group's threshold of operators set the same one, so they set and remove the allowance - * together. + * together. A grant not applied within `trafficBalanceReconciliationDelay` is sent again on the + * next poll. */ class ReconcileDedicatedSequencerTrafficTrigger( override protected val context: TriggerContext, @@ -106,6 +108,22 @@ class ReconcileDedicatedSequencerTrafficTrigger( s"Set the traffic limit of ${task.member} from $limit to ${current.target}" ) ) + // Where a grant needs more than one sequencer, it is not applied until enough of the + // sequencers' operators send the same limit, so a timeout is expected while they + // disagree. Retrying the task would resend it until the retries run out and then log + // an error, so the next poll sends it again instead. Where one sequencer is enough, a + // timeout is a real failure and is handled as any other. + .recover { + case GrpcException(GrpcStatus(Status.Code.DEADLINE_EXCEEDED, _), _) + if sequencerState.mapping.threshold.value > 1 => + logger.info( + s"The grant of a traffic limit of ${current.target} for ${task.member} was not " + + s"applied within $trafficBalanceReconciliationDelay: it needs " + + s"${sequencerState.mapping.threshold.value} of the synchronizer's sequencers to " + + "send the same limit. The next poll sends it again." + ) + TaskNoop + } } yield outcome override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = @@ -150,7 +168,7 @@ object ReconcileDedicatedSequencerTrafficTrigger { * path and is granted unlimited traffic by [[MediatorUnlimitedTrafficTrigger]], which a purchase * someone made for it must not lower, and a sequencer is not charged traffic. */ - def isCovered(member: Member): Boolean = member match { + private[automation] def isCovered(member: Member): Boolean = member match { case _: ParticipantId => true case _ => false } diff --git a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala index f840e3667a8..c11f39e1954 100644 --- a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala +++ b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala @@ -128,7 +128,7 @@ class TopupMemberTrafficTrigger( (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks( candidates.filter(_.nonEmpty), budget, - ) + )(_.unblocks) // we do not even submit the topup tx if the validator does not have sufficient funds because we know // the tx would fail but it would still drain synchronizer traffic which we would like to avoid (see #11915). _ = unfunded.foreach(task => @@ -137,16 +137,19 @@ class TopupMemberTrafficTrigger( s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." + (if (task.shortfall > 0) s" This member is also ${BigDecimal(task.shortfall) / 1e6} MB below zero there: for one purchase to get it submitting again, the wallet needs enough amulets for " + - s"${BigDecimal(task.target.topupParameters.topupAmount + task.shortfall) / 1e6} MB, the top-up and the shortfall together." + s"${BigDecimal(task.trafficToBuy + task.shortfall) / 1e6} MB, the top-up and the shortfall together." else "") ) ) + // A member below zero that gets the top-up alone, because the wallet does not cover both. _ = funded - .filterNot(_.withShortfall) + .filter(task => task.shortfall > 0 && task.extraTraffic == 0) .foreach(task => logger.warn( s"Insufficient funds to buy the ${BigDecimal(task.shortfall) / 1e6} MB of traffic this member is below zero on ${task.target.alias} " + - "together with the configured top-up, so only the top-up is bought, and the member cannot submit there until its top-ups have covered it. " + + "together with the configured top-up, so only the top-up is bought" + + (if (task.unblocks) ". " + else ", and the member cannot submit there until its top-ups have covered it. ") + "Please ensure that the validator's wallet has enough amulets to buy both." ) ) @@ -187,20 +190,20 @@ class TopupMemberTrafficTrigger( val shortfall = TopupMemberTrafficTrigger.shortfall(currentTrafficState.extraTrafficRemainder) // The top-up alone still pays a shortfall down, one interval at a time. - val withShortfallOptions = if (shortfall > 0) Seq(true, false) else Seq(true) + val extraTrafficOptions = if (shortfall > 0) Seq(shortfall, 0L) else Seq(0L) TopupUtil // The cost is derived from this target's throughput and interval, plus the - // shortfall where the purchase covers it. + // extra traffic bought with the top-up. .topupCosts(scanConnection, target.topupConfig, clock, registration) .map(costOf => - withShortfallOptions.map(withShortfall => + extraTrafficOptions.map(extraTraffic => TopupMemberTrafficTrigger.Task( target, topupState, currentTrafficState, registration, - withShortfall, - ) -> costOf(if (withShortfall) shortfall else 0L) + extraTraffic, + ) -> costOf(extraTraffic) ) ) } @@ -377,27 +380,43 @@ object TopupMemberTrafficTrigger { ) } - /** Splits candidates into the purchases the balance covers and the candidates it does not, in - * target order. Each candidate lists its purchases preferred first and gets the first one the - * balance covers; one it covers none of is reported by its last, cheapest, purchase. Every buy - * draws on the same wallet, so a target is only funded once the ones before it are. A `None` - * budget does not bound the purchases. + /** Splits candidates into the purchases the balance covers and the candidates it covers none + * of, both in target order. Each candidate lists its purchases preferred first, and every buy + * draws on the same wallet, so the balance is allocated in two passes, each in target order. + * The first gives each candidate its first purchase that the balance covers and that `unblocks` + * its member, leaving it able to submit. The second gives each candidate still unfunded its + * first purchase the rest covers, such as a top-up that pays a shortfall down without covering + * it. A purchase that leaves its member blocked thus cannot take the balance from one that keeps + * a later target submitting. A candidate the balance covers none of is reported by its last, + * cheapest, purchase. A `None` budget funds each candidate's first purchase. */ private[automation] def fundedTasks[A]( candidates: Seq[Seq[(A, BigDecimal)]], budget: Option[BigDecimal], - ): (Seq[A], Seq[A]) = budget match { + )(unblocks: A => Boolean): (Seq[A], Seq[A]) = budget match { case None => (candidates.flatMap(_.headOption.map(_._1)), Seq.empty) case Some(balance) => - val (funded, unfunded, _) = - candidates.foldLeft((Seq.empty[A], Seq.empty[A], balance)) { - case ((funded, unfunded, remaining), purchases) => - purchases.find { case (_, cost) => cost <= remaining } match { - case Some((task, cost)) => (funded :+ task, unfunded, remaining - cost) - case None => (funded, unfunded ++ purchases.lastOption.map(_._1), remaining) + // One pass in target order: each candidate not funded yet gets its first purchase that the + // balance left covers and that is `eligible`. + def fund( + chosen: Seq[Option[A]], + balanceLeft: BigDecimal, + eligible: A => Boolean, + ): (Seq[Option[A]], BigDecimal) = + candidates.zip(chosen).foldLeft((Seq.empty[Option[A]], balanceLeft)) { + case ((done, remaining), (_, Some(task))) => (done :+ Some(task), remaining) + case ((done, remaining), (purchases, None)) => + purchases.find { case (task, cost) => cost <= remaining && eligible(task) } match { + case Some((task, cost)) => (done :+ Some(task), remaining - cost) + case None => (done :+ None, remaining) } } - (funded, unfunded) + val (unblocking, balanceLeft) = fund(candidates.map(_ => Option.empty[A]), balance, unblocks) + val (funded, _) = fund(unblocking, balanceLeft, _ => true) + val unfunded = candidates.zip(funded).collect { case (purchases, None) => + purchases.lastOption.map(_._1) + } + (funded.flatten, unfunded.flatten) } /** Resolves the configured top-up targets against the synchronizers the participant is @@ -454,11 +473,11 @@ object TopupMemberTrafficTrigger { */ def shortfall(extraTrafficRemainder: Long): Long = math.max(0L, -extraTrafficRemainder) - /** The configured top-up plus any shortfall, bought in one purchase: the configured amount alone - * would leave the member blocked until several top-ups had covered the shortfall. + /** Whether buying `trafficToBuy` leaves a member with `extraTrafficRemainder` able to submit, + * that is above zero: at zero, with no base rate, it still cannot. */ - def trafficToBuy(topupAmount: Long, extraTrafficRemainder: Long): Long = - topupAmount + shortfall(extraTrafficRemainder) + def unblocks(trafficToBuy: Long, extraTrafficRemainder: Long): Boolean = + trafficToBuy > shortfall(extraTrafficRemainder) final case class Task( target: Target, @@ -467,27 +486,26 @@ object TopupMemberTrafficTrigger { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], - // Whether the purchase covers the member's shortfall as well as the configured top-up. False - // only where the wallet does not cover both, so the top-up alone pays the shortfall down. - withShortfall: Boolean, + // Bought together with the configured top-up: the member's shortfall, so that one purchase + // covers it, or 0 for the top-up alone. That is every ordinary top-up, and the fallback + // where the wallet does not cover both. + extraTraffic: Long, ) extends PrettyPrinting { def shortfall: Long = TopupMemberTrafficTrigger.shortfall(trafficState.extraTrafficRemainder) - def trafficToBuy: Long = - if (withShortfall) - TopupMemberTrafficTrigger.trafficToBuy( - target.topupParameters.topupAmount, - trafficState.extraTrafficRemainder, - ) - else target.topupParameters.topupAmount + def trafficToBuy: Long = target.topupParameters.topupAmount + extraTraffic + + /** Whether the member can submit once this purchase is granted. */ + def unblocks: Boolean = + TopupMemberTrafficTrigger.unblocks(trafficToBuy, trafficState.extraTrafficRemainder) override def pretty: Pretty[Task] = prettyOfClass[Task]( param("target", _.target), param("topupState", _.topupState), param("trafficState", _.trafficState), - param("withShortfall", _.withShortfall), + param("extraTraffic", _.extraTraffic), ) } } diff --git a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala index 8f687b66362..2b96f8ba482 100644 --- a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala +++ b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala @@ -109,45 +109,87 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { "TopupMemberTrafficTrigger.fundedTasks" should { - // Costs only; what they are attached to does not affect the allocation. - val candidates = - Seq(Seq(globalAlias -> BigDecimal(100)), Seq(dedicatedAlias -> BigDecimal(100))) + // A purchase, and whether it leaves its member able to submit. What else it is attached to + // does not affect the allocation. + final case class Purchase(name: String, unblocks: Boolean = true) + def fundedTasks(candidates: Seq[Seq[(Purchase, BigDecimal)]], budget: Option[BigDecimal]) = + TopupMemberTrafficTrigger.fundedTasks(candidates, budget)(_.unblocks) + + val global = Purchase("global") + val dedicated = Purchase("dedicated") + val candidates = Seq(Seq(global -> BigDecimal(100)), Seq(dedicated -> BigDecimal(100))) "fund only what the balance covers, in order" in { // Enough for either one alone, not for both, so the second one misses out. - val (funded, unfunded) = - TopupMemberTrafficTrigger.fundedTasks(candidates, Some(BigDecimal(150))) - funded shouldBe Seq(globalAlias) - unfunded shouldBe Seq(dedicatedAlias) + fundedTasks(candidates, Some(BigDecimal(150))) shouldBe (Seq(global), Seq(dedicated)) } "fund every target when the balance does not bound the purchases" in { - val (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks(candidates, None) - funded shouldBe Seq(globalAlias, dedicatedAlias) - unfunded shouldBe empty + fundedTasks(candidates, None) shouldBe (Seq(global, dedicated), Seq.empty) } - // A member below zero: its shortfall with the top-up, or the top-up alone. - val withShortfall = Seq("top-up and shortfall" -> BigDecimal(300), "top-up" -> BigDecimal(100)) + // A member below zero: its shortfall with the top-up, or the top-up alone, which leaves it + // below zero where the shortfall is larger than the top-up. + val topupAndShortfall = Purchase("top-up and shortfall") + val topupBelowZero = Purchase("top-up", unblocks = false) + val withShortfall = + Seq(topupAndShortfall -> BigDecimal(300), topupBelowZero -> BigDecimal(100)) "buy the shortfall with the top-up where the balance covers both" in { - TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(300))) shouldBe - (Seq("top-up and shortfall"), Seq.empty) + fundedTasks(Seq(withShortfall), Some(BigDecimal(300))) shouldBe + (Seq(topupAndShortfall), Seq.empty) } "fall back to the top-up alone where the balance does not cover the shortfall too" in { - TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(150))) shouldBe - (Seq("top-up"), Seq.empty) + fundedTasks(Seq(withShortfall), Some(BigDecimal(150))) shouldBe + (Seq(topupBelowZero), Seq.empty) } "report the top-up alone where the balance does not cover even that" in { - TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), Some(BigDecimal(50))) shouldBe - (Seq.empty, Seq("top-up")) + fundedTasks(Seq(withShortfall), Some(BigDecimal(50))) shouldBe + (Seq.empty, Seq(topupBelowZero)) } "buy the shortfall with the top-up where the balance does not bound the purchases" in { - TopupMemberTrafficTrigger.fundedTasks(Seq(withShortfall), None) shouldBe - (Seq("top-up and shortfall"), Seq.empty) + fundedTasks(Seq(withShortfall), None) shouldBe (Seq(topupAndShortfall), Seq.empty) + } + + // A member 500 below zero with a top-up of 100, ahead of a healthy target whose top-up costs + // 100 too. + val blocked = Seq( + Purchase("blocked: top-up and shortfall") -> BigDecimal(600), + Purchase("blocked: top-up", unblocks = false) -> BigDecimal(100), + ) + val healthy = Purchase("healthy") + val healthyTarget = Seq(healthy -> BigDecimal(100)) + + "fund a later target before a top-up that leaves an earlier member below zero" in { + // Enough for one top-up: the healthy target's keeps it submitting, the other would not get + // its member submitting again. + fundedTasks(Seq(blocked, healthyTarget), Some(BigDecimal(150))) shouldBe + (Seq(healthy), Seq(blocked.last._1)) + } + + "fund a top-up that leaves its member below zero with what the other targets leave" in { + fundedTasks(Seq(blocked, healthyTarget), Some(BigDecimal(250))) shouldBe + (Seq(blocked.last._1, healthy), Seq.empty) + } + + "fund a top-up that covers the shortfall alone ahead of later targets" in { + // A shortfall smaller than the top-up, so the top-up alone gets the member submitting again. + val coveredByTopup = Purchase("top-up covering the shortfall") + val smallShortfall = Seq( + Purchase("top-up and small shortfall") -> BigDecimal(120), + coveredByTopup -> BigDecimal(100), + ) + // Enough for one top-up, which goes to the first target. + fundedTasks(Seq(smallShortfall, healthyTarget), Some(BigDecimal(110))) shouldBe + (Seq(coveredByTopup), Seq(healthy)) + } + + "fund each candidate's first purchase where the balance does not bound the purchases" in { + fundedTasks(Seq(blocked, healthyTarget), None) shouldBe + (Seq(blocked.head._1, healthy), Seq.empty) } } @@ -163,24 +205,39 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { } } - "TopupMemberTrafficTrigger.trafficToBuy" should { + "TopupMemberTrafficTrigger.unblocks" should { + + "hold for a purchase that takes the balance above zero" in { + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 2_300L, + extraTrafficRemainder = -300L, + ) shouldBe true + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 301L, + extraTrafficRemainder = -300L, + ) shouldBe true + } - "buy the shortfall together with the configured top-up" in { - TopupMemberTrafficTrigger.trafficToBuy( - topupAmount = 2_000L, + "not hold for a purchase that leaves the balance at or below zero" in { + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 300L, extraTrafficRemainder = -300L, - ) shouldBe 2_300L + ) shouldBe false + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 2_000L, + extraTrafficRemainder = -2_001L, + ) shouldBe false } - "buy only the configured top-up for a balance at or above zero" in { - TopupMemberTrafficTrigger.trafficToBuy( - topupAmount = 2_000L, + "hold for any purchase at a balance at or above zero" in { + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 2_000L, extraTrafficRemainder = 0L, - ) shouldBe 2_000L - TopupMemberTrafficTrigger.trafficToBuy( - topupAmount = 2_000L, + ) shouldBe true + TopupMemberTrafficTrigger.unblocks( + trafficToBuy = 2_000L, extraTrafficRemainder = 500L, - ) shouldBe 2_000L + ) shouldBe true } } } From 6f76e2801e59ebbf1bebeb50f8e9eb8cd84bbcab Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Tue, 13 Oct 2026 22:45:18 -0400 Subject: [PATCH 08/11] refactor: leave the outage settlement top-up to a follow-up PR [ci] Per review, the handling for removing the outage traffic allowance moves to a separate PR, so this one no longer touches the validator's top-up trigger or TopupUtil: buying a member's shortfall with its top-up, the plain top-up fallback and the two-pass budget all go there, with an integration test for that scenario. The integration test here ends once removing the allowance sets the limits back and bob is refused. listTotalPurchasedMemberTraffic now warns when it skips a member id that does not parse. Signed-off-by: Timothy Wu --- .../SyncOperatorTrafficIntegrationTest.scala | 34 +--- .../store/db/MemberTrafficQueries.scala | 11 +- .../TopupMemberTrafficTrigger.scala | 151 +++++------------- .../TopupMemberTrafficTriggerTest.scala | 132 ++------------- .../splice/wallet/util/TopupUtil.scala | 54 ++----- .../splice/wallet/util/TopupUtilTest.scala | 40 ----- test-full-class-names-non-integration.log | 1 - 7 files changed, 69 insertions(+), 354 deletions(-) delete mode 100644 apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index 7963a9dc6bf..7d9c3c61b90 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -64,9 +64,6 @@ class SyncOperatorTrafficIntegrationTest private val bobPurchase = 50_000L // Bought once the global synchronizer is back but before the allowance is removed. private val bobPurchaseWithAllowanceSet = 20_000L - // Less than the shortfall bob is left with once the allowance is removed, so one top-up only - // covers it if it buys the shortfall too. - private val bobTopupAmount = 12_000L // The same operator app with the allowance set, which the outage test runs in place of // syncOperator. Both use one store, so they never run at the same time. private val syncOperatorWithAllowanceName = "syncOperatorWithAllowance" @@ -96,20 +93,7 @@ class SyncOperatorTrafficIntegrationTest ConfigTransforms.updateAllValidatorConfigs { case (name, validatorConfig) => if (name == "bobValidator") validatorConfig - .focus(_.domains.global.buyExtraTraffic.targetThroughput) - .replace(NonNegativeNumeric.tryCreate(BigDecimal(0))) - .focus(_.domains.extra) - .modify(_.map { extra => - if (extra.alias == splitwellAlias) - extra - // 100 bytes a second over a two-minute interval, so bobTopupAmount per top-up - .focus(_.topup.targetThroughput) - .replace(NonNegativeNumeric.tryCreate(BigDecimal(100))) - .focus(_.topup.minTopupInterval) - .replace(NonNegativeFiniteDuration.ofMinutes(2)) - else extra - }) - // Resumed only once the outage test needs bob's top-up to cover his shortfall. + // Paused throughout, so bob's traffic on splitwell is only what the test buys him. .focus(_.automation) .modify(_.withPausedTrigger[TopupMemberTrafficTrigger]) else if (name == "aliceValidator") @@ -533,22 +517,6 @@ class SyncOperatorTrafficIntegrationTest ) } - // Refused, bob consumes nothing more, so this is what his top-up finds. - val shortfall = consumed(bob) - totalsWithoutAllowance(bob) - val topupTrigger = - bobValidatorBackend.appState.automation.trigger[TopupMemberTrafficTrigger] - setTriggersWithin(triggersToResumeAtStart = Seq(topupTrigger)) { - clue("one top-up buys bob's shortfall with it, and he transacts again") { - // Inside bob's two-minute top-up interval, so no second top-up is bought meanwhile. - eventually(100.seconds) { - extraTrafficLimit(bob) should be > consumed(bob) - } - // One purchase of the top-up and the shortfall together, not the top-up alone or two - // top-ups. - purchased()(bob) - totalsWithoutAllowance(bob) shouldBe bobTopupAmount + shortfall - ping(bobParticipant, synchronizerId) - } - } } } diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala index 4a66d5b4bee..a7c9d302217 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala @@ -81,7 +81,14 @@ trait MemberTrafficQueries extends AcsJdbcTypes with LimitHelpers { this: NamedL "listTotalPurchasedMemberTraffic", ) } yield rows.flatMap { case (member, total) => - // ingestion stores only members that parse, so this never drops a row in practice - Member.fromProtoPrimitive_(member).toOption.map(_ -> total) + Member.fromProtoPrimitive_(member) match { + case Right(parsed) => Some(parsed -> total) + case Left(error) => + // Ingestion only stores members that parse, so this is not expected. + logger.warn( + s"Skipping the purchased traffic of member $member, which does not parse: $error" + ) + None + } }.toMap } diff --git a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala index c11f39e1954..d25083443a7 100644 --- a/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala +++ b/apps/validator/src/main/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTrigger.scala @@ -116,56 +116,34 @@ class TopupMemberTrafficTrigger( ) validatorWallet <- ValidatorUtil.getValidatorWallet(store, walletManager) budget <- TopupUtil.topupBudget(scanConnection, validatorWallet.store) - candidates <- MonadUtil.sequentialTraverse(targets)(target => + candidates <- MonadUtil.sequentialTraverseFilter(targets)(target => // The traversal is sequential, so a failure would otherwise cost every remaining target // its top-up for this poll. Info, not warn: a synchronizer is legitimately unreachable // for a stretch of polls while it is being upgraded or recovered. retrieveTaskFor(target, activeSynchronizerId).recover { case ex => logger.info(s"Skipping the top-up for ${target.alias} in this poll", ex) - Seq.empty + None } ) - (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks( - candidates.filter(_.nonEmpty), - budget, - )(_.unblocks) + (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks(candidates, budget) // we do not even submit the topup tx if the validator does not have sufficient funds because we know // the tx would fail but it would still drain synchronizer traffic which we would like to avoid (see #11915). _ = unfunded.foreach(task => logger.warn( s"Insufficient funds to buy configured traffic amount. Please ensure that the validator's wallet has enough amulets to purchase " + - s"${BigDecimal(task.trafficToBuy) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." + - (if (task.shortfall > 0) - s" This member is also ${BigDecimal(task.shortfall) / 1e6} MB below zero there: for one purchase to get it submitting again, the wallet needs enough amulets for " + - s"${BigDecimal(task.trafficToBuy + task.shortfall) / 1e6} MB, the top-up and the shortfall together." - else "") + s"${BigDecimal(task.target.topupParameters.topupAmount) / 1e6} MB of traffic on ${task.target.alias} to continue healthy operation." ) ) - // A member below zero that gets the top-up alone, because the wallet does not cover both. - _ = funded - .filter(task => task.shortfall > 0 && task.extraTraffic == 0) - .foreach(task => - logger.warn( - s"Insufficient funds to buy the ${BigDecimal(task.shortfall) / 1e6} MB of traffic this member is below zero on ${task.target.alias} " + - "together with the configured top-up, so only the top-up is bought" + - (if (task.unblocks) ". " - else ", and the member cannot submit there until its top-ups have covered it. ") + - "Please ensure that the validator's wallet has enough amulets to buy both." - ) - ) } yield funded } - /** The purchases this target is due, preferred first, each paired with what it would cost the - * wallet: for a member below zero, its shortfall together with the configured top-up, and the - * top-up alone in case the wallet does not cover both. - */ + /** The task this target is due, paired with what it would cost the wallet. */ private def retrieveTaskFor( target: TopupMemberTrafficTrigger.Target, submissionSynchronizerId: SynchronizerId, )(implicit tc: TraceContext - ): Future[Seq[(TopupMemberTrafficTrigger.Task, BigDecimal)]] = + ): Future[Option[(TopupMemberTrafficTrigger.Task, BigDecimal)]] = for { currentTrafficState <- participantAdminConnection.getParticipantTrafficState( target.synchronizerId @@ -173,48 +151,44 @@ class TopupMemberTrafficTrigger( registration <- if (!target.needsRegistration) Future.successful(None) else scanConnection.lookupSynchronizerRegistration(target.synchronizerId.toProtoPrimitive) - purchases <- + task <- if (target.needsRegistration && registration.isEmpty) { // A buy without it is rejected on-ledger, and lastPurchasedAt never advances, so // nothing would stop the retry. logger.info( s"Not topping up ${target.alias}: Scan serves no registration for ${target.synchronizerId}" ) - Future.successful(Seq.empty) + Future.successful(None) } else for { topupState <- getOrCreateValidatorTopupState(target, submissionSynchronizerId) - tasks <- - if (!wantsTopup(target, currentTrafficState, topupState)) Future.successful(Seq.empty) - else { - val shortfall = - TopupMemberTrafficTrigger.shortfall(currentTrafficState.extraTrafficRemainder) - // The top-up alone still pays a shortfall down, one interval at a time. - val extraTrafficOptions = if (shortfall > 0) Seq(shortfall, 0L) else Seq(0L) + cost <- + if (!wantsTopup(target, currentTrafficState, topupState)) Future.successful(None) + else TopupUtil - // The cost is derived from this target's throughput and interval, plus the - // extra traffic bought with the top-up. - .topupCosts(scanConnection, target.topupConfig, clock, registration) - .map(costOf => - extraTrafficOptions.map(extraTraffic => - TopupMemberTrafficTrigger.Task( - target, - topupState, - currentTrafficState, - registration, - extraTraffic, - ) -> costOf(extraTraffic) - ) + // The cost is derived from this target's throughput and interval. + .minWalletBalanceForTopup( + scanConnection, + target.topupConfig, + clock, + registration, ) - } - } yield tasks - } yield purchases + .map(Some(_)) + } yield cost.map( + TopupMemberTrafficTrigger.Task( + target, + topupState, + currentTrafficState, + registration, + ) -> _ + ) + } yield task override protected def completeTask( task: TopupMemberTrafficTrigger.Task )(implicit tc: TraceContext): Future[TaskOutcome] = { val coBuyMemberTraffic = new CO_BuyMemberTraffic( - task.trafficToBuy, + task.target.topupParameters.topupAmount, task.topupState.payload.memberId, task.topupState.payload.synchronizerId, task.topupState.payload.migrationId, @@ -380,43 +354,23 @@ object TopupMemberTrafficTrigger { ) } - /** Splits candidates into the purchases the balance covers and the candidates it covers none - * of, both in target order. Each candidate lists its purchases preferred first, and every buy - * draws on the same wallet, so the balance is allocated in two passes, each in target order. - * The first gives each candidate its first purchase that the balance covers and that `unblocks` - * its member, leaving it able to submit. The second gives each candidate still unfunded its - * first purchase the rest covers, such as a top-up that pays a shortfall down without covering - * it. A purchase that leaves its member blocked thus cannot take the balance from one that keeps - * a later target submitting. A candidate the balance covers none of is reported by its last, - * cheapest, purchase. A `None` budget funds each candidate's first purchase. + /** Splits candidates into the ones the balance covers and the ones it does not, in target order. + * Every buy draws on the same wallet, so a target is only funded once the ones before it are. + * A `None` budget does not bound the purchases. */ private[automation] def fundedTasks[A]( - candidates: Seq[Seq[(A, BigDecimal)]], + candidates: Seq[(A, BigDecimal)], budget: Option[BigDecimal], - )(unblocks: A => Boolean): (Seq[A], Seq[A]) = budget match { - case None => (candidates.flatMap(_.headOption.map(_._1)), Seq.empty) + ): (Seq[A], Seq[A]) = budget match { + case None => (candidates.map(_._1), Seq.empty) case Some(balance) => - // One pass in target order: each candidate not funded yet gets its first purchase that the - // balance left covers and that is `eligible`. - def fund( - chosen: Seq[Option[A]], - balanceLeft: BigDecimal, - eligible: A => Boolean, - ): (Seq[Option[A]], BigDecimal) = - candidates.zip(chosen).foldLeft((Seq.empty[Option[A]], balanceLeft)) { - case ((done, remaining), (_, Some(task))) => (done :+ Some(task), remaining) - case ((done, remaining), (purchases, None)) => - purchases.find { case (task, cost) => cost <= remaining && eligible(task) } match { - case Some((task, cost)) => (done :+ Some(task), remaining - cost) - case None => (done :+ None, remaining) - } + val (funded, unfunded, _) = + candidates.foldLeft((Seq.empty[A], Seq.empty[A], balance)) { + case ((funded, unfunded, remaining), (task, cost)) => + if (cost <= remaining) (funded :+ task, unfunded, remaining - cost) + else (funded, unfunded :+ task, remaining) } - val (unblocking, balanceLeft) = fund(candidates.map(_ => Option.empty[A]), balance, unblocks) - val (funded, _) = fund(unblocking, balanceLeft, _ => true) - val unfunded = candidates.zip(funded).collect { case (purchases, None) => - purchases.lastOption.map(_._1) - } - (funded.flatten, unfunded.flatten) + (funded, unfunded) } /** Resolves the configured top-up targets against the synchronizers the participant is @@ -468,17 +422,6 @@ object TopupMemberTrafficTrigger { // Two aliases can resolve to one synchronizer; topupTargets is global-first, so global wins. .distinctBy(_.synchronizerId) - /** Traffic below zero, as once a dedicated synchronizer's operator removes an outage traffic - * allowance the member used. The member cannot submit until its purchases cover it. - */ - def shortfall(extraTrafficRemainder: Long): Long = math.max(0L, -extraTrafficRemainder) - - /** Whether buying `trafficToBuy` leaves a member with `extraTrafficRemainder` able to submit, - * that is above zero: at zero, with no base rate, it still cannot. - */ - def unblocks(trafficToBuy: Long, extraTrafficRemainder: Long): Boolean = - trafficToBuy > shortfall(extraTrafficRemainder) - final case class Task( target: Target, topupState: Contract[ValidatorTopUpState.ContractId, ValidatorTopUpState], @@ -486,26 +429,12 @@ object TopupMemberTrafficTrigger { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], - // Bought together with the configured top-up: the member's shortfall, so that one purchase - // covers it, or 0 for the top-up alone. That is every ordinary top-up, and the fallback - // where the wallet does not cover both. - extraTraffic: Long, ) extends PrettyPrinting { - - def shortfall: Long = TopupMemberTrafficTrigger.shortfall(trafficState.extraTrafficRemainder) - - def trafficToBuy: Long = target.topupParameters.topupAmount + extraTraffic - - /** Whether the member can submit once this purchase is granted. */ - def unblocks: Boolean = - TopupMemberTrafficTrigger.unblocks(trafficToBuy, trafficState.extraTrafficRemainder) - override def pretty: Pretty[Task] = prettyOfClass[Task]( param("target", _.target), param("topupState", _.topupState), param("trafficState", _.trafficState), - param("extraTraffic", _.extraTraffic), ) } } diff --git a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala index 2b96f8ba482..7e06f67f5cc 100644 --- a/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala +++ b/apps/validator/src/test/scala/org/lfdecentralizedtrust/splice/validator/automation/TopupMemberTrafficTriggerTest.scala @@ -109,135 +109,21 @@ class TopupMemberTrafficTriggerTest extends AnyWordSpec with BaseTest { "TopupMemberTrafficTrigger.fundedTasks" should { - // A purchase, and whether it leaves its member able to submit. What else it is attached to - // does not affect the allocation. - final case class Purchase(name: String, unblocks: Boolean = true) - def fundedTasks(candidates: Seq[Seq[(Purchase, BigDecimal)]], budget: Option[BigDecimal]) = - TopupMemberTrafficTrigger.fundedTasks(candidates, budget)(_.unblocks) - - val global = Purchase("global") - val dedicated = Purchase("dedicated") - val candidates = Seq(Seq(global -> BigDecimal(100)), Seq(dedicated -> BigDecimal(100))) + // Costs only; what they are attached to does not affect the allocation. + val candidates = Seq(globalAlias -> BigDecimal(100), dedicatedAlias -> BigDecimal(100)) "fund only what the balance covers, in order" in { // Enough for either one alone, not for both, so the second one misses out. - fundedTasks(candidates, Some(BigDecimal(150))) shouldBe (Seq(global), Seq(dedicated)) + val (funded, unfunded) = + TopupMemberTrafficTrigger.fundedTasks(candidates, Some(BigDecimal(150))) + funded shouldBe Seq(globalAlias) + unfunded shouldBe Seq(dedicatedAlias) } "fund every target when the balance does not bound the purchases" in { - fundedTasks(candidates, None) shouldBe (Seq(global, dedicated), Seq.empty) - } - - // A member below zero: its shortfall with the top-up, or the top-up alone, which leaves it - // below zero where the shortfall is larger than the top-up. - val topupAndShortfall = Purchase("top-up and shortfall") - val topupBelowZero = Purchase("top-up", unblocks = false) - val withShortfall = - Seq(topupAndShortfall -> BigDecimal(300), topupBelowZero -> BigDecimal(100)) - - "buy the shortfall with the top-up where the balance covers both" in { - fundedTasks(Seq(withShortfall), Some(BigDecimal(300))) shouldBe - (Seq(topupAndShortfall), Seq.empty) - } - - "fall back to the top-up alone where the balance does not cover the shortfall too" in { - fundedTasks(Seq(withShortfall), Some(BigDecimal(150))) shouldBe - (Seq(topupBelowZero), Seq.empty) - } - - "report the top-up alone where the balance does not cover even that" in { - fundedTasks(Seq(withShortfall), Some(BigDecimal(50))) shouldBe - (Seq.empty, Seq(topupBelowZero)) - } - - "buy the shortfall with the top-up where the balance does not bound the purchases" in { - fundedTasks(Seq(withShortfall), None) shouldBe (Seq(topupAndShortfall), Seq.empty) - } - - // A member 500 below zero with a top-up of 100, ahead of a healthy target whose top-up costs - // 100 too. - val blocked = Seq( - Purchase("blocked: top-up and shortfall") -> BigDecimal(600), - Purchase("blocked: top-up", unblocks = false) -> BigDecimal(100), - ) - val healthy = Purchase("healthy") - val healthyTarget = Seq(healthy -> BigDecimal(100)) - - "fund a later target before a top-up that leaves an earlier member below zero" in { - // Enough for one top-up: the healthy target's keeps it submitting, the other would not get - // its member submitting again. - fundedTasks(Seq(blocked, healthyTarget), Some(BigDecimal(150))) shouldBe - (Seq(healthy), Seq(blocked.last._1)) - } - - "fund a top-up that leaves its member below zero with what the other targets leave" in { - fundedTasks(Seq(blocked, healthyTarget), Some(BigDecimal(250))) shouldBe - (Seq(blocked.last._1, healthy), Seq.empty) - } - - "fund a top-up that covers the shortfall alone ahead of later targets" in { - // A shortfall smaller than the top-up, so the top-up alone gets the member submitting again. - val coveredByTopup = Purchase("top-up covering the shortfall") - val smallShortfall = Seq( - Purchase("top-up and small shortfall") -> BigDecimal(120), - coveredByTopup -> BigDecimal(100), - ) - // Enough for one top-up, which goes to the first target. - fundedTasks(Seq(smallShortfall, healthyTarget), Some(BigDecimal(110))) shouldBe - (Seq(coveredByTopup), Seq(healthy)) - } - - "fund each candidate's first purchase where the balance does not bound the purchases" in { - fundedTasks(Seq(blocked, healthyTarget), None) shouldBe - (Seq(blocked.head._1, healthy), Seq.empty) - } - } - - "TopupMemberTrafficTrigger.shortfall" should { - - "be what a balance below zero needs to reach zero" in { - TopupMemberTrafficTrigger.shortfall(-300L) shouldBe 300L - } - - "be zero for a balance at or above zero" in { - TopupMemberTrafficTrigger.shortfall(0L) shouldBe 0L - TopupMemberTrafficTrigger.shortfall(500L) shouldBe 0L - } - } - - "TopupMemberTrafficTrigger.unblocks" should { - - "hold for a purchase that takes the balance above zero" in { - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 2_300L, - extraTrafficRemainder = -300L, - ) shouldBe true - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 301L, - extraTrafficRemainder = -300L, - ) shouldBe true - } - - "not hold for a purchase that leaves the balance at or below zero" in { - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 300L, - extraTrafficRemainder = -300L, - ) shouldBe false - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 2_000L, - extraTrafficRemainder = -2_001L, - ) shouldBe false - } - - "hold for any purchase at a balance at or above zero" in { - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 2_000L, - extraTrafficRemainder = 0L, - ) shouldBe true - TopupMemberTrafficTrigger.unblocks( - trafficToBuy = 2_000L, - extraTrafficRemainder = 500L, - ) shouldBe true + val (funded, unfunded) = TopupMemberTrafficTrigger.fundedTasks(candidates, None) + funded shouldBe Seq(globalAlias, dedicatedAlias) + unfunded shouldBe empty } } } diff --git a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala index c90de02a130..a8c2b7c119c 100644 --- a/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala +++ b/apps/wallet/src/main/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtil.scala @@ -37,24 +37,7 @@ object TopupUtil { registration: Option[ ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] ], - )(implicit tc: TraceContext, ec: ExecutionContext, mat: Materializer): Future[BigDecimal] = - topupCosts(scanConnection, validatorTopupConfig, clock, registration).map(costOf => costOf(0L)) - - /** What one top-up, plus the extra traffic bought with it, costs in amulet, for any amount of - * extra traffic, from one read of the rules and the open round. - */ - def topupCosts( - scanConnection: ScanConnection, - validatorTopupConfig: ValidatorTopupConfig, - clock: Clock, - registration: Option[ - ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] - ], - )(implicit - tc: TraceContext, - ec: ExecutionContext, - mat: Materializer, - ): Future[Long => BigDecimal] = for { + )(implicit tc: TraceContext, ec: ExecutionContext, mat: Materializer): Future[BigDecimal] = for { amuletRules <- scanConnection.getAmuletRulesWithState() synchronizerFeesConfig = AmuletConfigSchedule(amuletRules) .getConfigAsOf(clock.now) @@ -67,33 +50,16 @@ object TopupUtil { validatorTopupConfig.topupTriggerPollingInterval, ) latestRound <- scanConnection.getLatestOpenMiningRound() - } yield (extraTraffic: Long) => - topupCost( - topupParameters, - extraTraffic, - BigDecimal(synchronizerFeesConfig.extraTrafficPrice), - latestRound.payload.amuletPrice, - registration, + amuletPrice = latestRound.payload.amuletPrice + extraTrafficPrice = BigDecimal(synchronizerFeesConfig.extraTrafficPrice) + } yield SpliceUtil + .synchronizerFees( + topupParameters.topupAmount, + extraTrafficPrice, + amuletPrice, + discountFactor(registration), ) - - /** What one top-up, plus `extraTraffic` bought with it, costs in amulet. */ - def topupCost( - topupParameters: ExtraTrafficTopupParameters, - extraTraffic: Long, - extraTrafficPrice: BigDecimal, - amuletPrice: BigDecimal, - registration: Option[ - ContractWithState[RegisteredSynchronizer.ContractId, RegisteredSynchronizer] - ], - ): BigDecimal = - SpliceUtil - .synchronizerFees( - topupParameters.topupAmount + extraTraffic, - extraTrafficPrice, - amuletPrice, - discountFactor(registration), - ) - ._2 + ._2 private def currentWalletBalance(scanConnection: ScanConnection, store: UserWalletStore)(implicit tc: TraceContext, diff --git a/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala b/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala deleted file mode 100644 index 89906b9b08a..00000000000 --- a/apps/wallet/src/test/scala/org/lfdecentralizedtrust/splice/wallet/util/TopupUtilTest.scala +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package org.lfdecentralizedtrust.splice.wallet.util - -import com.digitalasset.canton.BaseTestWordSpec -import com.digitalasset.canton.config.NonNegativeFiniteDuration - -class TopupUtilTest extends BaseTestWordSpec { - - // 2,000 bytes per top-up at $60 per MB is $0.12, or 24 amulets at $0.005 each. - private val topupParameters = - ExtraTrafficTopupParameters(2_000L, NonNegativeFiniteDuration.ofMinutes(1)) - private val extraTrafficPrice = BigDecimal(60) - private val amuletPrice = BigDecimal("0.005") - - "TopupUtil.topupCost" should { - - "price one top-up" in { - TopupUtil.topupCost( - topupParameters, - extraTraffic = 0L, - extraTrafficPrice, - amuletPrice, - registration = None, - ) shouldBe BigDecimal(24) - } - - "price a shortfall bought with the top-up as part of it" in { - // 5,000 bytes in all, so the funds check covers everything the purchase buys. - TopupUtil.topupCost( - topupParameters, - extraTraffic = 3_000L, - extraTrafficPrice, - amuletPrice, - registration = None, - ) shouldBe BigDecimal(60) - } - } -} diff --git a/test-full-class-names-non-integration.log b/test-full-class-names-non-integration.log index d199d50ee60..392ca705358 100644 --- a/test-full-class-names-non-integration.log +++ b/test-full-class-names-non-integration.log @@ -75,5 +75,4 @@ org.lfdecentralizedtrust.splice.validator.automation.TopupMemberTrafficTriggerTe org.lfdecentralizedtrust.splice.validator.store.DbValidatorConfigProviderTest org.lfdecentralizedtrust.splice.validator.store.DbValidatorStoreTest org.lfdecentralizedtrust.splice.wallet.util.ExtraTrafficTopupParametersTest -org.lfdecentralizedtrust.splice.wallet.util.TopupUtilTest org.lfdecentralizedtrust.splice.wart.ParTraverseTest From 17f563e4631b4263b2a35a456245f40b6e8ca0e9 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Wed, 14 Oct 2026 11:10:15 -0400 Subject: [PATCH 09/11] fix: address the first review of the split outage allowance [ci] - State the multi-operator requirement in the trigger and config docs: operators set and remove the allowance together, and a partial rollout only leaves limits stable when the sequencer threshold is above half the active sequencers. The config also notes the restart, the Scan dependency at startup, and that participants joining while the allowance is set get it too. - Only a grant that setSequencerTrafficControlState reports as not applied in time is skipped until the next poll, where it needs more than one sequencer; a failed call to the sequencer goes through the normal failure handling. A new attemptSetSequencerTrafficControlState returns that outcome as a status, and setSequencerTrafficControlState behaves as before. - Warn at ingestion, once per contract, when a purchase is dropped because its member id does not parse; the total query no longer has a warning that could not fire. - Unit tests for completeTask and isStaleTask, and a store test for the dropped purchase. - Scope the outage test's log suppression to the splitwell validator's expected warnings and bob's refused pings. Signed-off-by: Timothy Wu --- .../integration/EnvironmentDefinition.scala | 4 +- .../SyncOperatorTrafficIntegrationTest.scala | 14 +- .../SequencerAdminConnection.scala | 29 +- .../store/db/MemberTrafficQueries.scala | 20 +- ...cileDedicatedSequencerTrafficTrigger.scala | 57 ++-- .../config/SyncOperatorAppConfig.scala | 11 +- .../store/SyncOperatorStore.scala | 16 +- .../store/db/DbSyncOperatorStore.scala | 2 +- .../store/db/SyncOperatorStoreTest.scala | 41 ++- ...DedicatedSequencerTrafficTriggerTest.scala | 278 +++++++++++++++++- 10 files changed, 413 insertions(+), 59 deletions(-) diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala index 7483ff6b657..5f98ad7c30b 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/EnvironmentDefinition.scala @@ -526,8 +526,8 @@ case class EnvironmentDefinition( def withOnlyAliceValidatorConnectingToSplitwell: EnvironmentDefinition = withValidatorsDisconnectedFromSplitwell("bobValidator", "splitwellValidator") - /** Drops the splitwell apps and disconnects the given validators from the splitwell - * synchronizer. + /** Drops all splitwell apps, and clears all of the given validators' extra synchronizers, not + * just splitwell, so the global synchronizer is the only one they connect to. */ def withValidatorsDisconnectedFromSplitwell(validators: String*): EnvironmentDefinition = this diff --git a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala index 7d9c3c61b90..82f6f7a6fc3 100644 --- a/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala +++ b/apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala @@ -373,9 +373,17 @@ class SyncOperatorTrafficIntegrationTest totals.keySet shouldBe Set(alice, bob) try { - // The other apps on the operator's participant warn about the submissions that fail - // while it is cut off from the global synchronizer, so this tolerates their warnings. - loggerFactory.suppress(SuppressionRule.LevelAndAbove(Level.WARN)) { + // Suppresses only what is expected while the operator's participant is cut off from the + // global synchronizer, so the sync operator apps' own warnings and errors still show: + // the warnings of the operator's validator, whose triggers fail meanwhile, and the + // errors the test console (created in EnvironmentDefinition) logs for bob's refused + // pings. + loggerFactory.suppress( + (SuppressionRule.LoggerNameContains("validator=splitwellValidator") && + SuppressionRule.Level(Level.WARN)) || + (SuppressionRule.forLogger[EnvironmentDefinition] && + SuppressionRule.Level(Level.ERROR)) + ) { // Cut off from the global synchronizer, the operator's participant sees no purchases, // which is what an outage looks like to the operator. operatorParticipant.synchronizers.disconnect(globalAlias) diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala index 602f461d21c..1ef4b045ce1 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala @@ -429,7 +429,31 @@ class SequencerAdminConnection( timeout: NonNegativeFiniteDuration, )(implicit traceContext: TraceContext - ): Future[Unit] = { + ): Future[Unit] = + attemptSetSequencerTrafficControlState( + currentTrafficState, + currentSequencerState, + newTotalExtraTrafficLimit, + clock, + timeout, + ).flatMap(status => + if (status.isOk) Future.unit else Future.failed(status.asRuntimeException()) + ) + + /** As [[setSequencerTrafficControlState]], but returns its outcome as a status instead of + * failing on it: OK once the limit is set, DEADLINE_EXCEEDED if it was not set within + * `timeout`, and ABORTED on a concurrent change. Any other error, such as a call to the + * sequencer that keeps failing, still fails the returned future. + */ + def attemptSetSequencerTrafficControlState( + currentTrafficState: TrafficState, + currentSequencerState: TopologyResult[SequencerSynchronizerState], + newTotalExtraTrafficLimit: NonNegativeLong, + clock: Clock, + timeout: NonNegativeFiniteDuration, + )(implicit + traceContext: TraceContext + ): Future[io.grpc.Status] = { val msgPrefix = s"setting traffic state for ${currentTrafficState.member} to $newTotalExtraTrafficLimit with next serial ${currentTrafficState.nextSerial}:" val deadline = clock.now.plus(timeout.asJavaApproximation) @@ -485,9 +509,6 @@ class SequencerAdminConnection( ).map(_ => ()), logger, ) - .flatMap(status => - if (status.isOk) Future.unit else Future.failed(status.asRuntimeException()) - ) } def getSequencerPruningStatus()(implicit diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala index a7c9d302217..38968e4744c 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/store/db/MemberTrafficQueries.scala @@ -60,7 +60,10 @@ trait MemberTrafficQueries extends AcsJdbcTypes with LimitHelpers { this: NamedL ec: ExecutionContext, tc: TraceContext, closeContext: CloseContext, - ): Future[Map[Member, Long]] = + ): Future[Map[Member, Long]] = { + // Throws on a member that does not parse: ingestion stores a member only once it parses, so + // that would be a corrupt row. + import Member.DbStorageImplicits.getResultMember for { rows <- storage .query( @@ -76,19 +79,10 @@ trait MemberTrafficQueries extends AcsJdbcTypes with LimitHelpers { this: NamedL and member_traffic_domain = $synchronizerId and member_traffic_member is not null group by member_traffic_member - """.as[(String, Long)], + """.as[(Member, Long)], // the callers' method name, which is what the DB retry logs report "listTotalPurchasedMemberTraffic", ) - } yield rows.flatMap { case (member, total) => - Member.fromProtoPrimitive_(member) match { - case Right(parsed) => Some(parsed -> total) - case Left(error) => - // Ingestion only stores members that parse, so this is not expected. - logger.warn( - s"Skipping the purchased traffic of member $member, which does not parse: $error" - ) - None - } - }.toMap + } yield rows.toMap + } } diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index 5ea02e3fca6..dfceb8620ff 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -4,7 +4,6 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import cats.syntax.apply.* -import com.daml.grpc.{GrpcException, GrpcStatus} import com.digitalasset.canton.config.NonNegativeFiniteDuration import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLong} import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} @@ -48,9 +47,12 @@ import scala.concurrent.{ExecutionContext, Future} * would not scale to the global synchronizer. * * On a synchronizer whose sequencers several operators run, a limit changes only once the - * sequencer group's threshold of operators set the same one, so they set and remove the allowance - * together. A grant not applied within `trafficBalanceReconciliationDelay` is sent again on the - * next poll. + * sequencer group's threshold of operators have sent the same one, and the operators have to set + * and remove the allowance together. While only some of them have it set, limits only stay stable + * if the threshold is above half the active sequencers: with a lower threshold, the operators with + * the allowance and the ones without it can each reach the threshold, and override each other's + * limits on every poll. A grant not applied within `trafficBalanceReconciliationDelay` is sent + * again on the next poll. */ class ReconcileDedicatedSequencerTrafficTrigger( override protected val context: TriggerContext, @@ -96,33 +98,38 @@ class ReconcileDedicatedSequencerTrafficTrigger( if (current.isDone(limit)) Future.successful(TaskNoop) else connection - .setSequencerTrafficControlState( + .attemptSetSequencerTrafficControlState( trafficState, sequencerState, NonNegativeLong.tryCreate(current.target), context.pollingClock, trafficBalanceReconciliationDelay, ) - .map[TaskOutcome](_ => - TaskSuccess( - s"Set the traffic limit of ${task.member} from $limit to ${current.target}" - ) - ) - // Where a grant needs more than one sequencer, it is not applied until enough of the - // sequencers' operators send the same limit, so a timeout is expected while they - // disagree. Retrying the task would resend it until the retries run out and then log - // an error, so the next poll sends it again instead. Where one sequencer is enough, a - // timeout is a real failure and is handled as any other. - .recover { - case GrpcException(GrpcStatus(Status.Code.DEADLINE_EXCEEDED, _), _) - if sequencerState.mapping.threshold.value > 1 => - logger.info( - s"The grant of a traffic limit of ${current.target} for ${task.member} was not " + - s"applied within $trafficBalanceReconciliationDelay: it needs " + - s"${sequencerState.mapping.threshold.value} of the synchronizer's sequencers to " + - "send the same limit. The next poll sends it again." - ) - TaskNoop + .flatMap { status => + status.getCode match { + case Status.Code.OK => + Future.successful( + TaskSuccess( + s"Set the traffic limit of ${task.member} from $limit to ${current.target}" + ) + ) + // Where a grant needs more than one sequencer, it is not applied until enough of + // the sequencers' operators send the same limit, so it not being applied in time is + // expected while they disagree. Retrying the task would resend it until the retries + // run out and then log an error, so the next poll sends it again instead. A call to + // the sequencer that times out fails the future rather than returning this status, + // so it is handled as any other failure, as is a grant not applied in time where + // one sequencer is enough. + case Status.Code.DEADLINE_EXCEEDED if sequencerState.mapping.threshold.value > 1 => + logger.info( + s"The grant of a traffic limit of ${current.target} for ${task.member} was " + + s"not applied within $trafficBalanceReconciliationDelay: it needs " + + s"${sequencerState.mapping.threshold.value} of the synchronizer's " + + "sequencers to send the same limit. The next poll sends it again." + ) + Future.successful(TaskNoop) + case _ => Future.failed(status.asRuntimeException()) + } } } yield outcome diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala index 8d4e24a6408..b912c902d6d 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala @@ -98,9 +98,14 @@ case class SyncOperatorAppBackendConfig( NonNegativeFiniteDuration.ofSeconds(10), // Bytes each participant on this synchronizer may use past what has been bought for it. The // operator sets it while the global synchronizer is unavailable, so nothing can be bought, and - // removes it once it is back, each time followed by a restart. While it is set, this app keeps - // every participant's limit at its purchased total plus this, including participants with no - // purchase on record and ones that join meanwhile; while it is unset, at its purchased total. + // removes it once it is back. Setting or removing it only takes effect on a restart of this + // app, and the app only starts once the configured Scan is reachable. While it is set, this app + // keeps every participant's limit at its purchased total plus this; while it is unset, at its + // purchased total. Every participant gets it, including ones with no purchase on record and + // ones that join while it is set, so keep onboarding to the synchronizer restricted meanwhile. + // Where several operators run the synchronizer's sequencers, they set and remove it together: + // while only some have it set, limits only stay stable with a sequencer threshold above half + // the active sequencers. outageTrafficAllowance: Option[PositiveLong] = None, // Set to false to disable the DB-level exclusive lock that prevents two sync operator instances // from running concurrently against the same database. Only disable for migration scenarios diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala index eae0711758b..98aceea1d8d 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala @@ -4,7 +4,7 @@ package org.lfdecentralizedtrust.splice.syncoperator.store import com.digitalasset.canton.lifecycle.CloseContext -import com.digitalasset.canton.logging.NamedLoggerFactory +import com.digitalasset.canton.logging.{NamedLoggerFactory, TracedLogger} import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} import com.digitalasset.canton.resource.DbStorage import com.digitalasset.canton.topology.{Member, ParticipantId, PartyId, SynchronizerId} @@ -88,7 +88,7 @@ object SyncOperatorStore { ) } - def contractFilter(key: Key): MultiDomainAcsStore.ContractFilter[ + def contractFilter(key: Key, logger: TracedLogger): MultiDomainAcsStore.ContractFilter[ SyncOperatorAcsStoreRowData, AcsInterfaceViewRowData.NoInterfacesIngested, ] = { @@ -112,7 +112,17 @@ object SyncOperatorStore { .fromProtoPrimitive_(contract.payload.memberId) // we ignore cases where the member id is invalid instead of throwing an exception // to avoid killing the entire ingestion pipeline as a result - .fold(_ => None, Some(_)), + .fold( + error => { + // Ingestion has no trace context to log with. + logger.warn( + s"Skipping the purchased traffic in ${contract.contractId.contractId} of member " + + s"${contract.payload.memberId}, which does not parse: $error" + )(TraceContext.empty) + None + }, + Some(_), + ), // the filter has already established these agree, so avoid parsing per contract memberTrafficDomain = Some(key.synchronizerId), totalTrafficPurchased = Some(contract.payload.totalPurchased), diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala index b945fe705ef..c5e134fb0ab 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/db/DbSyncOperatorStore.scala @@ -76,7 +76,7 @@ class DbSyncOperatorStore( override lazy val acsContractFilter: MultiDomainAcsStore.ContractFilter[ SyncOperatorAcsStoreRowData, AcsInterfaceViewRowData.NoInterfacesIngested, - ] = SyncOperatorStore.contractFilter(key) + ] = SyncOperatorStore.contractFilter(key, logger) import multiDomainAcsStore.waitUntilAcsIngested diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala index 5b1e5fd50e9..0ddb14b8898 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/store/db/SyncOperatorStoreTest.scala @@ -13,7 +13,7 @@ import com.digitalasset.canton.topology.{Member, PartyId, SynchronizerId} import org.lfdecentralizedtrust.splice.codegen.java.splice.decentralizedsynchronizer.MemberTraffic import org.lfdecentralizedtrust.splice.config.IngestionConfig import org.lfdecentralizedtrust.splice.environment.{DarResources, RetryProvider} -import org.lfdecentralizedtrust.splice.store.{HardLimit, Limit, StoreTestBase} +import org.lfdecentralizedtrust.splice.store.{HardLimit, Limit, MultiDomainAcsStore, StoreTestBase} import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore import org.lfdecentralizedtrust.splice.syncoperator.store.db.DbSyncOperatorStore import org.lfdecentralizedtrust.splice.util.{Contract, ResourceTemplateDecoder, TemplateJsonDecoder} @@ -41,10 +41,25 @@ abstract class SyncOperatorStoreTest extends StoreTestBase with HasExecutionCont synchronizerId: SynchronizerId = ourSynchronizer, operator: Option[PartyId] = Some(operatorParty), migrationId: Long = 0L, + ): Contract[MemberTraffic.ContractId, MemberTraffic] = + memberTrafficOfId( + member.toProtoPrimitive, + totalPurchased, + synchronizerId, + operator, + migrationId, + ) + + private def memberTrafficOfId( + memberId: String, + totalPurchased: Long, + synchronizerId: SynchronizerId = ourSynchronizer, + operator: Option[PartyId] = Some(operatorParty), + migrationId: Long = 0L, ): Contract[MemberTraffic.ContractId, MemberTraffic] = { val template = new MemberTraffic( dsoParty.toProtoPrimitive, - member.toProtoPrimitive, + memberId, synchronizerId.toProtoPrimitive, migrationId, totalPurchased, @@ -63,8 +78,13 @@ abstract class SyncOperatorStoreTest extends StoreTestBase with HasExecutionCont private def ingest( store: SyncOperatorStore, contracts: Seq[Contract[MemberTraffic.ContractId, MemberTraffic]], + ): Future[Unit] = ingestInto(store.multiDomainAcsStore.testIngestionSink, contracts) + + private def ingestInto( + sink: MultiDomainAcsStore.IngestionSink, + contracts: Seq[Contract[MemberTraffic.ContractId, MemberTraffic]], ): Future[Unit] = - store.multiDomainAcsStore.testIngestionSink.ingestAcs( + sink.ingestAcs( nextOffset(), // the purchase is made and recorded on the decentralized synchronizer, not on the dedicated // one it names @@ -159,6 +179,21 @@ abstract class SyncOperatorStoreTest extends StoreTestBase with HasExecutionCont } yield totals shouldBe Map(alice -> 350L, bob -> 700L) } + "leave a purchase whose member id does not parse out of every total, with a warning" in { + for { + store <- mkStore() + _ <- loggerFactory.assertLogs( + // Past the test sink, which fails on any warning. + ingestInto( + store.multiDomainAcsStore.ingestionSink, + Seq(memberTraffic(alice, 100L), memberTrafficOfId("not-a-member", 250L)), + ), + _.warningMessage should include("of member not-a-member, which does not parse"), + ) + totals <- store.listTotalPurchasedMemberTraffic() + } yield totals shouldBe Map(alice -> 100L) + } + "list no totals before any purchase" in { for { store <- mkStore() diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala index fc5e4f47c34..36f810db8ba 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala @@ -3,21 +3,79 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation -import com.digitalasset.canton.BaseTest +import com.daml.metrics.api.noop.NoOpMetricsFactory +import com.daml.nonempty.NonEmpty +import com.digitalasset.canton.admin.api.client.data.{ + NodeStatus, + SequencerAdminStatus, + SequencerHealthStatus, + SequencerStatus, + TopologyQueueStatus, +} +import com.digitalasset.canton.admin.api.client.data.topology.BaseResult +import com.digitalasset.canton.concurrent.FutureSupervisor +import com.digitalasset.canton.config.{NonNegativeFiniteDuration, ProcessingTimeout} +import com.digitalasset.canton.config.RequireTypes.{ + NonNegativeInt, + NonNegativeLong, + PositiveInt, + PositiveLong, +} +import com.digitalasset.canton.crypto.{Fingerprint, TestHash} +import com.digitalasset.canton.data.CantonTimestamp +import com.digitalasset.canton.sequencing.protocol +import com.digitalasset.canton.time.{Clock, SimClock} +import com.digitalasset.canton.topology.admin.grpc.TopologyStoreId +import com.digitalasset.canton.topology.transaction.{SequencerSynchronizerState, TopologyChangeOp} +import com.digitalasset.canton.topology.transaction.TopologyTransaction.TxHash import com.digitalasset.canton.topology.{ MediatorId, Member, ParticipantId, + PartyId, + PhysicalSynchronizerId, SequencerId, + SynchronizerId, UniqueIdentifier, } +import com.digitalasset.canton.tracing.TraceContext +import com.digitalasset.canton.version.{ProtocolVersion, ReleaseVersion} +import com.digitalasset.canton.{BaseTest, HasActorSystem, HasExecutionContext} +import io.grpc.{Status, StatusRuntimeException} +import org.lfdecentralizedtrust.splice.automation.{ + TaskNoop, + TaskOutcome, + TaskSuccess, + TriggerContext, + TriggerEnabledSynchronization, +} +import org.lfdecentralizedtrust.splice.config.AutomationConfig +import org.lfdecentralizedtrust.splice.environment.{ + RetryProvider, + SequencerAdminConnection, + SynchronizerNodeService, +} +import org.lfdecentralizedtrust.splice.environment.SequencerAdminConnection.TrafficState +import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.{ + TopologyResult, + TopologySnapshot, +} +import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode import org.lfdecentralizedtrust.splice.syncoperator.automation.ReconcileDedicatedSequencerTrafficTrigger.{ Task, tasks, } +import org.lfdecentralizedtrust.splice.syncoperator.store.SyncOperatorStore import org.scalatest.wordspec.AnyWordSpec -class ReconcileDedicatedSequencerTrafficTriggerTest extends AnyWordSpec with BaseTest { +import java.time.Instant +import scala.concurrent.Future + +class ReconcileDedicatedSequencerTrafficTriggerTest + extends AnyWordSpec + with BaseTest + with HasActorSystem + with HasExecutionContext { private val alice: Member = ParticipantId.tryFromProtoPrimitive("PAR::alice::default") private val bob: Member = ParticipantId.tryFromProtoPrimitive("PAR::bob::default") @@ -27,6 +85,12 @@ class ReconcileDedicatedSequencerTrafficTriggerTest extends AnyWordSpec with Bas private val allowance = Some(50L) + // What the store holds as alice's purchased total. + private val aliceTotal = 100L + + private val synchronizerId = SynchronizerId.tryFromString("dedicated::default") + private val otherSynchronizerId = SynchronizerId.tryFromString("other::default") + "ReconcileDedicatedSequencerTrafficTrigger.tasks" should { "grant each participant its purchased total" in { @@ -161,4 +225,214 @@ class ReconcileDedicatedSequencerTrafficTriggerTest extends AnyWordSpec with Bas ) shouldBe empty } } + + "ReconcileDedicatedSequencerTrafficTrigger" should { + + val task = Task(alice, aliceTotal, allowance) + // Her purchased total plus the allowance. + val target = 150L + + "set a participant's limit to its target" in { + val f = new Fixture(threshold = 1, limit = 0L) + f.grantReturns(Future.successful(Status.OK)) + f.trigger.complete(task).futureValue shouldBe a[TaskSuccess] + verify(f.connection).attemptSetSequencerTrafficControlState( + any[TrafficState], + any[TopologyResult[SequencerSynchronizerState]], + eqTo(NonNegativeLong.tryCreate(target)), + any[Clock], + any[NonNegativeFiniteDuration], + )(any[TraceContext]) + } + + "leave a participant alone that its total as it is now already puts at its target" in { + // Retrieved before a purchase of the whole total, which has been granted since. + val f = new Fixture(threshold = 1, limit = target) + f.trigger.complete(task.copy(total = 0L)).futureValue shouldBe TaskNoop + f.verifyNoGrant() + } + + "leave a grant not applied in time to the next poll where it needs several sequencers" in { + val f = new Fixture(threshold = 2, limit = 0L) + f.grantReturns(Future.successful(Status.DEADLINE_EXCEEDED)) + f.trigger.complete(task).futureValue shouldBe TaskNoop + } + + "fail a grant not applied in time where one sequencer is enough" in { + val f = new Fixture(threshold = 1, limit = 0L) + f.grantReturns(Future.successful(Status.DEADLINE_EXCEEDED)) + failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED + } + + "fail a grant whose call to the sequencer times out, also where it needs several sequencers" in { + val f = new Fixture(threshold = 2, limit = 0L) + f.grantReturns(Future.failed(Status.DEADLINE_EXCEEDED.asRuntimeException())) + failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED + } + + "fail a grant a concurrent change aborted, so that it is retried" in { + val f = new Fixture(threshold = 2, limit = 0L) + f.grantReturns(Future.successful(Status.ABORTED)) + failureCode(f.trigger.complete(task)) shouldBe Status.Code.ABORTED + } + + "set no limit on a sequencer that serves another synchronizer" in { + val f = new Fixture(threshold = 1, limit = 0L, served = otherSynchronizerId) + failureCode(f.trigger.complete(task)) shouldBe Status.Code.FAILED_PRECONDITION + f.verifyNoGrant() + } + + "treat a task as stale once the participant is at its target" in { + new Fixture(threshold = 1, limit = target).trigger.stale(task).futureValue shouldBe true + } + + "treat a task as stale once the participant has no traffic state" in { + new Fixture(threshold = 1, limit = 0L, hasTrafficState = false).trigger + .stale(task) + .futureValue shouldBe true + } + + "not treat a task as stale while the participant is not at its target" in { + new Fixture(threshold = 1, limit = 0L).trigger.stale(task).futureValue shouldBe false + } + } + + private lazy val clock = new SimClock(loggerFactory = loggerFactory) + + private lazy val triggerContext = TriggerContext( + AutomationConfig(), + clock, + clock, + TriggerEnabledSynchronization.Noop, + RetryProvider(loggerFactory, ProcessingTimeout(), FutureSupervisor.Noop, NoOpMetricsFactory), + loggerFactory, + NoOpMetricsFactory, + ) + + private def failureCode(outcome: Future[TaskOutcome]): Status.Code = + inside(outcome.failed.futureValue) { case e: StatusRuntimeException => e.getStatus.getCode } + + /** The trigger on a mocked sequencer, store and node service, with alice at `limit` on a + * sequencer group of `threshold` sequencers that serves `served`. + */ + private class Fixture( + threshold: Int, + limit: Long, + hasTrafficState: Boolean = true, + served: SynchronizerId = synchronizerId, + ) { + val connection: SequencerAdminConnection = mock[SequencerAdminConnection] + private val store = mock[SyncOperatorStore] + private val nodeService = mock[SynchronizerNodeService[SyncOperatorSynchronizerNode]] + + when(nodeService.sequencerAdminConnection()(any[TraceContext])) + .thenReturn(Future.successful(connection)) + when(connection.getStatus(any[TraceContext])) + .thenReturn(Future.successful(sequencerStatus(served))) + when(store.key).thenReturn( + SyncOperatorStore.Key( + PartyId.tryFromProtoPrimitive("operator::default"), + PartyId.tryFromProtoPrimitive("dso::default"), + synchronizerId, + ) + ) + when(store.getTotalPurchasedMemberTraffic(eqTo(alice))(any[TraceContext])) + .thenReturn(Future.successful(aliceTotal)) + private val trafficState = TrafficState( + alice, + protocol.TrafficState + .empty(CantonTimestamp.Epoch) + .copy( + extraTrafficPurchased = NonNegativeLong.tryCreate(limit), + serial = Some(PositiveInt.one), + ), + ) + when(connection.getSequencerTrafficControlState(eqTo(alice))(any[TraceContext])) + .thenReturn( + if (hasTrafficState) Future.successful(trafficState) + else Future.failed(Status.NOT_FOUND.asRuntimeException()) + ) + when(connection.lookupSequencerTrafficControlState(eqTo(alice))(any[TraceContext])) + .thenReturn(Future.successful(Option.when(hasTrafficState)(trafficState))) + when(connection.getSequencerSynchronizerState(any[TopologySnapshot])(any[TraceContext])) + .thenReturn(Future.successful(sequencerState(threshold))) + + val trigger = new TestTrigger(store, nodeService) + + def grantReturns(status: Future[Status]): Unit = + when( + connection.attemptSetSequencerTrafficControlState( + any[TrafficState], + any[TopologyResult[SequencerSynchronizerState]], + any[NonNegativeLong], + any[Clock], + any[NonNegativeFiniteDuration], + )(any[TraceContext]) + ).thenReturn(status) + + def verifyNoGrant(): Unit = + verify(connection, never).attemptSetSequencerTrafficControlState( + any[TrafficState], + any[TopologyResult[SequencerSynchronizerState]], + any[NonNegativeLong], + any[Clock], + any[NonNegativeFiniteDuration], + )(any[TraceContext]) + } + + private class TestTrigger( + store: SyncOperatorStore, + nodeService: SynchronizerNodeService[SyncOperatorSynchronizerNode], + ) extends ReconcileDedicatedSequencerTrafficTrigger( + triggerContext, + store, + nodeService, + NonNegativeFiniteDuration.ofSeconds(10), + allowance.map(PositiveLong.tryCreate), + ) { + def complete(task: Task): Future[TaskOutcome] = completeTask(task) + def stale(task: Task): Future[Boolean] = isStaleTask(task) + } + + private def sequencerStatus(served: SynchronizerId): NodeStatus[SequencerStatus] = + NodeStatus.Success( + SequencerStatus( + uid = UniqueIdentifier.tryCreate("sequencer", "default"), + synchronizerId = + PhysicalSynchronizerId(served, NonNegativeInt.tryCreate(0), ProtocolVersion.latest), + uptime = java.time.Duration.ZERO, + ports = Map.empty, + connectedParticipants = Seq.empty, + connectedMediators = Seq.empty, + sequencer = SequencerHealthStatus(isActive = true), + topologyQueue = TopologyQueueStatus(0, 0, 0), + admin = SequencerAdminStatus(acceptsAdminChanges = true), + components = Seq.empty, + version = ReleaseVersion.current, + protocolVersion = ProtocolVersion.latest, + ) + ) + + // A group of `threshold` sequencers, all of which have to send a grant. + private def sequencerState(threshold: Int): TopologyResult[SequencerSynchronizerState] = + TopologyResult( + BaseResult( + storeId = TopologyStoreId.Authorized, + validFrom = Instant.EPOCH, + validUntil = None, + sequenced = Instant.EPOCH, + operation = TopologyChangeOp.Replace, + transactionHash = TxHash(TestHash.digest("sequencer-state")), + serial = PositiveInt.one, + signedBy = NonEmpty(Seq, Fingerprint.tryFromString("default")), + ), + SequencerSynchronizerState + .create( + synchronizerId, + PositiveInt.tryCreate(threshold), + (1 to threshold).map(i => SequencerId(UniqueIdentifier.tryCreate(s"seq$i", "default"))), + Seq.empty, + ) + .value, + ) } From 0eb7cc83b645bf93033654d9a8e4b04680d973eb Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Wed, 14 Oct 2026 11:57:48 -0400 Subject: [PATCH 10/11] docs: describe when operators' traffic limits can flip, and test the stale re-read [ci] The trigger and config docs now say limits only stay stable with a sequencer threshold above half the active sequencers whenever the operators' targets differ, which includes one operator's store not having ingested a purchase yet, not just a partial allowance rollout. Above half, a split where neither side reaches the threshold leaves the limits as they are until enough operators agree. Also: a test that isStaleTask re-reads the total, clearer test names, and the ingestion warning's comment says the row encoder gets no trace context. Signed-off-by: Timothy Wu --- ...ncileDedicatedSequencerTrafficTrigger.scala | 18 ++++++++++-------- .../config/SyncOperatorAppConfig.scala | 6 +++--- .../syncoperator/store/SyncOperatorStore.scala | 6 +++--- ...eDedicatedSequencerTrafficTriggerTest.scala | 11 +++++++++-- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index dfceb8620ff..48b58be7150 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -39,19 +39,21 @@ import scala.concurrent.{ExecutionContext, Future} * alone. Mediators are left to [[MediatorUnlimitedTrafficTrigger]], and sequencers are not charged * traffic. * - * Limits are set exactly, both ways: setting the allowance raises them, and removing it lowers them - * back to the purchased totals, so a participant that used the allowance is below zero until its - * purchases cover what it consumed. + * Limits are set exactly, both ways: setting the allowance raises them, and removing it lowers + * them back to the purchased totals, so a participant that used the allowance is below zero until + * its purchases cover what it consumed. * * Reading every member on each poll is fine for the few members of a dedicated synchronizer, but * would not scale to the global synchronizer. * * On a synchronizer whose sequencers several operators run, a limit changes only once the - * sequencer group's threshold of operators have sent the same one, and the operators have to set - * and remove the allowance together. While only some of them have it set, limits only stay stable - * if the threshold is above half the active sequencers: with a lower threshold, the operators with - * the allowance and the ones without it can each reach the threshold, and override each other's - * limits on every poll. A grant not applied within `trafficBalanceReconciliationDelay` is sent + * sequencer group's threshold of its sequencers have sent the same one, so the operators have to + * set and remove the allowance together. Whenever their targets differ, as while only some have + * the allowance set or while one operator's store has not ingested a purchase yet, limits only + * stay stable if that threshold is above half the active sequencers: with a lower one, the + * sequencers on each side can reach it and override each other's limits on every poll. Above + * half, a split where neither side reaches it leaves the limits where they are until enough of + * them send the same limit. A grant not applied within `trafficBalanceReconciliationDelay` is sent * again on the next poll. */ class ReconcileDedicatedSequencerTrafficTrigger( diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala index b912c902d6d..1af10c32faf 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/config/SyncOperatorAppConfig.scala @@ -103,9 +103,9 @@ case class SyncOperatorAppBackendConfig( // keeps every participant's limit at its purchased total plus this; while it is unset, at its // purchased total. Every participant gets it, including ones with no purchase on record and // ones that join while it is set, so keep onboarding to the synchronizer restricted meanwhile. - // Where several operators run the synchronizer's sequencers, they set and remove it together: - // while only some have it set, limits only stay stable with a sequencer threshold above half - // the active sequencers. + // Where several operators run the synchronizer's sequencers, they set and remove it together. + // While their targets differ, as while only some have it set, limits only stay stable with a + // sequencer threshold above half the active sequencers. outageTrafficAllowance: Option[PositiveLong] = None, // Set to false to disable the DB-level exclusive lock that prevents two sync operator instances // from running concurrently against the same database. Only disable for migration scenarios diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala index 98aceea1d8d..7f00ede977a 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/store/SyncOperatorStore.scala @@ -114,10 +114,10 @@ object SyncOperatorStore { // to avoid killing the entire ingestion pipeline as a result .fold( error => { - // Ingestion has no trace context to log with. + // The row encoder gets no trace context. logger.warn( - s"Skipping the purchased traffic in ${contract.contractId.contractId} of member " + - s"${contract.payload.memberId}, which does not parse: $error" + s"Skipping the purchased traffic in ${contract.contractId.contractId} of " + + s"member ${contract.payload.memberId}, which does not parse: $error" )(TraceContext.empty) None }, diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala index 36f810db8ba..45846492c97 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala @@ -245,7 +245,7 @@ class ReconcileDedicatedSequencerTrafficTriggerTest )(any[TraceContext]) } - "leave a participant alone that its total as it is now already puts at its target" in { + "send no grant when the participant's current total already puts it at its target" in { // Retrieved before a purchase of the whole total, which has been granted since. val f = new Fixture(threshold = 1, limit = target) f.trigger.complete(task.copy(total = 0L)).futureValue shouldBe TaskNoop @@ -264,7 +264,7 @@ class ReconcileDedicatedSequencerTrafficTriggerTest failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED } - "fail a grant whose call to the sequencer times out, also where it needs several sequencers" in { + "fail a grant whose sequencer call times out, even where several sequencers are needed" in { val f = new Fixture(threshold = 2, limit = 0L) f.grantReturns(Future.failed(Status.DEADLINE_EXCEEDED.asRuntimeException())) failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED @@ -286,6 +286,13 @@ class ReconcileDedicatedSequencerTrafficTriggerTest new Fixture(threshold = 1, limit = target).trigger.stale(task).futureValue shouldBe true } + "treat a task as stale once the participant's current total puts it at its target" in { + // Retrieved before a purchase of the whole total, which has been granted since. + new Fixture(threshold = 1, limit = target).trigger + .stale(task.copy(total = 0L)) + .futureValue shouldBe true + } + "treat a task as stale once the participant has no traffic state" in { new Fixture(threshold = 1, limit = 0L, hasTrafficState = false).trigger .stale(task) From 1e7b8cd660e69a6f88c77bb390d58ab0bbc2cf07 Mon Sep 17 00:00:00 2001 From: Timothy Wu Date: Wed, 14 Oct 2026 13:17:28 -0400 Subject: [PATCH 11/11] refactor: handle dedicated traffic grants as on the global synchronizer [ci] Per review: completeTask always sends the grant for the target computed from the member's current total, without an early return when it is already there; a grant that is not applied in time fails and is retried, as on the global synchronizer, so SequencerAdminConnection is back to main; and the trigger no longer checks on every call which synchronizer the sequencer serves, as the store only holds purchases for this one. Signed-off-by: Timothy Wu --- .../SequencerAdminConnection.scala | 29 +--- ...cileDedicatedSequencerTrafficTrigger.scala | 85 ++--------- ...DedicatedSequencerTrafficTriggerTest.scala | 132 ++++-------------- 3 files changed, 42 insertions(+), 204 deletions(-) diff --git a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala index 1ef4b045ce1..602f461d21c 100644 --- a/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala +++ b/apps/common/src/main/scala/org/lfdecentralizedtrust/splice/environment/SequencerAdminConnection.scala @@ -429,31 +429,7 @@ class SequencerAdminConnection( timeout: NonNegativeFiniteDuration, )(implicit traceContext: TraceContext - ): Future[Unit] = - attemptSetSequencerTrafficControlState( - currentTrafficState, - currentSequencerState, - newTotalExtraTrafficLimit, - clock, - timeout, - ).flatMap(status => - if (status.isOk) Future.unit else Future.failed(status.asRuntimeException()) - ) - - /** As [[setSequencerTrafficControlState]], but returns its outcome as a status instead of - * failing on it: OK once the limit is set, DEADLINE_EXCEEDED if it was not set within - * `timeout`, and ABORTED on a concurrent change. Any other error, such as a call to the - * sequencer that keeps failing, still fails the returned future. - */ - def attemptSetSequencerTrafficControlState( - currentTrafficState: TrafficState, - currentSequencerState: TopologyResult[SequencerSynchronizerState], - newTotalExtraTrafficLimit: NonNegativeLong, - clock: Clock, - timeout: NonNegativeFiniteDuration, - )(implicit - traceContext: TraceContext - ): Future[io.grpc.Status] = { + ): Future[Unit] = { val msgPrefix = s"setting traffic state for ${currentTrafficState.member} to $newTotalExtraTrafficLimit with next serial ${currentTrafficState.nextSerial}:" val deadline = clock.now.plus(timeout.asJavaApproximation) @@ -509,6 +485,9 @@ class SequencerAdminConnection( ).map(_ => ()), logger, ) + .flatMap(status => + if (status.isOk) Future.unit else Future.failed(status.asRuntimeException()) + ) } def getSequencerPruningStatus()(implicit diff --git a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala index 48b58be7150..83e8741745c 100644 --- a/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala +++ b/apps/syncoperator/src/main/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTrigger.scala @@ -9,20 +9,15 @@ import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveLon import com.digitalasset.canton.logging.pretty.{Pretty, PrettyPrinting} import com.digitalasset.canton.topology.{Member, ParticipantId} import com.digitalasset.canton.tracing.TraceContext -import io.grpc.Status import io.opentelemetry.api.trace.Tracer import org.apache.pekko.stream.Materializer import org.lfdecentralizedtrust.splice.automation.{ PollingParallelTaskExecutionTrigger, - TaskNoop, TaskOutcome, TaskSuccess, TriggerContext, } -import org.lfdecentralizedtrust.splice.environment.{ - SequencerAdminConnection, - SynchronizerNodeService, -} +import org.lfdecentralizedtrust.splice.environment.SynchronizerNodeService import org.lfdecentralizedtrust.splice.environment.TopologyAdminConnection.TopologySnapshot import org.lfdecentralizedtrust.splice.syncoperator.SyncOperatorSynchronizerNode import org.lfdecentralizedtrust.splice.syncoperator.automation.ReconcileDedicatedSequencerTrafficTrigger.Task @@ -53,8 +48,7 @@ import scala.concurrent.{ExecutionContext, Future} * stay stable if that threshold is above half the active sequencers: with a lower one, the * sequencers on each side can reach it and override each other's limits on every poll. Above * half, a split where neither side reaches it leaves the limits where they are until enough of - * them send the same limit. A grant not applied within `trafficBalanceReconciliationDelay` is sent - * again on the next poll. + * them send the same limit. */ class ReconcileDedicatedSequencerTrafficTrigger( override protected val context: TriggerContext, @@ -72,7 +66,7 @@ class ReconcileDedicatedSequencerTrafficTrigger( override protected def retrieveTasks()(implicit tc: TraceContext): Future[Seq[Task]] = for { - connection <- sequencerAdminConnection() + connection <- synchronizerNodeService.sequencerAdminConnection() (states, totals) <- ( // An empty filter lists every member the sequencer knows. connection.listSequencerTrafficControlState(), @@ -86,7 +80,7 @@ class ReconcileDedicatedSequencerTrafficTrigger( override protected def completeTask(task: Task)(implicit tc: TraceContext): Future[TaskOutcome] = for { - connection <- sequencerAdminConnection() + connection <- synchronizerNodeService.sequencerAdminConnection() // Read again, so the grant carries the serial the sequencer expects next. (trafficState, sequencerState) <- ( connection.getSequencerTrafficControlState(task.member), @@ -96,48 +90,18 @@ class ReconcileDedicatedSequencerTrafficTrigger( // the total, or its grant moved the serial and this one is refused and retried. current <- currentTask(task) limit = trafficState.extraTrafficLimit.value - outcome <- - if (current.isDone(limit)) Future.successful(TaskNoop) - else - connection - .attemptSetSequencerTrafficControlState( - trafficState, - sequencerState, - NonNegativeLong.tryCreate(current.target), - context.pollingClock, - trafficBalanceReconciliationDelay, - ) - .flatMap { status => - status.getCode match { - case Status.Code.OK => - Future.successful( - TaskSuccess( - s"Set the traffic limit of ${task.member} from $limit to ${current.target}" - ) - ) - // Where a grant needs more than one sequencer, it is not applied until enough of - // the sequencers' operators send the same limit, so it not being applied in time is - // expected while they disagree. Retrying the task would resend it until the retries - // run out and then log an error, so the next poll sends it again instead. A call to - // the sequencer that times out fails the future rather than returning this status, - // so it is handled as any other failure, as is a grant not applied in time where - // one sequencer is enough. - case Status.Code.DEADLINE_EXCEEDED if sequencerState.mapping.threshold.value > 1 => - logger.info( - s"The grant of a traffic limit of ${current.target} for ${task.member} was " + - s"not applied within $trafficBalanceReconciliationDelay: it needs " + - s"${sequencerState.mapping.threshold.value} of the synchronizer's " + - "sequencers to send the same limit. The next poll sends it again." - ) - Future.successful(TaskNoop) - case _ => Future.failed(status.asRuntimeException()) - } - } - } yield outcome + _ <- connection.setSequencerTrafficControlState( + trafficState, + sequencerState, + NonNegativeLong.tryCreate(current.target), + context.pollingClock, + trafficBalanceReconciliationDelay, + ) + } yield TaskSuccess(s"Set the traffic limit of ${task.member} from $limit to ${current.target}") override protected def isStaleTask(task: Task)(implicit tc: TraceContext): Future[Boolean] = for { - connection <- sequencerAdminConnection() + connection <- synchronizerNodeService.sequencerAdminConnection() state <- connection.lookupSequencerTrafficControlState(task.member) current <- currentTask(task) } yield state.forall(state => current.isDone(state.extraTrafficLimit.value)) @@ -146,29 +110,6 @@ class ReconcileDedicatedSequencerTrafficTrigger( // was retrieved is not taken back. private def currentTask(task: Task)(implicit tc: TraceContext): Future[Task] = store.getTotalPurchasedMemberTraffic(task.member).map(total => task.copy(total = total)) - - // The purchased totals are this operator's synchronizer's, so limits are only set on a sequencer - // that serves it: the one this app started with, or its successor once an upgrade switched over. - private def sequencerAdminConnection()(implicit - tc: TraceContext - ): Future[SequencerAdminConnection] = - for { - connection <- synchronizerNodeService.sequencerAdminConnection() - served <- connection.getStatus.map(_.successOption.map(_.synchronizerId.logical)) - } yield served match { - case Some(synchronizerId) if synchronizerId == store.key.synchronizerId => connection - case Some(synchronizerId) => - throw Status.FAILED_PRECONDITION - .withDescription( - s"The sequencer serves $synchronizerId, not this operator's synchronizer " + - s"${store.key.synchronizerId}" - ) - .asRuntimeException() - case None => - throw Status.FAILED_PRECONDITION - .withDescription("Sequencer is not yet initialized") - .asRuntimeException() - } } object ReconcileDedicatedSequencerTrafficTrigger { diff --git a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala index 45846492c97..9a36c21c7bc 100644 --- a/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala +++ b/apps/syncoperator/src/test/scala/org/lfdecentralizedtrust/splice/syncoperator/automation/ReconcileDedicatedSequencerTrafficTriggerTest.scala @@ -5,22 +5,10 @@ package org.lfdecentralizedtrust.splice.syncoperator.automation import com.daml.metrics.api.noop.NoOpMetricsFactory import com.daml.nonempty.NonEmpty -import com.digitalasset.canton.admin.api.client.data.{ - NodeStatus, - SequencerAdminStatus, - SequencerHealthStatus, - SequencerStatus, - TopologyQueueStatus, -} import com.digitalasset.canton.admin.api.client.data.topology.BaseResult import com.digitalasset.canton.concurrent.FutureSupervisor import com.digitalasset.canton.config.{NonNegativeFiniteDuration, ProcessingTimeout} -import com.digitalasset.canton.config.RequireTypes.{ - NonNegativeInt, - NonNegativeLong, - PositiveInt, - PositiveLong, -} +import com.digitalasset.canton.config.RequireTypes.{NonNegativeLong, PositiveInt, PositiveLong} import com.digitalasset.canton.crypto.{Fingerprint, TestHash} import com.digitalasset.canton.data.CantonTimestamp import com.digitalasset.canton.sequencing.protocol @@ -32,18 +20,14 @@ import com.digitalasset.canton.topology.{ MediatorId, Member, ParticipantId, - PartyId, - PhysicalSynchronizerId, SequencerId, SynchronizerId, UniqueIdentifier, } import com.digitalasset.canton.tracing.TraceContext -import com.digitalasset.canton.version.{ProtocolVersion, ReleaseVersion} import com.digitalasset.canton.{BaseTest, HasActorSystem, HasExecutionContext} import io.grpc.{Status, StatusRuntimeException} import org.lfdecentralizedtrust.splice.automation.{ - TaskNoop, TaskOutcome, TaskSuccess, TriggerContext, @@ -89,7 +73,6 @@ class ReconcileDedicatedSequencerTrafficTriggerTest private val aliceTotal = 100L private val synchronizerId = SynchronizerId.tryFromString("dedicated::default") - private val otherSynchronizerId = SynchronizerId.tryFromString("other::default") "ReconcileDedicatedSequencerTrafficTrigger.tasks" should { @@ -233,74 +216,45 @@ class ReconcileDedicatedSequencerTrafficTriggerTest val target = 150L "set a participant's limit to its target" in { - val f = new Fixture(threshold = 1, limit = 0L) - f.grantReturns(Future.successful(Status.OK)) + val f = new Fixture(limit = 0L) + f.grantReturns(Future.unit) f.trigger.complete(task).futureValue shouldBe a[TaskSuccess] - verify(f.connection).attemptSetSequencerTrafficControlState( - any[TrafficState], - any[TopologyResult[SequencerSynchronizerState]], - eqTo(NonNegativeLong.tryCreate(target)), - any[Clock], - any[NonNegativeFiniteDuration], - )(any[TraceContext]) - } - - "send no grant when the participant's current total already puts it at its target" in { - // Retrieved before a purchase of the whole total, which has been granted since. - val f = new Fixture(threshold = 1, limit = target) - f.trigger.complete(task.copy(total = 0L)).futureValue shouldBe TaskNoop - f.verifyNoGrant() - } - - "leave a grant not applied in time to the next poll where it needs several sequencers" in { - val f = new Fixture(threshold = 2, limit = 0L) - f.grantReturns(Future.successful(Status.DEADLINE_EXCEEDED)) - f.trigger.complete(task).futureValue shouldBe TaskNoop + f.verifyGrant(target) } - "fail a grant not applied in time where one sequencer is enough" in { - val f = new Fixture(threshold = 1, limit = 0L) - f.grantReturns(Future.successful(Status.DEADLINE_EXCEEDED)) - failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED + "set the target from the participant's total as it is now" in { + // Retrieved before a purchase of the whole total was ingested. + val f = new Fixture(limit = 0L) + f.grantReturns(Future.unit) + f.trigger.complete(task.copy(total = 0L)).futureValue shouldBe a[TaskSuccess] + f.verifyGrant(target) } - "fail a grant whose sequencer call times out, even where several sequencers are needed" in { - val f = new Fixture(threshold = 2, limit = 0L) + "fail a grant that fails, so that it is retried" in { + val f = new Fixture(limit = 0L) f.grantReturns(Future.failed(Status.DEADLINE_EXCEEDED.asRuntimeException())) failureCode(f.trigger.complete(task)) shouldBe Status.Code.DEADLINE_EXCEEDED } - "fail a grant a concurrent change aborted, so that it is retried" in { - val f = new Fixture(threshold = 2, limit = 0L) - f.grantReturns(Future.successful(Status.ABORTED)) - failureCode(f.trigger.complete(task)) shouldBe Status.Code.ABORTED - } - - "set no limit on a sequencer that serves another synchronizer" in { - val f = new Fixture(threshold = 1, limit = 0L, served = otherSynchronizerId) - failureCode(f.trigger.complete(task)) shouldBe Status.Code.FAILED_PRECONDITION - f.verifyNoGrant() - } - "treat a task as stale once the participant is at its target" in { - new Fixture(threshold = 1, limit = target).trigger.stale(task).futureValue shouldBe true + new Fixture(limit = target).trigger.stale(task).futureValue shouldBe true } "treat a task as stale once the participant's current total puts it at its target" in { // Retrieved before a purchase of the whole total, which has been granted since. - new Fixture(threshold = 1, limit = target).trigger + new Fixture(limit = target).trigger .stale(task.copy(total = 0L)) .futureValue shouldBe true } "treat a task as stale once the participant has no traffic state" in { - new Fixture(threshold = 1, limit = 0L, hasTrafficState = false).trigger + new Fixture(limit = 0L, hasTrafficState = false).trigger .stale(task) .futureValue shouldBe true } "not treat a task as stale while the participant is not at its target" in { - new Fixture(threshold = 1, limit = 0L).trigger.stale(task).futureValue shouldBe false + new Fixture(limit = 0L).trigger.stale(task).futureValue shouldBe false } } @@ -319,30 +273,14 @@ class ReconcileDedicatedSequencerTrafficTriggerTest private def failureCode(outcome: Future[TaskOutcome]): Status.Code = inside(outcome.failed.futureValue) { case e: StatusRuntimeException => e.getStatus.getCode } - /** The trigger on a mocked sequencer, store and node service, with alice at `limit` on a - * sequencer group of `threshold` sequencers that serves `served`. - */ - private class Fixture( - threshold: Int, - limit: Long, - hasTrafficState: Boolean = true, - served: SynchronizerId = synchronizerId, - ) { + /** The trigger on a mocked sequencer, store and node service, with alice at `limit`. */ + private class Fixture(limit: Long, hasTrafficState: Boolean = true) { val connection: SequencerAdminConnection = mock[SequencerAdminConnection] private val store = mock[SyncOperatorStore] private val nodeService = mock[SynchronizerNodeService[SyncOperatorSynchronizerNode]] when(nodeService.sequencerAdminConnection()(any[TraceContext])) .thenReturn(Future.successful(connection)) - when(connection.getStatus(any[TraceContext])) - .thenReturn(Future.successful(sequencerStatus(served))) - when(store.key).thenReturn( - SyncOperatorStore.Key( - PartyId.tryFromProtoPrimitive("operator::default"), - PartyId.tryFromProtoPrimitive("dso::default"), - synchronizerId, - ) - ) when(store.getTotalPurchasedMemberTraffic(eqTo(alice))(any[TraceContext])) .thenReturn(Future.successful(aliceTotal)) private val trafficState = TrafficState( @@ -362,26 +300,26 @@ class ReconcileDedicatedSequencerTrafficTriggerTest when(connection.lookupSequencerTrafficControlState(eqTo(alice))(any[TraceContext])) .thenReturn(Future.successful(Option.when(hasTrafficState)(trafficState))) when(connection.getSequencerSynchronizerState(any[TopologySnapshot])(any[TraceContext])) - .thenReturn(Future.successful(sequencerState(threshold))) + .thenReturn(Future.successful(sequencerState(threshold = 1))) val trigger = new TestTrigger(store, nodeService) - def grantReturns(status: Future[Status]): Unit = + def grantReturns(result: Future[Unit]): Unit = when( - connection.attemptSetSequencerTrafficControlState( + connection.setSequencerTrafficControlState( any[TrafficState], any[TopologyResult[SequencerSynchronizerState]], any[NonNegativeLong], any[Clock], any[NonNegativeFiniteDuration], )(any[TraceContext]) - ).thenReturn(status) + ).thenReturn(result) - def verifyNoGrant(): Unit = - verify(connection, never).attemptSetSequencerTrafficControlState( + def verifyGrant(target: Long): Unit = + verify(connection).setSequencerTrafficControlState( any[TrafficState], any[TopologyResult[SequencerSynchronizerState]], - any[NonNegativeLong], + eqTo(NonNegativeLong.tryCreate(target)), any[Clock], any[NonNegativeFiniteDuration], )(any[TraceContext]) @@ -401,26 +339,6 @@ class ReconcileDedicatedSequencerTrafficTriggerTest def stale(task: Task): Future[Boolean] = isStaleTask(task) } - private def sequencerStatus(served: SynchronizerId): NodeStatus[SequencerStatus] = - NodeStatus.Success( - SequencerStatus( - uid = UniqueIdentifier.tryCreate("sequencer", "default"), - synchronizerId = - PhysicalSynchronizerId(served, NonNegativeInt.tryCreate(0), ProtocolVersion.latest), - uptime = java.time.Duration.ZERO, - ports = Map.empty, - connectedParticipants = Seq.empty, - connectedMediators = Seq.empty, - sequencer = SequencerHealthStatus(isActive = true), - topologyQueue = TopologyQueueStatus(0, 0, 0), - admin = SequencerAdminStatus(acceptsAdminChanges = true), - components = Seq.empty, - version = ReleaseVersion.current, - protocolVersion = ProtocolVersion.latest, - ) - ) - - // A group of `threshold` sequencers, all of which have to send a grant. private def sequencerState(threshold: Int): TopologyResult[SequencerSynchronizerState] = TopologyResult( BaseResult(