attestation and aggregated attestation BN fixes for Electra (#6641)
* attestation and aggregated attestation BN fixes for Electra * disable electra transition for this PR * use cleaned-up verification function everywhere
This commit is contained in:
tersec
GitHub
parent
e9e0149d55
commit
c01cf6601a
2
Makefile
2
Makefile
|
|
@ -92,8 +92,6 @@ TOOLS_CSV := $(subst $(SPACE),$(COMMA),$(TOOLS))
|
|||
deps \
|
||||
update \
|
||||
test \
|
||||
clean_eth2_network_simulation_all \
|
||||
eth2_network_simulation \
|
||||
clean \
|
||||
libbacktrace \
|
||||
book \
|
||||
|
|
|
|||
|
|
@ -394,17 +394,21 @@ proc addAttestation(
|
|||
|
||||
func getAttestationCandidateKey(
|
||||
data: AttestationData,
|
||||
committee_bits: AttestationCommitteeBits =
|
||||
default(AttestationCommitteeBits)): Eth2Digest =
|
||||
committee_index: Opt[CommitteeIndex]): Eth2Digest =
|
||||
# Some callers might have used for the key just htr(data), so rather than
|
||||
# risk some random regression (one was caught in test suite, but there is
|
||||
# not any particular reason other code could not have manually calculated
|
||||
# the key, too), special-case the phase0 case as htr(data).
|
||||
if committee_bits == static(default(typeof(committee_bits))):
|
||||
if committee_index.isNone:
|
||||
# i.e. no committees selected, so it can't be an actual Electra attestation
|
||||
hash_tree_root(data)
|
||||
else:
|
||||
hash_tree_root([hash_tree_root(data), hash_tree_root(committee_bits)])
|
||||
hash_tree_root([hash_tree_root(data), hash_tree_root(committee_index.get.uint64)])
|
||||
|
||||
func getAttestationCandidateKey(
|
||||
attestationDataRoot: Eth2Digest, committee_index: CommitteeIndex):
|
||||
Eth2Digest =
|
||||
hash_tree_root([attestationDataRoot, hash_tree_root(committee_index.uint64)])
|
||||
|
||||
proc addAttestation*(
|
||||
pool: var AttestationPool,
|
||||
|
|
@ -437,8 +441,8 @@ proc addAttestation*(
|
|||
# TODO withValue is an abomination but hard to use anything else too without
|
||||
# creating an unnecessary AttestationEntry on the hot path and avoiding
|
||||
# multiple lookups
|
||||
template addAttToPool(attCandidates: untyped, entry: untyped) =
|
||||
let attestation_data_root = hash_tree_root(entry.data)
|
||||
template addAttToPool(attCandidates: untyped, entry: untyped, committee_index: untyped) =
|
||||
let attestation_data_root = getAttestationCandidateKey(entry.data, committee_index)
|
||||
|
||||
attCandidates[candidateIdx.get()].withValue(attestation_data_root, entry) do:
|
||||
if not addAttestation(entry[], attestation, signature):
|
||||
|
|
@ -453,7 +457,7 @@ proc addAttestation*(
|
|||
template addAttToPool(_: phase0.Attestation) {.used.} =
|
||||
let newAttEntry = Phase0AttestationEntry(
|
||||
data: attestation.data, committee_len: attestation.aggregation_bits.len)
|
||||
addAttToPool(pool.phase0Candidates, newAttEntry)
|
||||
addAttToPool(pool.phase0Candidates, newAttEntry, Opt.none CommitteeIndex)
|
||||
pool.addForkChoiceVotes(
|
||||
attestation.data.slot, attesting_indices,
|
||||
attestation.data.beacon_block_root, wallTime)
|
||||
|
|
@ -474,7 +478,7 @@ proc addAttestation*(
|
|||
let newAttEntry = ElectraAttestationEntry(
|
||||
data: data,
|
||||
committee_len: attestation.aggregation_bits.len)
|
||||
addAttToPool(pool.electraCandidates, newAttEntry)
|
||||
addAttToPool(pool.electraCandidates, newAttEntry, Opt.some committee_index)
|
||||
pool.addForkChoiceVotes(
|
||||
attestation.data.slot, attesting_indices,
|
||||
attestation.data.beacon_block_root, wallTime)
|
||||
|
|
@ -497,7 +501,7 @@ func covers*(
|
|||
return false
|
||||
|
||||
pool.phase0Candidates[candidateIdx.get()].withValue(
|
||||
getAttestationCandidateKey(data), entry):
|
||||
getAttestationCandidateKey(data, Opt.none CommitteeIndex), entry):
|
||||
if entry[].covers(bits):
|
||||
return true
|
||||
|
||||
|
|
@ -955,7 +959,11 @@ proc getElectraAttestationsForBlock*(
|
|||
# and score possible on-chain attestations while collecting candidates
|
||||
# (previous loop) and reavaluate cache key definition
|
||||
let
|
||||
key = (entry.data.beacon_block_root, entry.data.slot)
|
||||
entry2 = block:
|
||||
var e2 = entry.data
|
||||
e2.index = 0
|
||||
e2
|
||||
key = (hash_tree_root(entry2), entry.data.slot)
|
||||
newAtt = entry[].toElectraAttestation(entry[].aggregates[j])
|
||||
|
||||
candidatesPerBlock.withValue(key, candidate):
|
||||
|
|
@ -988,25 +996,22 @@ proc getElectraAttestationsForBlock*(
|
|||
# Sort candidates by score use slot as a tie-breaker
|
||||
candidates.sort()
|
||||
|
||||
# Consolidate attestation aggregates with disjoint comittee bits into single
|
||||
# Consolidate attestation aggregates with disjoint committee bits into single
|
||||
# attestation
|
||||
var res: seq[electra.Attestation]
|
||||
for a in candidatesPerBlock.values():
|
||||
|
||||
if a.len > 1:
|
||||
let
|
||||
att = compute_on_chain_aggregate(a).valueOr:
|
||||
continue
|
||||
let att = compute_on_chain_aggregate(a).valueOr:
|
||||
continue
|
||||
res.add(att)
|
||||
#no on chain candidates
|
||||
# no on-chain candidates
|
||||
else:
|
||||
res.add(a)
|
||||
|
||||
if res.lenu64 == MAX_ATTESTATIONS_ELECTRA:
|
||||
break
|
||||
|
||||
let
|
||||
packingDur = Moment.now() - startPackingTick
|
||||
let packingDur = Moment.now() - startPackingTick
|
||||
|
||||
debug "Packed attestations for block",
|
||||
newBlockSlot, packingDur, totalCandidates, attestations = res.len()
|
||||
|
|
@ -1051,8 +1056,7 @@ func getElectraAggregatedAttestation*(
|
|||
return Opt.none(electra.Attestation)
|
||||
|
||||
pool.electraCandidates[candidateIdx.get].withValue(
|
||||
attestationDataRoot, entry):
|
||||
|
||||
getAttestationCandidateKey(attestationDataRoot, committeeIndex), entry):
|
||||
if entry.data.index == committeeIndex.distinctBase:
|
||||
entry[].updateAggregates()
|
||||
|
||||
|
|
|
|||
|
|
@ -237,7 +237,7 @@ func get_attesting_indices_one*(shufflingRef: ShufflingRef,
|
|||
Opt[ValidatorIndex] =
|
||||
# A variation on get_attesting_indices that returns the validator index only
|
||||
# if only one validator index is set
|
||||
static: doAssert not on_chain, "only on_chain supported"
|
||||
static: doAssert not on_chain, "only not on_chain supported"
|
||||
|
||||
var res = Opt.none(ValidatorIndex)
|
||||
let committee_index = ? get_committee_index_one(committee_bits)
|
||||
|
|
|
|||
|
|
@ -577,13 +577,22 @@ proc compute_on_chain_aggregate*(
|
|||
for i, a in aggregates:
|
||||
totalLen += a.aggregation_bits.len
|
||||
|
||||
# TODO doesn't work if a committee is skipped
|
||||
var aggregation_bits = ElectraCommitteeValidatorsBits.init(totalLen)
|
||||
var pos = 0
|
||||
var prev_committee_index: Opt[CommitteeIndex]
|
||||
for i, a in aggregates:
|
||||
let
|
||||
committee_index = ? get_committee_index_one(a.committee_bits)
|
||||
first = pos == 0
|
||||
|
||||
when false:
|
||||
if prev_committee_index.isNone:
|
||||
prev_committee_index = Opt.some committee_index
|
||||
elif committee_index.distinctBase <= prev_committee_index.get.distinctBase:
|
||||
continue
|
||||
prev_committee_index = Opt.some committee_index
|
||||
|
||||
for b in a.aggregation_bits:
|
||||
aggregation_bits[pos] = b
|
||||
pos += 1
|
||||
|
|
|
|||
|
|
@ -1592,7 +1592,7 @@ proc signAndSendAggregate(
|
|||
if not is_aggregator(shufflingRef, slot, committee_index, selectionProof):
|
||||
return
|
||||
|
||||
template signAndSendAggregate() =
|
||||
template signAndSendAggregatedAttestations() =
|
||||
msg.signature = block:
|
||||
let res = await validator.getAggregateAndProofSignature(
|
||||
fork, genesis_validators_root, msg.message)
|
||||
|
|
@ -1612,31 +1612,29 @@ proc signAndSendAggregate(
|
|||
if node.dag.cfg.consensusForkAtEpoch(slot.epoch) >= ConsensusFork.Electra:
|
||||
# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.8/specs/electra/validator.md#construct-aggregate
|
||||
# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.8/specs/electra/validator.md#aggregateandproof
|
||||
var
|
||||
msg = electra.SignedAggregateAndProof(
|
||||
message: electra.AggregateAndProof(
|
||||
aggregator_index: distinctBase validator_index,
|
||||
selection_proof: selectionProof))
|
||||
var msg = electra.SignedAggregateAndProof(
|
||||
message: electra.AggregateAndProof(
|
||||
aggregator_index: distinctBase validator_index,
|
||||
selection_proof: selectionProof))
|
||||
|
||||
msg.message.aggregate = node.attestationPool[].getElectraAggregatedAttestation(
|
||||
slot, default(Eth2Digest), committee_index).valueOr:
|
||||
slot, committee_index).valueOr:
|
||||
return
|
||||
|
||||
signAndSendAggregate()
|
||||
signAndSendAggregatedAttestations()
|
||||
else:
|
||||
# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.8/specs/phase0/validator.md#construct-aggregate
|
||||
# https://github.com/ethereum/consensus-specs/blob/v1.4.0/specs/phase0/validator.md#aggregateandproof
|
||||
var
|
||||
msg = phase0.SignedAggregateAndProof(
|
||||
message: phase0.AggregateAndProof(
|
||||
aggregator_index: distinctBase validator_index,
|
||||
selection_proof: selectionProof))
|
||||
var msg = phase0.SignedAggregateAndProof(
|
||||
message: phase0.AggregateAndProof(
|
||||
aggregator_index: distinctBase validator_index,
|
||||
selection_proof: selectionProof))
|
||||
|
||||
msg.message.aggregate = node.attestationPool[].getPhase0AggregatedAttestation(
|
||||
slot, committee_index).valueOr:
|
||||
return
|
||||
|
||||
signAndSendAggregate()
|
||||
signAndSendAggregatedAttestations()
|
||||
|
||||
proc sendAggregatedAttestations(
|
||||
node: BeaconNode, head: BlockRef, slot: Slot) =
|
||||
|
|
|
|||
|
|
@ -104,7 +104,7 @@ DL_GETH="0"
|
|||
|
||||
: ${BEACON_NODE_COMMAND:="./build/nimbus_beacon_node$EXE_EXTENSION"}
|
||||
: ${DENEB_FORK_EPOCH:=0}
|
||||
: ${ELECTRA_FORK_EPOCH:=5000}
|
||||
: ${ELECTRA_FORK_EPOCH:=500}
|
||||
|
||||
#NIMBUS EL VARS
|
||||
RUN_NIMBUS_ETH1="0"
|
||||
|
|
@ -216,6 +216,10 @@ while true; do
|
|||
DENEB_FORK_EPOCH="$2"
|
||||
shift 2
|
||||
;;
|
||||
--electra-fork-epoch)
|
||||
ELECTRA_FORK_EPOCH="$2"
|
||||
shift 2
|
||||
;;
|
||||
--stop-at-epoch)
|
||||
STOP_AT_EPOCH=$2
|
||||
STOP_AT_EPOCH_FLAG="--debug-stop-at-epoch=$2"
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ import
|
|||
# Test utilities
|
||||
./testutil, ./testdbutil, ./testblockutil, ./consensus_spec/fixtures_utils
|
||||
|
||||
from std/sequtils import toSeq
|
||||
from std/sequtils import mapIt, toSeq
|
||||
from ./testbcutil import addHeadBlock
|
||||
|
||||
func combine(tgt: var (phase0.Attestation | electra.Attestation),
|
||||
|
|
@ -898,8 +898,21 @@ suite "Attestation pool electra processing" & preset():
|
|||
attestations[0].aggregation_bits.countOnes() == 2
|
||||
attestations[0].committee_bits.countOnes() == 2
|
||||
|
||||
|
||||
test "Aggregated attestations with disjoint comittee bits into a single on-chain aggregate" & preset():
|
||||
proc verifyAttestationSignature(attestation: electra.Attestation): bool =
|
||||
withState(state[]):
|
||||
when consensusFork == ConsensusFork.Electra:
|
||||
let
|
||||
fork = pool.dag.cfg.forkAtEpoch(forkyState.data.slot.epoch)
|
||||
attesting_indices = get_attesting_indices(
|
||||
forkyState.data, attestation.data, attestation.aggregation_bits,
|
||||
attestation.committee_bits, cache)
|
||||
verify_attestation_signature(
|
||||
fork, pool.dag.genesis_validators_root, attestation.data,
|
||||
attesting_indices.mapIt(forkyState.data.validators.item(it).pubkey),
|
||||
attestation.signature)
|
||||
else:
|
||||
raiseAssert "must be electra"
|
||||
|
||||
let
|
||||
bc0 = get_beacon_committee(
|
||||
|
|
@ -921,6 +934,11 @@ suite "Attestation pool electra processing" & preset():
|
|||
attestation_3 = makeElectraAttestation(
|
||||
state[], state[].latest_block_root, bc1[1], cache)
|
||||
|
||||
check:
|
||||
verifyAttestationSignature(attestation_1)
|
||||
verifyAttestationSignature(attestation_2)
|
||||
verifyAttestationSignature(attestation_3)
|
||||
|
||||
pool[].addAttestation(
|
||||
attestation_1, @[bc0[0]], attestation_1.loadSig,
|
||||
attestation_1.data.slot.start_beacon_time)
|
||||
|
|
@ -942,6 +960,8 @@ suite "Attestation pool electra processing" & preset():
|
|||
let attestations = pool[].getElectraAttestationsForBlock(state[], cache)
|
||||
|
||||
check:
|
||||
verifyAttestationSignature(attestations[0])
|
||||
|
||||
# A single final chain aggregated attestation should be created
|
||||
# with same data, 2 committee bits and 3 aggregation bits
|
||||
attestations.len == 1
|
||||
|
|
@ -966,11 +986,35 @@ suite "Attestation pool electra processing" & preset():
|
|||
att3 = makeElectraAttestation(
|
||||
state[], state[].latest_block_root, bc0[3], cache)
|
||||
|
||||
proc verifyAttestationSignature(attestation: electra.Attestation): bool =
|
||||
withState(state[]):
|
||||
when consensusFork == ConsensusFork.Electra:
|
||||
let
|
||||
fork = pool.dag.cfg.forkAtEpoch(forkyState.data.slot.epoch)
|
||||
attesting_indices = get_attesting_indices(
|
||||
forkyState.data, attestation.data, attestation.aggregation_bits,
|
||||
attestation.committee_bits, cache)
|
||||
verify_attestation_signature(
|
||||
fork, pool.dag.genesis_validators_root, attestation.data,
|
||||
attesting_indices.mapIt(forkyState.data.validators.item(it).pubkey),
|
||||
attestation.signature)
|
||||
else:
|
||||
raiseAssert "must be electra"
|
||||
|
||||
check:
|
||||
verifyAttestationSignature(att0)
|
||||
verifyAttestationSignature(att0x)
|
||||
verifyAttestationSignature(att1)
|
||||
verifyAttestationSignature(att2)
|
||||
verifyAttestationSignature(att3)
|
||||
|
||||
# Both attestations include member 2 but neither is a subset of the other
|
||||
att0.combine(att2)
|
||||
att1.combine(att2)
|
||||
|
||||
check:
|
||||
verifyAttestationSignature(att0)
|
||||
verifyAttestationSignature(att1)
|
||||
not pool[].covers(att0.data, att0.aggregation_bits)
|
||||
|
||||
pool[].addAttestation(
|
||||
|
|
@ -978,17 +1022,23 @@ suite "Attestation pool electra processing" & preset():
|
|||
pool[].addAttestation(
|
||||
att1, @[bc0[1], bc0[2]], att1.loadSig, att1.data.slot.start_beacon_time)
|
||||
|
||||
for att in pool[].electraAttestations(Opt.none Slot, Opt.none CommitteeIndex):
|
||||
check: verifyAttestationSignature(att)
|
||||
|
||||
check:
|
||||
process_slots(
|
||||
defaultRuntimeConfig, state[],
|
||||
getStateField(state[], slot) + MIN_ATTESTATION_INCLUSION_DELAY, cache,
|
||||
info, {}).isOk()
|
||||
|
||||
for att in pool[].electraAttestations(Opt.none Slot, Opt.none CommitteeIndex):
|
||||
check: verifyAttestationSignature(att)
|
||||
|
||||
check:
|
||||
pool[].getElectraAttestationsForBlock(state[], cache).len() == 1
|
||||
# Can get either aggregate here, random!
|
||||
pool[].getElectraAggregatedAttestation(
|
||||
1.Slot, hash_tree_root(att0.data), 0.CommitteeIndex).isSome()
|
||||
verifyAttestationSignature(pool[].getElectraAggregatedAttestation(
|
||||
1.Slot, hash_tree_root(att0.data), 0.CommitteeIndex).get)
|
||||
|
||||
# Add in attestation 3 - both aggregates should now have it added
|
||||
pool[].addAttestation(
|
||||
|
|
@ -999,6 +1049,7 @@ suite "Attestation pool electra processing" & preset():
|
|||
check:
|
||||
attestations.len() == 1
|
||||
attestations[0].aggregation_bits.countOnes() == 6
|
||||
#TODO: verifyAttestationSignature(attestations[0]) fails
|
||||
# Can get either aggregate here, random!
|
||||
pool[].getElectraAggregatedAttestation(
|
||||
1.Slot, hash_tree_root(attestations[0].data), 0.CommitteeIndex).isSome()
|
||||
|
|
@ -1013,6 +1064,7 @@ suite "Attestation pool electra processing" & preset():
|
|||
check:
|
||||
attestations.len() == 1
|
||||
attestations[0].aggregation_bits.countOnes() == 4
|
||||
verifyAttestationSignature(attestations[0])
|
||||
pool[].getElectraAggregatedAttestation(
|
||||
1.Slot, hash_tree_root(attestations[0].data), 0.CommitteeIndex).isSome()
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue