build/devenv: committee verifier starting up connecting to canton #617
Conversation
0xAustinWang
left a comment
0xAustinWang
left a comment
There was a problem hiding this comment.
Is there a scenario where the hydrated input would change post startup?
| Authority: authority, | ||
| }, | ||
| } | ||
| found = true |
There was a problem hiding this comment.
instead of using var found we can just return early if found, everything from line 233 will be an error scenario
There was a problem hiding this comment.
In theory there might be multiple Canton chains in the outputs slice - if there was only one, then I believe the return would be fine.
| // Get the full party ID (name + hex id) from the canton participant. | ||
| // TODO: how to support multiple participants? | ||
| grpcURL := output.NetworkSpecificData.CantonEndpoints.Participants[0].GRPCLedgerAPIURL | ||
| jwt := output.NetworkSpecificData.CantonEndpoints.Participants[0].JWT |
There was a problem hiding this comment.
it feels like a lot of the jwt / grpc implementation can be put into the startup of a canton accessor or canton source reader, separate from devenv.
It feels useful to have something like this in prod to protect against misconfigs
There was a problem hiding this comment.
The Canton source reader needs the URL and the JWT to connect to the Canton node - those need to be provided by config, since there's no other way to discover it at the moment.
| } | ||
|
|
||
| authority := grpcURL | ||
| if idx := strings.LastIndex(authority, ":"); idx != -1 { |
There was a problem hiding this comment.
Not sure how canton works, what is the authority used for and how is it different from the rpc url directly?
There was a problem hiding this comment.
This authority thing is to tell Nginx (which is proxying the Canton gRPC API) which server we're trying to access. The flow is source reader -> nginx -> canton gRPC.
Good question, this should not happen, unless the whole env is destroyed and recreated, in which case its another |
|
Code coverage report:
|
|
|
||
| // DisableFinalityCheckers is a list of chain selectors for which the finality violation checker should be disabled. | ||
| // The chain selectors are formatted as strings of the chain selector. | ||
| DisableFinalityCheckers []string `toml:"disable_finality_checkers"` |
There was a problem hiding this comment.
Isn't that already defined in GeneratedConfig.DisableFinalityCheckers?
There was a problem hiding this comment.
Good note, maybe this needs to be part of the GeneratedConfig somehow
| } | ||
|
|
||
| // hydrateCantonConfig hydrates the canton config with the full party ID for the CCIPOwnerParty. | ||
| func hydrateCantonConfig(in *VerifierInput, outputs []*blockchain.Output) error { |
There was a problem hiding this comment.
Food for thought, but I wonder if it's not the right time to split services/commiteVerifier.go into something like this
- commiteeVerifier (directory instead of single file )
|---- evm.go
|---- canton.go
There was a problem hiding this comment.
I 100% agree with this, I think we should make packages for each service within services. I can do it in a follow up.
3 participants
This PR adds some changes to get the committee verifier working when reading from Canton. The general approach here is that the env.toml adds some information that is then used to build the entire configuration, after the Canton chains are started.
This is currently necessary because there's no way for us to know ahead of time what the Canton Party ID (akin to a public key) will be that we can use in the Canton source reader.
CantonConfigsandDisableFinalityCheckerstoVerifierInput. These are pulled into the committee verifier config if they're non-nil/non-empty.hydrateCantonConfig.