feat: batch eots sign #715
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements batch signing functionality for EOTS (Extractable One-Time Signatures) to improve performance by processing multiple signatures in a single request instead of height-by-height. Performance testing shows a 12x improvement (from 1.149s to 95.10ms for a batch of 60 signatures).
Key changes:
- Introduces batch signing methods across the EOTS manager interface and implementations
- Adds database operations for batch retrieval and storage of signing records
- Replaces individual signing calls with batch processing in the finality submitter
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| finality-provider/service/finality_submitter.go | Implements batch signing workflow using new SignFinalitySigBatch method |
| eotsmanager/store/storedsigningrecord.go | Adds BatchSignRecord struct for batch operations |
| eotsmanager/store/eotsstore.go | Implements batch database operations for sign records |
| eotsmanager/service/rpcserver.go | Adds RPC handler for batch EOTS signing |
| eotsmanager/proto/ files | Defines protobuf messages and gRPC service for batch operations |
| eotsmanager/localmanager.go | Implements core batch signing logic with double-sign protection |
| eotsmanager/eotsmanager.go | Defines interface and data structures for batch signing |
| eotsmanager/client/rpcclient.go | Implements gRPC client for batch signing requests |
| CHANGELOG.md | Documents the new batch signing feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| heights = append(heights, request.Height) | ||
| } | ||
|
|
||
| existingRecords, err := lm.es.GetSignRecordsBatch(eotsPk, chainID, heights) |
eotsmanager/localmanager.go
Outdated
| lm.metrics.IncrementEotsFpTotalEotsSignCounter(hex.EncodeToString(eotsPk)) | ||
| lm.metrics.SetEotsFpLastEotsSignHeight(hex.EncodeToString(eotsPk), float64(height)) |
There was a problem hiding this comment.
Suggestion to move hex.EncodeToString(eotsPk) outside of for loop
|
|
||
| // Create slices to store only the valid items | ||
| // Use batch signing for better performance | ||
| batchSigMap, err := ds.SignFinalitySigBatch(ctx, blocks) |
There was a problem hiding this comment.
what happens if on SubmitBatchFinalitySigs only one of the signatures fails?
There was a problem hiding this comment.
if it's an unexpected error, the whole batch fails
RafilxTenfen
left a comment
There was a problem hiding this comment.
Great improvement, only one suggestion and one question
Thanks! We are already constrained by |
Instead of requesting eots to sign height by height, we do it in batches. Local modified (not committed) cosmos (as we have 1s block time) e2e catch-up test shows that for a batch of 60, execution time for old implementations is `1.149s` and for the new implementation `95.10ms`. These times should be even more exaggerated as the local e2e test has minimal network latency. (cherry picked from commit 69503ed)
Instead of requesting eots to sign height by height, we do it in batches. Local modified (not committed) cosmos (as we have 1s block time) e2e catch-up test shows that for a batch of 60, execution time for old implementations is `1.149s` and for the new implementation `95.10ms`. These times should be even more exaggerated as the local e2e test has minimal network latency. <hr>This is an automatic backport of pull request #715 done by [Mergify](https://mergify.com). Co-authored-by: Lazar <12626340+Lazar955@users.noreply.github.com>
Instead of requesting eots to sign height by height, we do it in batches.
Local modified (not committed) cosmos (as we have 1s block time) e2e catch-up test shows that for a batch of 60, execution time for old implementations is
1.149sand for the new implementation95.10ms. These times should be even more exaggerated as the local e2e test has minimal network latency.