Skip to content

adding debounce to remote key manager #16051

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

adding debounce to remote key manager #16051

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
80 changes: 49 additions & 31 deletions validator/keymanager/remote-web3signer/keymanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"sync"
"time"

"github.com/OffchainLabs/prysm/v7/async"
"github.com/OffchainLabs/prysm/v7/async/event"
fieldparams "github.com/OffchainLabs/prysm/v7/config/fieldparams"
"github.com/OffchainLabs/prysm/v7/crypto/bls"
Expand All @@ -34,8 +35,9 @@
)

const (
maxRetries = 60
retryDelay = 10 * time.Second
maxRetries = 60

Check failure on line 38 in validator/keymanager/remote-web3signer/keymanager.go

View workflow job for this annotation

GitHub Actions / Lint 

File is not properly formatted (gofmt)
retryDelay = 10 * time.Second
fileChangeDebounce = 3 * time.Second // Debounce interval for file change events to handle manual editing
)

// SetupConfig includes configuration values for initializing.
Expand Down Expand Up @@ -286,11 +288,6 @@
log.WithError(err).Error("Could not close file watcher")
}
}()
initialFileInfo, err := os.Stat(km.keyFilePath)
if err != nil {
return errors.Wrap(err, "could not stat remote signer public key file")
}
initialFileSize := initialFileInfo.Size()
if err := watcher.Add(km.keyFilePath); err != nil {
return errors.Wrap(err, "could not add file to file watcher")
}
Expand All @@ -308,6 +305,26 @@
}
km.updatePublicKeys(slices.Collect(maps.Values(fk)))
}

ctx, cancel := context.WithCancel(ctx)
defer cancel()
fileChangesChan := make(chan any, 100)
defer close(fileChangesChan)

// We debounce events sent over the file changes channel by an interval
// to ensure we are not overwhelmed by a ton of events fired over the channel in
// a short span of time (e.g., during manual file editing).
go async.Debounce(ctx, fileChangeDebounce, fileChangesChan, func(event any) {
e, ok := event.(fsnotify.Event)
if !ok {
log.Errorf("Type %T is not a valid file system event", event)
return
}
if e.Has(fsnotify.Write) || e.Has(fsnotify.Create) {
km.handleFileChange(ctx)
}
})

for {
select {
case e, ok := <-watcher.Events:
Expand All @@ -322,30 +339,8 @@
if e.Has(fsnotify.Remove) {
return errors.New("remote signer key file was removed")
}
currentFileInfo, err := os.Stat(km.keyFilePath)
if err != nil {
return errors.Wrap(err, "could not stat remote signer public key file")
}
if currentFileInfo.Size() != initialFileSize {
log.Info("Remote signer key file updated")
fileKeys, _, err := km.readKeyFile()
if err != nil {
return errors.New("could not read key file")
}
// prioritize file keys over flag keys
if len(fileKeys) == 0 {
log.Warnln("Remote signer key file no longer has keys, defaulting to flag provided keys")
fileKeys = slices.Collect(maps.Values(km.flagLoadedKeysMap))
}
currentKeys, err := km.FetchValidatingPublicKeys(ctx)
if err != nil {
return errors.Wrap(err, "could not fetch current keys")
}
if !slices.Equal(currentKeys, fileKeys) {
km.updatePublicKeys(fileKeys)
}
initialFileSize = currentFileInfo.Size()
}
// Send event to debounce channel for processing
fileChangesChan <- e
case err, ok := <-watcher.Errors:
if !ok { // Channel was closed (i.e. Watcher.Close() was called).
log.Info("Closing file watcher")
Expand All @@ -359,6 +354,29 @@
}
}

// handleFileChange processes a file change event after debouncing.
func (km *Keymanager) handleFileChange(ctx context.Context) {
log.Info("Remote signer key file updated")
fileKeys, _, err := km.readKeyFile()
if err != nil {
log.WithError(err).Error("Could not read key file")
return
}
// prioritize file keys over flag keys
if len(fileKeys) == 0 {
log.Warnln("Remote signer key file no longer has keys, defaulting to flag provided keys")
fileKeys = slices.Collect(maps.Values(km.flagLoadedKeysMap))
}
currentKeys, err := km.FetchValidatingPublicKeys(ctx)
if err != nil {
log.WithError(err).Error("Could not fetch current keys")
return
}
if !slices.Equal(currentKeys, fileKeys) {
km.updatePublicKeys(fileKeys)
}
}

func (km *Keymanager) updatePublicKeys(keys [][48]byte) {
km.lock.Lock()
defer km.lock.Unlock()
Expand Down
68 changes: 63 additions & 5 deletions validator/keymanager/remote-web3signer/keymanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ func TestNewKeyManager_ChangingFileCreated(t *testing.T) {
require.Equal(t, slices.Contains(wantSlice, keys[i]), true)
}
// sleep needs to be at the front because of how watching the file works
time.Sleep(1 * time.Second)
// Wait for debounce interval plus processing time
time.Sleep(fileChangeDebounce + 500*time.Millisecond)

// Open the file for writing, create it if it does not exist, and truncate it if it does.
f, err := os.OpenFile(keyFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
Expand All @@ -235,6 +236,9 @@ func TestNewKeyManager_ChangingFileCreated(t *testing.T) {
require.Equal(t, 1, len(ks))
require.Equal(t, "0x8000a9a6d3f5e22d783eefaadbcf0298146adb5d95b04db910a0d4e16976b30229d0b1e7b9cda6c7e0bfa11f72efe055", hexutil.Encode(ks[0][:]))

// Wait for debounce and processing to update providedPublicKeys
time.Sleep(fileChangeDebounce + 500*time.Millisecond)

require.Equal(t, 1, len(km.providedPublicKeys))
require.Equal(t, "0x8000a9a6d3f5e22d783eefaadbcf0298146adb5d95b04db910a0d4e16976b30229d0b1e7b9cda6c7e0bfa11f72efe055", hexutil.Encode(km.providedPublicKeys[0][:]))
}
Expand Down Expand Up @@ -267,15 +271,15 @@ func TestNewKeyManager_FileAndFlagsWithDifferentKeys(t *testing.T) {
for _, key := range keys {
require.Equal(t, slices.Contains(wantSlice, hexutil.Encode(key[:])), true)
}
// wait for reading to be done
time.Sleep(2 * time.Second)
// wait for watcher to initialize
time.Sleep(500 * time.Millisecond)
// test fall back by clearing file
go func() {
err = file.WriteFile(keyFilePath, []byte(" "))
require.NoError(t, err)
}()
// waiting for writing to be done
time.Sleep(2 * time.Second)
// waiting for debounce and processing to complete
time.Sleep(fileChangeDebounce + 500*time.Millisecond)
require.LogsContain(t, logHook, "Remote signer key file no longer has keys, defaulting to flag provided keys")

// fall back to flag provided keys
Expand Down Expand Up @@ -321,6 +325,60 @@ func TestRefreshRemoteKeysFromFileChangesWithRetry(t *testing.T) {
require.Equal(t, 1, len(keys))
}

func TestRefreshRemoteKeysFromFileChanges_SameSizeDifferentKeys(t *testing.T) {
// This test verifies that key changes are detected even when the file size stays the same
// (e.g., swapping one key for another of equal length).
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
logHook := logTest.NewGlobal()
root, err := hexutil.Decode("0x270d43e74ce340de4bca2b1936beca0f4f5408d9e78aec4850920baf659d5b69")
require.NoError(t, err)
keyFilePath := filepath.Join(t.TempDir(), "keyfile.txt")

// Two keys of the same length (96 hex chars each = 48 bytes)
key1 := "0x8000a9a6d3f5e22d783eefaadbcf0298146adb5d95b04db910a0d4e16976b30229d0b1e7b9cda6c7e0bfa11f72efe055"
key2 := "0x800077e04f8d7496099b3d30ac5430aea64873a45e5bcfe004d2095babcbf55e21138ff0d5691abc29da190aa32755c6"

// Create initial file with key1
err = file.WriteFile(keyFilePath, []byte(key1))
require.NoError(t, err)

km, err := NewKeymanager(ctx, &SetupConfig{
BaseEndpoint: "http://example.com",
GenesisValidatorsRoot: root,
KeyFilePath: keyFilePath,
})
require.NoError(t, err)

// Verify initial key loaded
keys, err := km.FetchValidatingPublicKeys(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(keys))
require.Equal(t, key1, hexutil.Encode(keys[0][:]))

// Start the file watcher
go func() {
_ = km.refreshRemoteKeysFromFileChanges(ctx)
}()
// Wait for watcher to initialize
time.Sleep(100 * time.Millisecond)
require.LogsContain(t, logHook, "Successfully initialized file watcher")

// Write key2 to the file (same size as key1)
err = file.WriteFile(keyFilePath, []byte(key2))
require.NoError(t, err)

// Wait for file change to be detected (debounce interval is 3s + processing time)
time.Sleep(fileChangeDebounce + 500*time.Millisecond)

// Verify key was updated despite same file size
keys, err = km.FetchValidatingPublicKeys(ctx)
require.NoError(t, err)
require.Equal(t, 1, len(keys))
require.Equal(t, key2, hexutil.Encode(keys[0][:]), "Key should have been updated even though file size is the same")
require.LogsContain(t, logHook, "Remote signer key file updated")
}

func TestReadKeyFile_PathMissing(t *testing.T) {
root, err := hexutil.Decode("0x270d43e74ce340de4bca2b1936beca0f4f5408d9e78aec4850920baf659d5b69")
require.NoError(t, err)
Expand Down
Loading