Skip to content

Add physical synchronizer id to periodic topology snapshot metadata#5337

Open
thibault-da wants to merge 3 commits intocanton-network:mainfrom
thibault-da:tj-export-synchronizer-id
Open

Add physical synchronizer id to periodic topology snapshot metadata#5337
thibault-da wants to merge 3 commits intocanton-network:mainfrom
thibault-da:tj-export-synchronizer-id

Conversation

@thibault-da
Copy link
Copy Markdown

We need the physical synchronizer Id the snapshot was taken from to correctly validate it in canton CI tests.
This PR adds it to the existing metadata file.
It also writes in a CSV-style format instead of toString the map which resulted in a not so nice to parse HashMap(key -> value) content

Pull Request Checklist

Cluster Testing

  • If a cluster test is required, comment /cluster_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a hard-migration test is required (from the latest release), comment /hdm_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.
  • If a logical synchronizer upgrade test is required (from canton-3.5), comment /lsu_test on this PR to request it, and ping someone with access to the DA-internal system to approve it.

PR Guidelines

  • Include any change that might be observable by our partners or affect their deployment in the release notes.
  • Specify fixed issues with Fixes #n, and mention issues worked on using #n
  • Include a screenshot for frontend-related PRs - see README or use your favorite screenshot tool

Merge Guidelines

  • Make the git commit message look sensible when squash-merging on GitHub (most likely: just copy your PR description).

@isegall-da isegall-da added the static Used to label PRs for which static tests suffice label May 1, 2026
@isegall-da
Copy link
Copy Markdown
Contributor

@thibault-da can you rebase on latest Splice main? We've had to make some changes following the move of Splice to CF. Thanks

@thibault-da thibault-da force-pushed the tj-export-synchronizer-id branch from ada1a77 to b141b12 Compare May 3, 2026 06:21
@thibault-da thibault-da requested a review from a team as a code owner May 3, 2026 06:21
@thibault-da
Copy link
Copy Markdown
Author

@isegall-da done :)

moritzkiefer-da
moritzkiefer-da previously approved these changes May 4, 2026
Copy link
Copy Markdown
Contributor

@moritzkiefer-da moritzkiefer-da left a comment

Choose a reason for hiding this comment

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

Thanks!

metadata = summary.map(e =>
(e._1.code, e._2.toString)
) + ("sequencerId" -> sequencerId.toProtoPrimitive)
metadataMap = summary.map(e => (e._1.code, e._2.toString)) +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: feels like a pretty weird csv file. instead of having headers at the top we instead have each row be a key value pair. How do you feel about making it a json file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure I can make it json

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done :)

moritzkiefer-da
moritzkiefer-da previously approved these changes May 5, 2026
Copy link
Copy Markdown
Contributor

@moritzkiefer-da moritzkiefer-da left a comment

Choose a reason for hiding this comment

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

Nice thanks

@moritzkiefer-da
Copy link
Copy Markdown
Contributor

CI doesn't like your change:

[info] compiling 19 Scala sources to /__w/splice/splice/apps/sv/target/scala-2.13/classes ...
Error:  /__w/splice/splice/apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala:10:66: Unused import
Error:  import com.digitalasset.canton.topology.{PhysicalSynchronizerId, SynchronizerId}
Error:                                                                   ^
Error:  /__w/splice/splice/apps/sv/src/main/scala/org/lfdecentralizedtrust/splice/sv/automation/PeriodicTopologySnapshotTrigger.scala:14:24: Unused import
Error:  import io.circe.syntax.*
Error:                         ^
Error:  No warnings can be incurred under -Werror.
Error:  three errors found
Error:  (apps-sv / Compile / compileIncremental) Compilation failed

@moritzkiefer-da moritzkiefer-da removed the static Used to label PRs for which static tests suffice label May 5, 2026
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.

3 participants