Signature aggregator binary #2829
Conversation
sukantoraymond
requested review from
learyce,
Tonix517,
jasonatran,
artemanosov,
meaghanfitzgerald and
cwend-ava
sdk/blockchain/blockchain.go
Outdated
| killExistingProcess := func() error { | ||
| // Try pkill first | ||
| cmd := exec.Command("pkill", "-f", "signature-aggregator") | ||
| if err := cmd.Run(); err == nil { | ||
| return nil | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
do we want to reuse signature aggregator instances, or do we kill and restart each time? @felipemadero
There was a problem hiding this comment.
we want to reuse instances. there is bootstrapping time spent connecting to pchain which we would like to happens once.
| @@ -215,17 +215,17 @@ var _ = ginkgo.Describe("[Validator Manager POA Set Up]", ginkgo.Ordered, func() | |||
| BootstrapValidators: avaGoBootstrapValidators, | |||
| } | |||
|
|
|||
| ctx, cancel := utils.GetSignatureAggregatorContext() | |||
| _, cancel := utils.GetSignatureAggregatorContext() | |||
There was a problem hiding this comment.
why do you call this if unused?
sdk/blockchain/blockchain.go
Outdated
| } | ||
|
|
||
| binPath := filepath.Join(signatureAggregatorBinDir, "signature-aggregator-v0.4.3", "signature-aggregator") | ||
| if _, err := interchain.StartSignatureAggregator(binPath, configPath, aggregatorLogger); err != nil { |
There was a problem hiding this comment.
what happen if a second L1 is deployed? does this aggregator dies? we need to be sure that:
- add validator, and such, also uses this aggregator
- a validator instance is started for any operation that requires aggregation
- if deploying many l1s, and on different networks, everything continues working (add validator, etc)
- we either need one aggregator that is reconfigured without lossing the previous conf, or many aggregator instances, first option is preferred
There was a problem hiding this comment.
it just updates the config and restarts the signature aggregator if second l1 is deployed
| func StartSignatureAggregator(binPath string, configPath string, logger logging.Logger) (int, error) { | ||
| // Function to check if port is in use | ||
| isPortInUse := func() bool { | ||
| conn, err := net.Dial("tcp", "localhost:8080") |
There was a problem hiding this comment.
- can we use a non standard port for the aggregator?
- can the port be in constants?
| // Try to start the aggregator with retries | ||
| maxRetries := 3 | ||
| for i := 0; i < maxRetries; i++ { | ||
| if isPortInUse() { |
There was a problem hiding this comment.
i should not base on port, but on a conf file with PID that indicates if the aggregator is executing
There was a problem hiding this comment.
you can easily kill some other process with this method
| InfoAPI: APIConfig{BaseURL: networkEndpoint}, | ||
| SignatureCacheSize: 1048576, | ||
| AllowPrivateIPs: true, | ||
| TrackedSubnetIDs: []string{subnetID}, |
There was a problem hiding this comment.
lets enabling reconfiguration to add more subnet IDs, and keep one aggregator for all L1s
This PR changes the signature aggregator from a dependency to a standalone binary