fix: skip unparseable and foreign sync ids in traffic triggers (Scala) - #14
Open
sadiq1971 wants to merge 2 commits into
Open
fix: skip unparseable and foreign sync ids in traffic triggers (Scala)#14sadiq1971 wants to merge 2 commits into
sadiq1971 wants to merge 2 commits into
Conversation
sadiq1971
force-pushed
the
fix/sync-id-parse-hardening
branch
2 times, most recently
from
August 10, 2026 17:20
c49a6da to
55ae2f4
Compare
sadiq1971
added a commit
that referenced
this pull request
Aug 13, 2026
Signed-off-by: sadiq1971 <[email protected]>
sadiq1971
added a commit
that referenced
this pull request
Aug 14, 2026
Signed-off-by: sadiq1971 <[email protected]>
timwu20
approved these changes
Aug 19, 2026
timwu20
left a comment
Collaborator
There was a problem hiding this comment.
One pre-merge mechanical: the branch predates the #8/#12 squashes, so it lacks the Flyway ignore-pattern fix that rode in with #12 — the two red checks are the Aug-13 known flakes (simtime clog gate with sbt green underneath), but any fresh run on this tree will fail ~7 jobs on the Postgres-18.6 pattern. Suggest rebasing onto current feat/dedicated-sync (5186e84) and letting [ci] rerun; expect green, then this is merge-ready. (#15 stacks on this branch, so it rebases in the same pass.)
…ers [ci] Traffic can be purchased for any registered synchronizer, so an SV sequencer observes MemberTraffic contracts for synchronizers it does not serve. Parse failures and foreign synchronizer ids now skip the contract instead of failing the trigger. Signed-off-by: sadiq1971 <[email protected]>
A foreign synchronizer id is routine once dedicated synchronizers exist, but an unparseable one means corrupt data, so it is logged as a warning as well as skipped. Matches the acceptance criterion in canton-extending-mainnet#33. Signed-off-by: sadiq1971 <[email protected]>
sadiq1971
force-pushed
the
fix/sync-id-parse-hardening
branch
from
August 19, 2026 23:00
4d4b9af to
36e365c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two SV traffic triggers treat a
MemberTrafficcontract they cannot act on as a fault. Traffic can be purchased for any registered synchronizer, so an SV's sequencer legitimately observes contracts naming synchronizers it does not serve. Scala side only, no Daml or config changes.What this does
SynchronizerId.tryFromString->fromString+foldin both triggers. The throwing variant fails the trigger and leaves it retrying the same contract; theEitherform skips it with aTaskSuccess, matching howmemberIdis already handled directly above in both files.ReconcileSequencerLimitWithMemberTrafficTriggerno longer raisesStatus.INTERNALwhen the contract names a different synchronizer. Those grants are applied by that synchronizer's own operator on its own sequencer, so this node skips them rather than failing.MergeMemberTrafficContractsTriggerparses ahead of theforcomprehension, dropping agetDsoRules()round trip on ids that are skipped anyway.How it's verified
apps-sv/compile,scalafmtCheck,headerCheckandscalafix --checkall pass locally.No new tests:
apps/svholds 79 triggers and no trigger unit tests, so this follows the existing pattern.Both skip paths land in the Appendix B acceptance e2e (register, buy, then reconcile across two synchronizers), which needs the two-synchronizer topology the foreign path requires anyway. Naming it here so the debt is tracked rather than implicit.
One forward-looking note for whoever writes that test: the case that feeds an unparseable synchronizer id will trip the CI log gate on the
logger.warnadded in 4d4b9af, and needs an ignore-pattern entry alongside the test.Tracked in
Implements E2-3 (ChainSafe/canton-extending-mainnet#33).