feat: serve the registration for explicit disclosure - #29
Merged
Conversation
Adds GET /v0/domains/{domain_id}/registration, returning the
RegisteredSynchronizer as a ContractWithState so a buying validator can disclose
it on AmuletRules_BuyMemberTraffic. The buyer is neither signatory nor observer,
and update history drops the created event blob by design, so the ACS store is
the only source.
Orders by created_at desc, contract_id: duplicates are representable during an
operator change, a stale pick would credit the superseded operator, and the
tiebreak keeps every Scan in agreement under bftCall.
Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
Reshuffles the wall-clock-time sharding, which is recomputed per run. The previous grouping put SplitwellIntegrationTest under enough co-tenancy to starve its base traffic on the splitwell synchronizer. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
Say what is surprising about the code rather than why the change is right. The reasoning is in the commit messages and the PR. Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
| "404": | ||
| $ref: "../../../../common/src/main/openapi/common-external.yaml#/components/responses/404" | ||
|
|
||
| /v0/domains/{domain_id}/registration: |
There was a problem hiding this comment.
Suggested change
| /v0/domains/{domain_id}/registration: | |
| /v0/synchronizers/{synchronizer_id}/registration: |
Comment on lines
+1134
to
+1135
| synchronizer. Served from the ACS store, so the response carries the created event blob | ||
| a buyer needs in order to disclose it. |
There was a problem hiding this comment.
Suggested change
| synchronizer. Served from the ACS store, so the response carries the created event blob | |
| a buyer needs in order to disclose it. | |
| synchronizer. |
that's not useful info for a user of that API
Collaborator
Author
There was a problem hiding this comment.
Applied your suggestion verbatim
| domainMigrationId, | ||
| RegisteredSynchronizer.COMPANION, | ||
| additionalWhere = sql""" | ||
| and acs.create_arguments->>'synchronizerId' = ${lengthLimited(synchronizerId)} |
There was a problem hiding this comment.
that's gonna do a linear scan, that's no good. we need to add a new column to the acs store and an index for that to make this a better query.
Collaborator
Author
There was a problem hiding this comment.
Done. V075 adds scan_acs_store.registered_synchronizer_id, populated at ingestion, and the query filters on it. Followed V072's split.
| } | ||
| } | ||
|
|
||
| // Both registrations are live during an operator change; the superseded one would |
There was a problem hiding this comment.
I thought we said we don't support operator change?
Collaborator
Author
There was a problem hiding this comment.
Right, oops! Newest-wins and its test are gone.
The query extracted synchronizerId from create_arguments on every candidate row.
V075 adds scan_acs_store.registered_synchronizer_id, populated at ingestion, and
the query filters on it. The column goes in the migration and the concurrent
index in SqlIndexInitializationTrigger, the way V072 split the same operation.
Operator change is not supported, so the newest-wins ordering and its test are
gone. order by contract_id stays: the template has no key and
DsoRules_RegisterSynchronizer creates unconditionally, so duplicates are still
representable, and every Scan has to return the same row for bftCall.
Renames the path to /v0/synchronizers/{synchronizer_id}/registration, which needs
a new rate-limit prefix and the regenerated cluster configs that follow from it.
Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com>
Signed-off-by: Sebastian Lindner <33971232+salindne@users.noreply.github.com> # Conflicts: # apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SyncOperatorTrafficIntegrationTest.scala
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
2 participants
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.
Closes ChainSafe/canton-extending-mainnet#42
Summary: Scan serves the
RegisteredSynchronizeratGET /v0/synchronizers/{synchronizer_id}/registrationas aContractWithState, so a buying validator can disclose it onAmuletRules_BuyMemberTraffic; the ACS store is the only source of the created event blob, since update history drops it by design.Review fixes: the query now filters on a new indexed
scan_acs_store.registered_synchronizer_idinstead of a jsonb extraction. Split the way V072 did, so the column is in the migration and the concurrent index is inSqlIndexInitializationTrigger. Operator change is not supported, so newest-wins is gone;order by contract_idremains because duplicates are still representable and every Scan must return the same row forbftCall.