Skip to content

Proposal of new model for Web3Signer key set management #17322

Description

@syjn99

Background and Problems

With #17229, we have multiple channels that can write the list of public keys in the keymanager:

  • File update (needs --validators-external-signer-key-file): Adding/Deleting the entry of the key file would trigger fsnotify. Note that just swapping one pubkey into another will not trigger as file size change is only notified (known bug).
  • Keymanager API (needs --rpc or --web): Receiving {POST,DELETE} /eth/v1/remotekeys would trigger. Note that those POST/DELETE operation only affects the list public keys in-memory when we don't have a key file path. (Thus imported status is incorrect as it says "imported to validator client permanent storage" as per spec.)
  • 🆕 Polling URL (GET /api/v1/eth2/publicKeys) (needs --validators-external-signer-poll-interval): Polling every interval and overrides the list of public keys.

As @james-prysm stated (#17229 (comment)), #17229 has a fundamental limitation because we are managing the list of public keys as a simple Go slice:

// Keymanager defines the web3signer keymanager.
type Keymanager struct {
client internal.HttpSignerClient
genesisValidatorsRoot []byte
providedPublicKeys [][48]byte // (source of truth) flag loaded + file loaded + api loaded keys
flagLoadedKeysMap map[string][48]byte // stores what was provided from flag ( as opposed to from file )
accountsChangedFeed *event.Feed
validator *validator.Validate
retriesRemaining int
keyFilePath string
lock sync.RWMutex
}

Proposal

So I'd like to propose source-based key set management like this schema:

type keySource uint8
const (
	sourceFlag keySource = iota // command line; fixed at startup
	sourceURL                   // public-keys URL; replaced whole on every poll
	sourceFile                  // key file; written by the operator and the keymanager API
	numKeySources
)

type keySets struct {
	mu    sync.RWMutex
	sets  [numKeySources]map[pubkey]struct{}
	union map[pubkey]struct{}
	list  [][48]byte
}

func (k *keySets) replace(src keySource, keys map[pubkey]struct{}) (changed bool, union [][48]byte)
func (k *keySets) get(src keySource) map[pubkey]struct{} // clone
func (k *keySets) all() [][48]byte
func (k *keySets) owner(key pubkey) (keySource, bool)    // precedence: flag, url, file

There are a couple of rules:

  • Every key has exactly one owner. A source may only replace its own set. => Resolves James' concern. Keymanager API can never mutate sourceFlag or sourceURL.
  • The validating set is the union. (keySets.get) Slices shouldn't be passed all over the world as it can be mutated.

Spec compliance

Request Behavior Status
POST, key file configured Written to the file before responding imported — truthful
POST, no key file error: "no persistent storage for remote keys is configured; set --validators-external-signer-key-file" never claims imported for unpersisted keys
POST, key already in union duplicate regardless of owner "already known to the validator client"
DELETE, file-owned Removed from the file deleted
DELETE, derived-owned error naming the owner verbatim the spec's error case: "the key was actually found, but we couldn't stop using it … slashing conditions etc."
DELETE, unknown not_found
DELETE, nothing matches HTTP 200 with per-key statuses in request order "should never return a 404"

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions