From 4bc290ddb9acde39d5e75035718d80631cae261b Mon Sep 17 00:00:00 2001 From: Nicu Reut Date: Wed, 13 Oct 2021 17:05:06 +0200 Subject: [PATCH 1/2] Use the ledger configured time for command dedup in normal validation mode instead of always using wall-clock CHANGELOG_BEGIN kvutils - Command deduplication uses the configured time model instead of always using wall clock CHANGELOG_END --- .../com/daml/ledger/on/memory/InMemoryLedgerWriter.scala | 4 ---- .../scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala | 1 - .../participant/state/kvutils/KeyValueCommitting.scala | 5 +---- .../kvutils/committer/transaction/TransactionCommitter.scala | 5 +---- .../com/daml/ledger/validator/SubmissionValidator.scala | 3 +-- .../committer/transaction/TransactionCommitterSpec.scala | 1 - .../RawPreExecutingCommitStrategySupport.scala | 2 +- 7 files changed, 4 insertions(+), 17 deletions(-) diff --git a/ledger/ledger-on-memory/src/main/scala/com/daml/ledger/on/memory/InMemoryLedgerWriter.scala b/ledger/ledger-on-memory/src/main/scala/com/daml/ledger/on/memory/InMemoryLedgerWriter.scala index 68c5abd964bd..104918b134fe 100644 --- a/ledger/ledger-on-memory/src/main/scala/com/daml/ledger/on/memory/InMemoryLedgerWriter.scala +++ b/ledger/ledger-on-memory/src/main/scala/com/daml/ledger/on/memory/InMemoryLedgerWriter.scala @@ -108,7 +108,6 @@ object InMemoryLedgerWriter { val keyValueCommitting = new KeyValueCommitting( engine, metrics, - inStaticTimeMode = needStaticTimeModeFor(timeProvider), ) new Committer( transformStateReader = transformStateReader(keySerializationStrategy, stateValueCache), @@ -136,7 +135,4 @@ object InMemoryLedgerWriter { } } - private def needStaticTimeModeFor(timeProvider: TimeProvider): Boolean = - timeProvider != TimeProvider.UTC - } diff --git a/ledger/ledger-on-sql/src/main/scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala b/ledger/ledger-on-sql/src/main/scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala index 9e64936f48b2..6d1601bdde7c 100644 --- a/ledger/ledger-on-sql/src/main/scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala +++ b/ledger/ledger-on-sql/src/main/scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala @@ -117,7 +117,6 @@ object SqlLedgerReaderWriter { stateValueCache = stateValueCache, engine = engine, metrics = metrics, - inStaticTimeMode = timeProvider != TimeProvider.UTC, ) committer = new ValidatingCommitter[Index]( () => timeProvider.getCurrentTime, diff --git a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueCommitting.scala b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueCommitting.scala index 8536484006e0..171606a004c8 100644 --- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueCommitting.scala +++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueCommitting.scala @@ -42,14 +42,11 @@ import scala.jdk.CollectionConverters._ class KeyValueCommitting private[daml] ( engine: Engine, metrics: Metrics, - inStaticTimeMode: Boolean, ) { import KeyValueCommitting.submissionOutputs private val logger = ContextualizedLogger.get(getClass) - def this(engine: Engine, metrics: Metrics) = this(engine, metrics, false) - /** Processes a Daml submission, given the allocated log entry id, the submission and its resolved inputs. * Produces the log entry to be committed, and Daml state updates. * @@ -145,7 +142,7 @@ class KeyValueCommitting private[daml] ( new ConfigCommitter(defaultConfig, maximumRecordTime, metrics) case DamlSubmission.PayloadCase.TRANSACTION_ENTRY => - new TransactionCommitter(defaultConfig, engine, metrics, inStaticTimeMode) + new TransactionCommitter(defaultConfig, engine, metrics) case DamlSubmission.PayloadCase.PAYLOAD_NOT_SET => throw Err.InvalidSubmission("DamlSubmission payload not set") diff --git a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala index d07424c4325a..8870cf472619 100644 --- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala +++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala @@ -56,7 +56,6 @@ private[kvutils] class TransactionCommitter( defaultConfig: Configuration, engine: Engine, override protected val metrics: Metrics, - inStaticTimeMode: Boolean, ) extends Committer[DamlTransactionEntrySummary] { import TransactionCommitter._ @@ -119,9 +118,7 @@ private[kvutils] class TransactionCommitter( .map { recordTime => val dedupKey = commandDedupKey(transactionEntry.submitterInfo) val dedupEntry = commitContext.get(dedupKey) - val submissionTime = - if (inStaticTimeMode) Instant.now() else recordTime.toInstant - if (dedupEntry.forall(isAfterDeduplicationTime(submissionTime, _))) { + if (dedupEntry.forall(isAfterDeduplicationTime(recordTime.toInstant, _))) { StepContinue(transactionEntry) } else { rejections.reject( diff --git a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/validator/SubmissionValidator.scala b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/validator/SubmissionValidator.scala index 044d092dfcd3..d2de155e411e 100644 --- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/validator/SubmissionValidator.scala +++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/validator/SubmissionValidator.scala @@ -332,11 +332,10 @@ object SubmissionValidator { stateValueCache: StateValueCache, engine: Engine, metrics: Metrics, - inStaticTimeMode: Boolean, ): SubmissionValidator[LogResult] = new SubmissionValidator( ledgerStateAccess, - processSubmission(new KeyValueCommitting(engine, metrics, inStaticTimeMode)), + processSubmission(new KeyValueCommitting(engine, metrics)), logEntryIdAllocator, checkForMissingInputs, stateValueCache, diff --git a/ledger/participant-state/kvutils/src/test/suite/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitterSpec.scala b/ledger/participant-state/kvutils/src/test/suite/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitterSpec.scala index f9bdb1d01299..e15ca54f07e3 100644 --- a/ledger/participant-state/kvutils/src/test/suite/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitterSpec.scala +++ b/ledger/participant-state/kvutils/src/test/suite/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitterSpec.scala @@ -409,7 +409,6 @@ class TransactionCommitterSpec defaultConfig, mock[Engine], metrics, - inStaticTimeMode = false, ) private def newDedupValue(deduplicationTime: Timestamp): DamlStateValue = diff --git a/ledger/participant-state/kvutils/tools/integrity-check/src/main/scala/ledger/participant/state/kvutils/tools/integritycheck/RawPreExecutingCommitStrategySupport.scala b/ledger/participant-state/kvutils/tools/integrity-check/src/main/scala/ledger/participant/state/kvutils/tools/integritycheck/RawPreExecutingCommitStrategySupport.scala index 3d62889b08b0..c52c9a1a9c74 100644 --- a/ledger/participant-state/kvutils/tools/integrity-check/src/main/scala/ledger/participant/state/kvutils/tools/integritycheck/RawPreExecutingCommitStrategySupport.scala +++ b/ledger/participant-state/kvutils/tools/integrity-check/src/main/scala/ledger/participant/state/kvutils/tools/integritycheck/RawPreExecutingCommitStrategySupport.scala @@ -59,7 +59,7 @@ final class RawPreExecutingCommitStrategySupport( ]( transformStateReader = SerializingStateReader(stateKeySerializationStrategy), validator = new PreExecutingSubmissionValidator( - new KeyValueCommitting(new Engine(), metrics, inStaticTimeMode = true), + new KeyValueCommitting(new Engine(), metrics), new RawPreExecutingCommitStrategy(stateKeySerializationStrategy), metrics = metrics, ), From 56570b2cda8b14a374edf1ef23f61bc58f86bfea Mon Sep 17 00:00:00 2001 From: Nicu Reut Date: Thu, 14 Oct 2021 17:06:19 +0200 Subject: [PATCH 2/2] Remove documentation --- .../committer/transaction/TransactionCommitter.scala | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala index 8870cf472619..310f1bcbe708 100644 --- a/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala +++ b/ledger/participant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala @@ -41,17 +41,6 @@ import com.google.protobuf.{Timestamp => ProtoTimestamp} import scala.annotation.tailrec import scala.jdk.CollectionConverters._ -// The parameter inStaticTimeMode indicates that the ledger is running in static time mode. -// -// Command deduplication is always based on wall clock time and not ledger time. In static time mode, -// record time cannot be used for command deduplication. This flag indicates that the system clock should -// be used as submission time for commands instead of record time. -// -// Other possible solutions that we discarded: -// * Pass in an additional time provider, but this hides the intent -// * Adding and additional submission field commandDedupSubmissionTime field. While having participants -// provide this field *could* lead to possible exploits, they are not exploits that could do any harm. -// The bigger concern is adding a public API for the specific use case of Sandbox with static time. private[kvutils] class TransactionCommitter( defaultConfig: Configuration, engine: Engine,