Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvutils - Use the ledger configured time for command dedup [KVL-1149] #11239

Merged
merged 2 commits into from
Oct 15, 2021
Merged

kvutils - Use the ledger configured time for command dedup [KVL-1149] #11239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bc290d
Use the ledger configured time for command dedup in normal validation…
nicu-da Oct 13, 2021
56570b2
Remove documentation
nicu-da Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions ledger/ledger-on-memory/src/main/scala/com/daml/ledger/on/memory/InMemoryLedgerWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ object InMemoryLedgerWriter {
val keyValueCommitting = new KeyValueCommitting(
engine,
metrics,
inStaticTimeMode = needStaticTimeModeFor(timeProvider),
)
new Committer(
transformStateReader = transformStateReader(keySerializationStrategy, stateValueCache),
Expand Down Expand Up @@ -136,7 +135,4 @@ object InMemoryLedgerWriter {
}
}

private def needStaticTimeModeFor(timeProvider: TimeProvider): Boolean =
timeProvider != TimeProvider.UTC

}
1 change: 0 additions & 1 deletion ledger/ledger-on-sql/src/main/scala/com/daml/ledger/on/sql/SqlLedgerReaderWriter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ object SqlLedgerReaderWriter {
stateValueCache = stateValueCache,
engine = engine,
metrics = metrics,
inStaticTimeMode = timeProvider != TimeProvider.UTC,
)
committer = new ValidatingCommitter[Index](
() => timeProvider.getCurrentTime,
Expand Down
5 changes: 1 addition & 4 deletions ...kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/KeyValueCommitting.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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")
Expand Down
16 changes: 1 addition & 15 deletions ...om/daml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,10 @@ 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,
override protected val metrics: Metrics,
inStaticTimeMode: Boolean,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also remove/tune the documentation above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, good catch

) extends Committer[DamlTransactionEntrySummary] {

import TransactionCommitter._
Expand Down Expand Up @@ -119,9 +107,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(
Expand Down
3 changes: 1 addition & 2 deletions ...ticipant-state/kvutils/src/main/scala/com/daml/ledger/validator/SubmissionValidator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion ...aml/ledger/participant/state/kvutils/committer/transaction/TransactionCommitterSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,6 @@ class TransactionCommitterSpec
defaultConfig,
mock[Engine],
metrics,
inStaticTimeMode = false,
)

private def newDedupValue(deduplicationTime: Timestamp): DamlStateValue =
Expand Down
2 changes: 1 addition & 1 deletion ...participant/state/kvutils/tools/integritycheck/RawPreExecutingCommitStrategySupport.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand Down