-
Notifications
You must be signed in to change notification settings - Fork 49
feat(external-call): engine execution, model conformance and validation SPI #552
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.
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
Changes from all commits
1bda6b7
9bf99ad
0cd6f7d
c6a3b1f
1226217
f3e4399
542d8d5
301e1f2
04ecba9
6a97e1c
d074db0
a9fe0a7
42bfe55
8d61347
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright (c) 2026 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package com.digitalasset.canton.participant.protocol.validation | ||
|
|
||
| import com.digitalasset.base.error.{Alarm, AlarmErrorCode, Explanation, Resolution} | ||
| import com.digitalasset.canton.error.CantonErrorGroups.ParticipantErrorGroup.TransactionErrorGroup.LocalRejectionGroup | ||
|
|
||
| object ExternalCallValidationError extends LocalRejectionGroup { | ||
|
|
||
| @Explanation( | ||
| """The participant observed external call results that record different outputs for the same | ||
| |external call. | ||
| |""" | ||
| ) | ||
| @Resolution( | ||
| "Inspect the submitting participant and the external-call service deployments/configuration for inconsistent or non-deterministic results for the same external-call identity." | ||
| ) | ||
| object ExternalCallResultDisagreementAlarm | ||
| extends AlarmErrorCode("EXTERNAL_CALL_RESULT_DISAGREEMENT_ALARM") { | ||
| final case class Warn(override val cause: String) extends Alarm(cause) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| // Copyright (c) 2026 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package com.digitalasset.canton.participant.protocol.validation | ||
|
|
||
| import com.digitalasset.canton.lifecycle.FutureUnlessShutdown | ||
| import com.digitalasset.canton.participant.util.DAMLe | ||
| import com.digitalasset.canton.tracing.TraceContext | ||
| import com.digitalasset.daml.lf.data.Bytes | ||
|
|
||
| trait ExternalCallValidator { | ||
| def validate( | ||
| key: DAMLe.ExternalCallKey, | ||
| recordedOutput: Bytes, | ||
| )(implicit traceContext: TraceContext): FutureUnlessShutdown[ExternalCallValidator.Result] | ||
| } | ||
|
|
||
| object ExternalCallValidator { | ||
| sealed trait Result extends Product with Serializable | ||
| case object Matched extends Result | ||
| final case class Mismatched(computedOutput: Bytes, recordedOutput: Bytes) extends Result | ||
| final case class UnableToValidate(reason: String) extends Result | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,11 @@ import com.digitalasset.canton.logging.{NamedLoggerFactory, NamedLogging} | |
| import com.digitalasset.canton.participant.protocol.EngineController.GetEngineAbortStatus | ||
| import com.digitalasset.canton.participant.store.ReplayContractLookup | ||
| import com.digitalasset.canton.participant.util.DAMLe.* | ||
| import com.digitalasset.canton.participant.util.ExternalCallPayloadDescription.{ | ||
| byteCount, | ||
| byteSize, | ||
| hexPayloadSize, | ||
| } | ||
| import com.digitalasset.canton.protocol.* | ||
| import com.digitalasset.canton.topology.ParticipantId | ||
| import com.digitalasset.canton.topology.client.TopologySnapshot | ||
|
|
@@ -23,12 +28,16 @@ import com.digitalasset.canton.util.PackageConsumer.PackageResolver | |
| import com.digitalasset.canton.util.Thereafter.syntax.* | ||
| import com.digitalasset.canton.{LfCommand, LfPackageId, LfPartyId} | ||
| import com.digitalasset.daml.lf.data.Ref.{PackageId, PackageName} | ||
| import com.digitalasset.daml.lf.data.{ImmArray, Ref} | ||
| import com.digitalasset.daml.lf.data.{Bytes as LfBytes, ImmArray, Ref} | ||
| import com.digitalasset.daml.lf.engine.ResultNeedContract.Response | ||
| import com.digitalasset.daml.lf.engine.{Enricher as _, *} | ||
| import com.digitalasset.daml.lf.interpretation.InterpretationConfig | ||
| import com.digitalasset.daml.lf.language.{Ast, LanguageVersion} | ||
| import com.digitalasset.daml.lf.transaction.{FatContractInstance, NeedKeyProgression} | ||
| import com.digitalasset.daml.lf.transaction.{ | ||
| ExternalCallResult, | ||
| FatContractInstance, | ||
| NeedKeyProgression, | ||
| } | ||
| import com.digitalasset.daml.lf.value.ContractIdVersion | ||
|
|
||
| import java.nio.file.Path | ||
|
|
@@ -120,6 +129,96 @@ object DAMLe { | |
| override protected def pretty: Pretty[EnrichmentError] = adHocPrettyInstance | ||
| } | ||
|
|
||
| final case class ExternalCallRecordedResultDisagreement( | ||
| key: ExternalCallKey, | ||
| outputs: Set[LfBytes], | ||
| ) extends ReinterpretationError { | ||
| override protected def pretty: Pretty[ExternalCallRecordedResultDisagreement] = prettyOfClass( | ||
| param("key", _.key), | ||
| param("recorded output count", _.outputs.size), | ||
| param( | ||
| "recorded output bytes", | ||
| disagreement => | ||
| disagreement.outputs.toSeq | ||
| .sortBy(byteCount) | ||
| .map(byteSize) | ||
| .mkString("[", ", ", "]") | ||
| .doubleQuoted, | ||
| ), | ||
| ) | ||
| } | ||
|
|
||
| final case class ExternalCallReplayMissing( | ||
| key: ExternalCallKey | ||
| ) extends ReinterpretationError { | ||
| override protected def pretty: Pretty[ExternalCallReplayMissing] = prettyOfClass( | ||
| param("key", _.key) | ||
| ) | ||
| } | ||
|
|
||
| /** Deterministic external-call identity. Config and input are engine-emitted canonical hex | ||
| * strings. The pretty-printed form deliberately shows only payload sizes, never the payloads. | ||
| */ | ||
| final case class ExternalCallKey( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an optional design improvement for code cleanliness and is not required for approval. Feel free to track this as a post-merge refactoring chore. From an architectural standpoint, it looks like ExternalCallKey was introduced primarily to avoid typing out Map[(String, String, String, String), Set[LfBytes]] inside ExternalCallReplayData. If that's the case, we could improve encapsulation in one of two ways:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The key stays public because the consistency checker and response router later in the series (#554/#555) group and order by it, which rules out option 1; option 2 would change the engine's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move this class to a separate file in com.digitalasset.canton.participant.protocol.validation. Happy to track that as post-merge cleanup task. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #565 |
||
| extensionId: String, | ||
| functionId: String, | ||
| config: String, | ||
| input: String, | ||
| ) extends PrettyPrinting { | ||
| override protected def pretty: Pretty[ExternalCallKey] = prettyOfClass( | ||
| param("extension id", _.extensionId.doubleQuoted), | ||
| param("function id", _.functionId.doubleQuoted), | ||
| param("config bytes", key => hexPayloadSize(key.config).doubleQuoted), | ||
| param("input bytes", key => hexPayloadSize(key.input).doubleQuoted), | ||
| ) | ||
| } | ||
|
|
||
| object ExternalCallKey { | ||
|
|
||
| /** Orders by the semantic identity fields, lexicographically. */ | ||
| implicit val externalCallKeyOrdering: Ordering[ExternalCallKey] = | ||
| Ordering.by(key => (key.extensionId, key.functionId, key.config, key.input)) | ||
|
|
||
| def fromResult(result: ExternalCallResult): ExternalCallKey = | ||
| ExternalCallKey( | ||
| result.extensionId, | ||
| result.functionId, | ||
| result.config.toHexString, | ||
| result.input.toHexString, | ||
| ) | ||
| } | ||
|
|
||
| /** External-call replay data: recorded outputs indexed by semantic key. Multiple outputs for one | ||
| * semantic key are preserved so replay can report a recorded-result disagreement. | ||
| */ | ||
| final case class ExternalCallReplayData private ( | ||
| outputsByKey: Map[ExternalCallKey, Set[LfBytes]] | ||
| ) { | ||
| def size: Int = outputsByKey.size | ||
|
|
||
| def outputFor( | ||
| key: ExternalCallKey | ||
| ): Either[ExternalCallRecordedResultDisagreement, Option[LfBytes]] = | ||
| outputsByKey.get(key) match { | ||
| case None => Right(None) | ||
| case Some(outputs) if outputs.sizeCompare(1) == 0 => Right(outputs.headOption) | ||
| case Some(outputs) => Left(ExternalCallRecordedResultDisagreement(key, outputs)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a very important validation step, but from my point of view the implementation is unnecessarily complex, as it is spread over ViewParticipantData, ModelConformanceChecker and DAMLe.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The question never posed itself, because this code was written against the full feature rather than this slice — and in the full series the validation you're pointing at deliberately lives elsewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, thanks for the explanation. We can defer the discussion until we reach the reviews for #554 and #555. Please note that I am uneasy with this approach; relying on logic in future unmerged PRs to justify current complexity is highly prone to causing architectural friction and unexpected delays, especially under such a tight timeline.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks again for the detailed breakdown, @angelol. Having now looked ahead and reviewed the entire trajectory of this design across PRs 5–8, I am still strongly in favor of centralizing this in TransactionView rather than scattering it across ModelConformanceChecker and DAMLe. However, to avoid deadlocking your current review train, I am willing to defer this structural change to an explicit, post-merge architectural cleanup task. For the record, and to frame that upcoming cleanup, here are the core technical reasons why the current layout introduces structural regressions:
Regarding the specific downstream trade-offs you mentioned:
Note that the protocol will ultimately reject the whole tree (recte: whole view) anyway, even with the downstream machinery introduced in PRs 5–8.
I don't think the per-party confirmation semantics provide any practical advantage in this scenario. Every hosted confirming party can observe the disagreement; therefore, it is entirely sufficient to reject on behalf of every hosted confirming party rather than building a complex routing mechanism to isolate them. Let's proceed with this slice as-is for now so you can move forward, on the strict condition that we open a tracked technical debt issue to refactor this validation logic back to a centralized, fail-fast model post-milestone. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #565 |
||
| } | ||
| } | ||
|
|
||
| object ExternalCallReplayData { | ||
| val empty: ExternalCallReplayData = ExternalCallReplayData(Map.empty) | ||
|
|
||
| def fromResults(results: Iterable[ExternalCallResult]): ExternalCallReplayData = | ||
| ExternalCallReplayData( | ||
| results | ||
| .groupMap(ExternalCallKey.fromResult)(_.output) | ||
| .view | ||
| .mapValues(_.toSet) | ||
| .toMap | ||
| ) | ||
| } | ||
|
|
||
| trait HasReinterpret { | ||
| def reinterpret( | ||
| contracts: ReplayContractLookup, | ||
|
|
@@ -133,6 +232,7 @@ object DAMLe { | |
| packageResolution: Map[Ref.PackageName, Ref.PackageId], | ||
| expectFailure: Boolean, | ||
| getEngineAbortStatus: GetEngineAbortStatus, | ||
| externalCallReplayData: () => ExternalCallReplayData, | ||
| )(implicit traceContext: TraceContext): EitherT[ | ||
| FutureUnlessShutdown, | ||
| ReinterpretationError, | ||
|
|
@@ -221,6 +321,7 @@ class DAMLe( | |
| packageResolution: Map[PackageName, PackageId], | ||
| expectFailure: Boolean, | ||
| getEngineAbortStatus: GetEngineAbortStatus, | ||
| externalCallReplayData: () => ExternalCallReplayData, | ||
| )(implicit traceContext: TraceContext): EitherT[ | ||
| FutureUnlessShutdown, | ||
| ReinterpretationError, | ||
|
|
@@ -291,6 +392,7 @@ class DAMLe( | |
| contractAuthenticator, | ||
| result, | ||
| getEngineAbortStatus, | ||
| externalCallReplayData, | ||
| ) | ||
| ) | ||
| (tx, metadata) = txWithMetadata | ||
|
|
@@ -323,9 +425,27 @@ class DAMLe( | |
| contractAuthenticator: ContractAuthenticatorFn, | ||
| result: Result[A], | ||
| getEngineAbortStatus: GetEngineAbortStatus, | ||
| externalCallReplayData: () => ExternalCallReplayData, | ||
| )(implicit | ||
| traceContext: TraceContext | ||
| ): FutureUnlessShutdown[Either[ReinterpretationError, A]] = { | ||
| def handleExternalCall( | ||
| externalCallKey: ExternalCallKey, | ||
| resume: Either[ResultNeedExternalCall.Error, String] => Result[A], | ||
| ): FutureUnlessShutdown[Either[ReinterpretationError, A]] = | ||
| externalCallReplayData().outputFor(externalCallKey) match { | ||
| case Left(disagreement) => | ||
| FutureUnlessShutdown.pure(Left(disagreement)) | ||
|
|
||
| case Right(None) => | ||
| FutureUnlessShutdown.pure(Left(ExternalCallReplayMissing(externalCallKey))) | ||
|
|
||
| case Right(Some(storedOutput)) => | ||
| logger.debug( | ||
| s"Replaying recorded external call result for extension=${externalCallKey.extensionId}, function=${externalCallKey.functionId}" | ||
| ) | ||
| handleResultInternal(resume(Right(storedOutput.toHexString))) | ||
| } | ||
|
|
||
| def handleResultInternal( | ||
| result: Result[A] | ||
|
|
@@ -407,7 +527,9 @@ class DAMLe( | |
| case Left(_) => | ||
| Response.UnsupportedContractIdVersion | ||
| } | ||
| FutureUnlessShutdown.pure(response).flatMap(r => handleResultInternal(resume(r))) | ||
| FutureUnlessShutdown | ||
| .pure(response) | ||
| .flatMap(r => handleResultInternal(resume(r))) | ||
|
|
||
| case ResultError(err) => FutureUnlessShutdown.pure(Left(EngineError(err))) | ||
| case ResultInterruption(continue, _) => | ||
|
|
@@ -420,22 +542,14 @@ class DAMLe( | |
| case ResultPrefetch(_, _, resume) => | ||
| // we do not need to prefetch here as Canton includes the keys as a static map in Phase 3 | ||
| handleResultInternal(resume()) | ||
| // TODO(https://github.com/digital-asset/canton/issues/513): Replay or validate recorded external-call results during reinterpretation. | ||
| case ResultNeedExternalCall(_, _, _, _, _) => | ||
| FutureUnlessShutdown.pure( | ||
| Left( | ||
| EngineError( | ||
| Error.Interpretation( | ||
| Error.Interpretation.Internal( | ||
| "reinterpretation", | ||
| "External calls are not supported during reinterpretation", | ||
| None, | ||
| ), | ||
| None, | ||
| ) | ||
| ) | ||
| ) | ||
| case ResultNeedExternalCall(extensionId, functionId, configHash, input, resume) => | ||
| val externalCallKey = ExternalCallKey( | ||
| extensionId, | ||
| functionId, | ||
| configHash, | ||
| input, | ||
| ) | ||
| handleExternalCall(externalCallKey, resume) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // Copyright (c) 2026 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package com.digitalasset.canton.participant.util | ||
|
|
||
| import com.digitalasset.daml.lf.data.Bytes | ||
|
|
||
| /** Size-only descriptions for external-call payload diagnostics. | ||
| * | ||
| * External-call payloads may contain application data, so diagnostics must describe payload sizes | ||
| * without rendering the payload bytes themselves. | ||
| */ | ||
| private[canton] object ExternalCallPayloadDescription { | ||
|
|
||
| def byteCount(bytes: Bytes): Int = bytes.toByteString.size() | ||
|
|
||
| def byteSize(bytes: Bytes): String = | ||
| s"${byteCount(bytes)} bytes" | ||
|
|
||
| def hexPayloadSize(hex: String): String = | ||
| Bytes | ||
| .fromString(hex) | ||
| .fold(_ => s"${hex.length} input characters", byteSize) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason of adding this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the SPI for validating a recorded external-call result against the extension service — same situation as the handler SPI in #551: the implementation arrives with the extension service (#553) and the consumers with the consistency checker and response router (#554/#555). It rides in this PR with the other external-call validation types.