feat: bootstrap a dedicated synchronizer - #33
Conversation
ee11c3b to
3dd7eca
Compare
The operator keeps traffic control on for the synchronizer it serves, at a zero base traffic amount so that all of its traffic is paid for. Enforcement waits for the DSO registration, since members onboard with charged topology transactions and traffic cannot be bought for an unregistered synchronizer. Its store ingests that registration, and it now waits for Scan instead of failing when Scan is still initializing. On LocalNet the operator runs behind the sync-operator profile, which splice-localnet-compose.sh -O turns on, since it cannot start until the app-synchronizer has been bootstrapped. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
3dd7eca to
4855555
Compare
The two reward-accounting BFT reads already have their 'disagreed with consensus' warning ignored by trigger. They emit 'Consensus not reached.' when no peer converges at all, which was ignored only for named suites, so SvTimeBasedAmuletPriceIntegrationTest failed the simtime shard. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
| parameters.tryUpdate(trafficControlParameters = | ||
| Some( | ||
| parameters.trafficControl | ||
| .getOrElse(TrafficControlParameters()) |
There was a problem hiding this comment.
don't you want all of them to be configurable?
Also are you sure that going from no traffic -> traffic enabled works properly (changing traffic parameters if traffic was already enabled is definitely fine)? If we do want to support this I think we need a lot more testing around this. I'd lean towards making it an error if traffic management was not already enabled.
There was a problem hiding this comment.
I thought it rather an enforcement from the sync operator perspective to have base rate 0 and rest configured from the bootstrap of the synchronizer.
But yeah make sense to add rest.
There was a problem hiding this comment.
And on no traffic -> traffic yes it works - checked loally. Also if traffic is already set it can be overwritten as well
There was a problem hiding this comment.
And also may be in future we will be passing more broader in general config for the seqeuncer like we are doing in sv automation
There was a problem hiding this comment.
rest configured from the bootstrap of the synchronizer.
I think you want the option to change it later relatively easily so it makes sense to have it in the automation not just bootstrap.
And on no traffic -> traffic yes it works - checked loally. Also if traffic is already set it can be overwritten as well
There are two issues
- This code path is not really tested at the moment.
- We don't have a good migration story. Right now you would turn on traffic management, everything fails until nodes are able to top-up. That's a bit of a mess.
We also explicitly said that the migration from phase 1 -> phase 2 which might go no traffic management -> traffic management is out of scope.
So I would turn this around:
- Assume the synchronizer is bootstrapped with traffic management.
- Check this once during startup and if it isn't hard fail. If a user wants to switch they can do so through console commands but more likely this seems to be an accident at this point.
- In the trigger adjust the parameters but never switch from no traffic management -> traffic management.
There was a problem hiding this comment.
Thanks for the clarifications. I have updated this in the new commit -
- Bootstrap enables traffic control with canton's default
- Sync Operator have the configurable traffic params to update on the dsync. If traffic control is not enabled on the dsyn- hard fail.
- Localnet e2e test has been added for dedicated sync.
1cbb4cb to
4855555
Compare
It is unrelated to this PR: the simtime shard flake it silences comes from SvTimeBasedAmuletPriceIntegrationTest, not from anything here. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Look the registration up with findAnyContractWithOffset rather than a one-element listContracts. Make the traffic control parameters configurable, the same four the SV exposes through SynchronizerFeesConfig, defaulting to Canton's own values. The trigger applies those four and leaves the rest as the synchronizer has them. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
…end to end [ci] The app-synchronizer is bootstrapped with traffic control at Canton's defaults, and the sync operator refuses to start against a synchronizer that does not run it. Adds a LocalNet end-to-end test: the DSO registers the synchronizer, the operator narrows the base rate to zero, and a member buys traffic before it can transact on it. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
The compose start returns before the SV serves its DSO info and before the validator has onboarded the operator's ledger API user, so CI failed on a 404 from /v1/dso. Signed-off-by: sadiq1971 <sadiqurr8@gmail.com>
2 participants
Closes ChainSafe/canton-extending-mainnet#98
Summary: The app-synchronizer is bootstrapped with traffic control on, and the sync operator
narrows its base traffic amount to zero once the DSO registers the synchronizer, refusing to start
if traffic control is off. A LocalNet end-to-end test covers registration, the zero base rate, and
a member buying traffic before it can transact.