Skip to content

fix(external-call): serialize logical synchronizer id in prepared metadata - #16

Draft
trusch wants to merge 1 commit into
external-call/06-runtime-integrationfrom
fix/external-call-logical-synchronizer-id
Draft

fix(external-call): serialize logical synchronizer id in prepared metadata#16
trusch wants to merge 1 commit into
external-call/06-runtime-integrationfrom
fix/external-call-logical-synchronizer-id

Conversation

@trusch

@trusch trusch commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Prepared external-call metadata now serializes the logical synchronizer id even when the prepare path carries a physical synchronizer id. This keeps prepared metadata consistent with the execute-side decoder and avoids malformed synchronizer_id INVALID_ARGUMENT during execute.

Root cause

PreparedTransactionEncoder encoded Metadata.synchronizer_id through Synchronizer.toProtoPrimitive. When the prepare path supplied a PhysicalSynchronizerId, this wrote the physical form (logical::protocol-version-serial) into prepared metadata.

PreparedTransactionDecoder reads the same metadata field with SynchronizerId.fromProtoPrimitive, so it accepts the logical form only. That mismatch lets prepare succeed but makes execute reject the signed prepared transaction before it can resolve the logical synchronizer to the connected physical synchronizer.

The BFF should not rewrite this field after signing: the prepared envelope is passed through as signed data, and app-side metadata mutation would change what the external party signed.

Fix

The encoder now writes synchronizer.logical.toProtoPrimitive for prepared metadata. The physical synchronizer remains available where it belongs: execute-side signature verification resolves the logical synchronizer back to the connected physical id through RoutingSynchronizerState.

Tests

Added a focused PreparedTransactionCodecV1Spec regression that encodes a prepared transaction with a PhysicalSynchronizerId and asserts metadata contains the logical id parseable by SynchronizerId.fromProtoPrimitive.

Verification run locally:

  • sbt 'ledger-api-core/Test/scalafmtCheck' passed
  • sbt 'ledger-api-core/Compile/scalafmtCheck' passed
  • sbt 'ledger-api-core/testOnly com.digitalasset.canton.platform.apiserver.services.command.interactive.PreparedTransactionCodecV1Spec' was attempted, but this checkout's sbt-installed Daml compiler failed before tests ran: dpm: option --target: Unknown Daml-LF version: 2.2 while building ledger-common-dars and generated ledger-api-core test Daml packages.

Zenith impact

This targets the current Zenith symptom where M2 propose prepares successfully after removing Daml contract keys, but execute fails with malformed synchronizer_id. With prepared metadata using the logical synchronizer id, BFF can continue passing the signed prepared transaction through verbatim.

Upstreaming notes

This is a narrow codec consistency fix. It does not change external transaction hashing rules or physical synchronizer resolution; it only aligns the serialized prepared metadata with the existing decoder contract.

@greptile-apps

greptile-apps Bot commented May 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a serialization mismatch in PreparedTransactionEncoder where Metadata.synchronizer_id was written using toProtoPrimitive on the raw Synchronizer value. When the prepare path supplied a PhysicalSynchronizerId, this produced the physical form (logical::pv-serial) that SynchronizerId.fromProtoPrimitive on the decode side cannot parse, causing execute to fail with INVALID_ARGUMENT.

  • Encoder fix: synchronizerTransformer now calls .logical.toProtoPrimitive, so synchronizer_id in prepared metadata is always the logical form regardless of whether prepare received a PhysicalSynchronizerId or a SynchronizerId.
  • Regression test: PreparedTransactionCodecV1Spec adds a property test that encodes with an explicit PhysicalSynchronizerId, asserts the serialized field equals psid.logical.toProtoPrimitive, and confirms SynchronizerId.fromProtoPrimitive round-trips correctly.

Confidence Score: 5/5

Safe to merge — the change is a single-line fix that corrects the serialized form of synchronizer_id in prepared metadata to always be the logical ID, which is exactly what the decoder has always expected.

The encoder change is minimal and correct: SynchronizerId.logical returns this, so the transformer is unchanged for logical-only callers; for physical callers it now strips the physical suffix before serializing. The execute-side hash computation uses forExternalTransactionHashing independently and is not affected. The new property test directly exercises the fixed path and verifies both the encoded value and round-trip parseability.

No files require special attention.

Important Files Changed

Filename Overview
community/ledger/ledger-api-core/src/main/scala/com/digitalasset/canton/platform/apiserver/services/command/interactive/codec/PreparedTransactionEncoder.scala Single-line fix: synchronizerTransformer now calls .logical.toProtoPrimitive, ensuring logical form is always written to prepared metadata regardless of physical/logical input type.
community/ledger/ledger-api-core/src/test/scala/com/digitalasset/canton/platform/apiserver/services/command/interactive/PreparedTransactionCodecV1Spec.scala Adds a focused regression test that encodes with a generated PhysicalSynchronizerId and asserts the metadata field equals the logical form and is parseable by SynchronizerId.fromProtoPrimitive.

Sequence Diagram

sequenceDiagram
    participant BFF as BFF / Prepare caller
    participant Encoder as PreparedTransactionEncoder
    participant ExternalParty as External Party
    participant Decoder as PreparedTransactionDecoder
    participant Execute as Execute path

    BFF->>Encoder: "encode(prepareTransactionData{synchronizer: PhysicalSynchronizerId})"
    note over Encoder: synchronizerTransformer:<br/>Before fix: psid.toProtoPrimitive (physical form)<br/>After fix: psid.logical.toProtoPrimitive (logical form)
    Encoder-->>BFF: "PreparedTransaction{metadata{synchronizer_id: logical_form}}"
    BFF->>ExternalParty: Sign prepared transaction
    ExternalParty-->>BFF: Signed prepared transaction
    BFF->>Decoder: decode metadata
    Decoder->>Decoder: SynchronizerId.fromProtoPrimitive(synchronizer_id) accepts logical form
    Decoder-->>Execute: logical SynchronizerId
    Execute->>Execute: resolve logical to physical via RoutingSynchronizerState
    Execute-->>BFF: Execute success
Loading

Reviews (1): Last reviewed commit: "fix(external-call): serialize logical sy..." | Re-trigger Greptile

@trusch
trusch marked this pull request as draft May 23, 2026 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant