ChainSafe / lodestar

🌟 TypeScript Implementation of Ethereum Consensus
https://lodestar.chainsafe.io
Apache License 2.0
1.18k stars 289 forks source link

verifying remote signing / web3signer #7174

Open franjoespejo opened 1 week ago

franjoespejo commented 1 week ago

Problem description

Lodestar already implements the w3signer standard https://chainsafe.github.io/lodestar/run/validator-management/external-signer

The standard assumes the w3s trusts the validator client, since there is no way for the w3s to verify the payload to be signed.

Solution description

The proposal to fix that is: https://github.com/ethereum/remote-signing-api/pull/10

Nimbus has implemented this: https://nimbus.guide/web3signer.html#verifying-web3signer, it is specially useful for diva which distributes the w3s signatures.

nflaig commented 1 week ago

I am curious what's the expected / best UX for this, in the Nimbus docs it is noted that you have to set an extra flag

You can instruct Nimbus to use the verifying Web3Signer protocol by either supplying the --verifying-web3-signer command-line option

but I don't think this should be required as you could determine based on /api/v1/eth2/sign/{pubkey} if for that specific pubkey / remote signer proofs should be provided or not (based on block_properties).

I am wondering if it's useful to enforce to only send verified message via such a flag but on the other hand it's the w3s that needs to enforce it and reject requests without proofs.

nflaig commented 1 week ago

@franjoespejo something else to consider, the keymanager api allows to manage remote signer keys, should consider updating that as well

franjoespejo commented 1 week ago

Thanks for checking in detail. Regarding UX, IMO once the verifying w3s is accepted, the pubkeys endpoint can send the verifying options, but meanwhile the options can be set in the validator client, because it seems more simple and compatible with the current w3s standard.

Regarding keymanager api, yes that's also an option, I understand that the pubkey endpoint is an alternative to that in which w3s service doesnt have to consume validator service but the other way around. We are in favour of using the pubkey endpoint.

nflaig commented 1 week ago

but meanwhile the options can be set in the validator client, because it seems more simple and compatible with the current w3s standard.

There is no standard to set this on the validator client other than the keymanager api to manage remote signers or what are you referencing? The file config Nimbus uses seems like a custom format

Regarding keymanager api, yes that's also an option, I understand that the pubkey endpoint is an alternative to that in which w3s service doesnt have to consume validator service but the other way around. We are in favour of using the pubkey endpoint.

yes I agree, all clients should just keep keys in sync with w3s by polling /pubkeys endpoint but for spec completeness this would have to be updated.

it is specially useful for diva which distributes the w3s signatures.

Could you please elaborate a bit on this and the usefulness / requirement of this feature in general? In a common node setup, the w3s <> validator client is a trusted setup and shouldn't require this, so would be good to add a bit more context for motivation / rationale.

In general, I think there should be a rough consensus on the spec to move forward with this first.

franjoespejo commented 1 week ago

There is no standard to set this on the validator client other than the keymanager api to manage remote signers or what are you referencing? The file config Nimbus uses seems like a custom format

Right, there is not, nimbus does take it from a config file or a flag --proven-block-property=.execution_payload.fee_recipient

Could you please elaborate a bit on this and the usefulness / requirement of this feature in general? In a common node setup, the w3s <> validator client is a trusted setup and shouldn't require this, so would be good to add a bit more context for motivation / rationale.

Yes, at diva we build a DVT, so 1 validator is signing using 5 different diva nodes, each with its vc/bc/ec. Each vc needs to send the signing duties to its w3s (diva-node). For each validator duty, one out of the 5 diva nodes aggregates the 5 bls signatures, (diva nodes talk to each other). For block proposals we need to be able to verify the fee recipient field of the block proposal message. Happy to chat about it more in detail if needed.

It can be useful also for plain w3s where the setup is not trusted.

nflaig commented 1 week ago

Right, there is not, nimbus does take it from a config file or a flag --proven-block-property=.execution_payload.fee_recipient

the cli flag option would be simple for us to add but seems better to let the w3s drive this and return the new format including the block properties in /pubkeys api, or alternatively configure it via keymanager api which is standardized

one out of the 5 diva nodes aggregates the 5 bls signatures

ah thanks got how it works now, was missing the part that you select one to do the aggregation

For block proposals we need to be able to verify the fee recipient field

so this is limited to local blocks for now? we can't proof this in case of blinded block but I see you have another PR open that seems to solve this

Thanks for the clarifications!

franjoespejo commented 1 week ago

so this is limited to local blocks for now? we can't proof this in case of blinded block but I see you have another PR open that seems to solve this

The other PR implies changes to the Beacon api to forward the relayer signature to the vc, so tbh we don't think it is going to be accepted. Right now with the verify-w3s we are able to validate that the fee recipient of a blinded block belongs to one of our whitelisted builders.

nflaig commented 1 week ago

the verify-w3s we are able to validate that the fee recipient of a blinded block belongs to one of our whitelisted builder

but how does the validator client create a proof for fee recipient of a blinded block? it does not have access to execution_payload.fee_recipient

franjoespejo commented 1 week ago

I think It doesnt have access to the tx list but a hash instead, and the rest info remains. Pls check: BlindedBlock, it drives to ExecutionPayloadCommon

For the w3s there wouldn't be difference between block/blindedblock, pls check BeaconBlockRequest

nflaig commented 1 week ago

Ah right, fee_recipient is part of execution payload header as well. In that case would you set execution_payload_header.fee_recipient in block properties? kinda tricky to work with field names I guess

franjoespejo commented 1 week ago

Right, that would be the correct path. Looking at the nimbus code it seems they use it as a kind of selector and map execution_payload-execution_payload_header.