fix EIP-7044 implementation when using batch verification (#5953)

In #5120, EIP-7044 support got added to the state transition function to
force `CAPELLA_FORK_VERSION` to be used when validiting `VoluntaryExit`
messages, irrespective of their `epoch`.

In #5637, similar logic was added when batch verifying BLS signatures,
which is used during gossip validation (libp2p gossipsub, and req/resp).
However, that logic did not match the one introduced in #5120, and only
uses `CAPELLA_FORK_VERSION` when a `VoluntaryExit`'s `epoch` was set to
a value `>= CAPELLA_FORK_EPOCH`. Otherwise, `BELLATRIX_FORK_VERSION`
would still be used when validating `VoluntaryExit`, e.g., with `epoch`
set to `0`, as is the case in this Holesky block:

- https://holesky.beaconcha.in/slot/1076985#voluntary-exits

Extracting the correct logic from #5120 into a function, and reusing it
when verifying BLS signatures fixes this issue, and also leverages the
exhaustive EF test suite that covers the (correct) #5120 logic.

This fix only affects networks that have EIP-7044 applied (post-Deneb).

Without the fix, Deneb blocks with a `VoluntaryExit` with `epoch` set to
`< CAPELLA_FORK_EPOCH` incorrectly fail to validate despite being valid.

Incorrect blocks that contain a malicious `VoluntaryExit` with `epoch`
set to `< CAPELLA_FORK_EPOCH` and signed using `BELLATRIX_FORK_VERSION`
_would_ pass the BLS verification stage, but subsequently fail the state
transition logic. Such blocks would still correctly be labeled invalid.
This commit is contained in:
Etan Kissling 2024-02-25 15:25:26 +01:00 committed by GitHub
parent d09bf3b587
commit f54fa083b4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 37 additions and 29 deletions

View File

@ -285,7 +285,7 @@ proc addHeadBlockWithParent*(
var sigs: seq[SignatureSet]
if (let e = sigs.collectSignatureSets(
signedBlock, dag.db.immutableValidators,
dag.clearanceState, dag.cfg.genesisFork(), dag.cfg.capellaFork(),
dag.clearanceState, dag.cfg.genesisFork(), dag.cfg.CAPELLA_FORK_VERSION,
cache); e.isErr()):
# A PublicKey or Signature isn't on the BLS12-381 curve
info "Unable to load signature sets",

View File

@ -216,6 +216,20 @@ func compute_voluntary_exit_signing_root*(
fork, DOMAIN_VOLUNTARY_EXIT, epoch, genesis_validators_root)
compute_signing_root(voluntary_exit, domain)
func voluntary_exit_signature_fork*(
consensusFork: static ConsensusFork,
state_fork: Fork,
capella_fork_version: Version): Fork =
when consensusFork >= ConsensusFork.Deneb:
# Always use Capella fork version, disregarding `VoluntaryExit` epoch
# [Modified in Deneb:EIP7044]
Fork(
previous_version: capella_fork_version,
current_version: capella_fork_version,
epoch: GENESIS_EPOCH) # irrelevant when current/previous identical
else:
state_fork
func get_voluntary_exit_signature*(
fork: Fork, genesis_validators_root: Eth2Digest,
voluntary_exit: VoluntaryExit,

View File

@ -235,7 +235,7 @@ proc collectSignatureSets*(
validatorKeys: openArray[ImmutableValidatorData2],
state: ForkedHashedBeaconState,
genesis_fork: Fork,
capella_fork: Fork,
capella_fork_version: Version,
cache: var StateCache): Result[void, cstring] =
## Collect all signature verifications that process_block would normally do
## except deposits, in one go.
@ -385,6 +385,9 @@ proc collectSignatureSets*(
# SSZ deserialization guarantees that blocks received from random sources
# including peer or RPC
# have at most MAX_VOLUNTARY_EXITS voluntary exits.
if signed_block.message.body.voluntary_exits.len > 0:
let voluntary_exit_fork = withConsensusFork(state.kind):
consensusFork.voluntary_exit_signature_fork(fork, capella_fork_version)
for i in 0 ..< signed_block.message.body.voluntary_exits.len:
# don't use "items" for iterating over large type
# due to https://github.com/nim-lang/Nim/issues/14421
@ -396,11 +399,7 @@ proc collectSignatureSets*(
sigs.add voluntary_exit_signature_set(
# https://eips.ethereum.org/EIPS/eip-7044
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.7/specs/deneb/beacon-chain.md#modified-process_voluntary_exit
(if state.kind >= ConsensusFork.Capella:
capella_fork
else:
fork),
genesis_validators_root, volex.message, key,
voluntary_exit_fork, genesis_validators_root, volex.message, key,
volex.signature.load.valueOr do:
return err(
"collectSignatureSets: cannot load voluntary exit signature"))

View File

@ -379,16 +379,11 @@ proc check_voluntary_exit*(
# Verify signature
if skipBlsValidation notin flags:
let exitSignatureFork =
when typeof(state).kind >= ConsensusFork.Deneb:
Fork(
previous_version: cfg.CAPELLA_FORK_VERSION,
current_version: cfg.CAPELLA_FORK_VERSION,
epoch: cfg.CAPELLA_FORK_EPOCH)
else:
state.fork
const consensusFork = typeof(state).kind
let voluntary_exit_fork = consensusFork.voluntary_exit_signature_fork(
state.fork, cfg.CAPELLA_FORK_VERSION)
if not verify_voluntary_exit_signature(
exitSignatureFork, state.genesis_validators_root, voluntary_exit,
voluntary_exit_fork, state.genesis_validators_root, voluntary_exit,
validator[].pubkey, signed_voluntary_exit.signature):
return err("Exit: invalid signature")