Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/worker/keymanager: Speed up secret generation in tests #5422

Merged
merged 1 commit into from
Oct 31, 2023
Merged

go/worker/keymanager: Speed up secret generation in tests #5422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added .changelog/5422.trivial.md
View file
Open in desktop
Empty file.
28 changes: 20 additions & 8 deletions go/worker/keymanager/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,9 @@ func (w *Worker) setAccessList(runtimeID common.Namespace, nodes []*node.Node) {
)
}

func (w *Worker) generateMasterSecret(runtimeID common.Namespace, generation uint64, epoch beacon.EpochTime, kmStatus *api.Status, rtStatus *runtimeStatus) error {
func (w *Worker) generateMasterSecret(runtimeID common.Namespace, height int64, generation uint64, epoch beacon.EpochTime, kmStatus *api.Status, rtStatus *runtimeStatus) error {
w.logger.Info("generating master secret",
"height", height,
"generation", generation,
"epoch", epoch,
)
Expand Down Expand Up @@ -713,8 +714,9 @@ func (w *Worker) generateMasterSecret(runtimeID common.Namespace, generation uin
return err
}

func (w *Worker) generateEphemeralSecret(runtimeID common.Namespace, epoch beacon.EpochTime, kmStatus *api.Status, rtStatus *runtimeStatus) error {
func (w *Worker) generateEphemeralSecret(runtimeID common.Namespace, height int64, epoch beacon.EpochTime, kmStatus *api.Status, rtStatus *runtimeStatus) error {
w.logger.Info("generating ephemeral secret",
"height", height,
"epoch", epoch,
)

Expand Down Expand Up @@ -967,6 +969,7 @@ func (w *Worker) handleStatusUpdate(kmStatus *api.Status) {
"generation", kmStatus.Generation,
"rotation_epoch", kmStatus.RotationEpoch,
"checksum", hex.EncodeToString(kmStatus.Checksum),
"nodes", kmStatus.Nodes,
)

// Update metrics.
Expand Down Expand Up @@ -1117,8 +1120,9 @@ func (w *Worker) handleNewEpoch(epoch beacon.EpochTime) {
crw.epochTransition()
}

// Choose a random height for generating master/ephemeral secrets.
// Avoid blocks at the end of the epoch as secret generation,
// Choose a random height for generating master/ephemeral secrets to prevent key managers from
// all publishing transactions simultaneously, which would result in unnecessary gas waste.
// Additionally, avoid selecting blocks at the end of the epoch, as secret generation,
// publication and replication takes some time.
height, err := w.randomBlockHeight(epoch, 50)
if err != nil {
Expand Down Expand Up @@ -1187,7 +1191,11 @@ func (w *Worker) handleGenerateMasterSecret(height int64, epoch beacon.EpochTime
if w.genMstSecInProgress || w.genMstSecRetry > generateSecretMaxRetries {
return
}
if w.genSecHeight > height || w.genMstSecEpoch > epoch {
if w.genSecHeight > height && len(w.kmStatus.Nodes) > 1 || w.genMstSecEpoch > epoch {
// Observe that the height for secret generation is respected only if there are multiple
// nodes in the key manager committee. In production, this should have no impact, but in
// test scenarios, it allows us to transition to the next epoch earlier, as soon as all
// required secrets are generated.
return
}

Expand All @@ -1204,7 +1212,7 @@ func (w *Worker) handleGenerateMasterSecret(height int64, epoch beacon.EpochTime

// Submitting transaction can take time, so don't block the loop.
generateMasterSecret := func(kmStatus *api.Status, rtStatus *runtimeStatus) {
if err := w.generateMasterSecret(w.runtimeID, nextGen, nextEpoch, kmStatus, rtStatus); err != nil {
if err := w.generateMasterSecret(w.runtimeID, height, nextGen, nextEpoch, kmStatus, rtStatus); err != nil {
w.logger.Error("failed to generate master secret",
"err", err,
"retry", retry,
Expand Down Expand Up @@ -1286,7 +1294,11 @@ func (w *Worker) handleGenerateEphemeralSecret(height int64, epoch beacon.EpochT
if w.genEphSecInProgress || w.genEphSecRetry > generateSecretMaxRetries {
return
}
if w.genSecHeight > height {
if w.genSecHeight > height && len(w.kmStatus.Nodes) > 1 {
// Observe that the height for secret generation is respected only if there are multiple
// nodes in the key manager committee. In production, this should have no impact, but in
// test scenarios, it allows us to transition to the next epoch earlier, as soon as all
// required secrets are generated.
return
}

Expand All @@ -1302,7 +1314,7 @@ func (w *Worker) handleGenerateEphemeralSecret(height int64, epoch beacon.EpochT

// Submitting transaction can take time, so don't block the loop.
generateEphemeralSecret := func(kmStatus *api.Status, rtStatus *runtimeStatus) {
if err := w.generateEphemeralSecret(w.runtimeID, nextEpoch, kmStatus, rtStatus); err != nil {
if err := w.generateEphemeralSecret(w.runtimeID, height, nextEpoch, kmStatus, rtStatus); err != nil {
w.logger.Error("failed to generate ephemeral secret",
"err", err,
"retry", retry,
Expand Down
Loading