Create separate SynchronizerId type, mirroring PartyId - #2339
Closed
ayushsingh82 wants to merge 2 commits into
Closed
Create separate SynchronizerId type, mirroring PartyId#2339ayushsingh82 wants to merge 2 commits into
ayushsingh82 wants to merge 2 commits into
Conversation
synchronizerId was passed around as a bare string in several places even where a sibling partyId parameter already used the PartyId type from core-types. Added a SynchronizerId zod schema/type following the same pattern as PartyId, and applied it at the sites that already sit next to a PartyId-typed value: SDKContext.defaultSynchronizerId, CreatePartyOptions, SignedPartyCreationService.allocate(), and MergeDelegationNamespace.setup(). Left the ~200 other bare `synchronizerId: string` occurrences as-is -- most are auto-generated OpenAPI/AsyncAPI/protobuf client code that would be overwritten by codegen anyway, consistent with how PartyId itself isn't applied everywhere either. Fixes canton-network#522 Signed-off-by: ayushsingh82 <ayushsinghmi711@gmail.com>
Contributor
Author
|
@alexmatson-da whenever you have a chance, would appreciate a look at this one. |
mjuchli-da
requested changes
Aug 21, 2026
mjuchli-da
left a comment
Contributor
There was a problem hiding this comment.
Minor correction needed
|
|
||
| export type PartyId = z.infer<typeof PartyId> | ||
|
|
||
| export const SYNCHRONIZER_ID_EXAMPLE = 'sync::122012312312312312123' |
Contributor
There was a problem hiding this comment.
Example should not be part of the types library and only be used in tests
Contributor
Author
There was a problem hiding this comment.
@mjuchli-da Done — moved SYNCHRONIZER_ID_EXAMPLE out of the types library and into index.test.ts as a local test-only constant. Pushed in efd2d19.
Address review feedback: the example constant shouldn't be exported from core/types since it's only used in tests. Signed-off-by: ayushsingh82 <ayushsinghmi711@gmail.com>
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.
Summary
synchronizerIdis passed around as a barestringin several places even where a siblingpartyIdparameter already uses thePartyIdtype fromcore-types.SynchronizerIdzod schema/type incore/types/src/index.ts, following the exact same pattern asPartyId(schema +z.infertype + example constant).PartyId-typed value:SDKContext.defaultSynchronizerId(sdk/wallet-sdk/src/wallet/init/types/context.ts)CreatePartyOptions.synchronizerId(sdk/wallet-sdk/src/wallet/namespace/party/external/types.ts)SignedPartyCreationService.allocate()'ssynchronizerIdparam (.../party/external/signed.ts, which already importsPartyId)MergeDelegationNamespace.setup()'ssynchronizerIdparam (.../token/utxos/mergeDelegation.ts, same file already importsPartyId)I deliberately left the ~200 other bare
synchronizerId: stringoccurrences elsewhere in the repo as-is — the large majority are in auto-generated OpenAPI/AsyncAPI/protobuf client code (core/ledger-client-types/src/generated-clients/**,core/ledger-proto/src/_proto/**) that would just get overwritten on the next codegen run. This mirrors howPartyIditself isn't retrofitted everywhere either (grep -rn "partyId: string"still finds ~27 files outside the generated clients).Note: like
PartyId,SynchronizerIdisn't a branded type (z.inferof az.string()schema resolves structurally tostring), so this is a type-level-only, zero-runtime-behavior change everywhere it's applied.Fixes #522
Test plan
SynchronizerIdschema (valid id, missing::separator, too-short) incore/types/src/index.test.ts— 14/14 passing in that suite.npx vitest run --project node src/wallet/namespace/party/party.test.ts(wallet-sdk) — 18/18 passing.eslintandprettier --checkclean on all 6 changed files.tsc --noEmitclean on every touched file (pre-existing unrelated module-resolution errors elsewhere inwallet-sdkcome from other packages needing a full monorepo build, not from this change).