Skip to content
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.

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

Shared validator pubkey #5883

Merged
merged 11 commits into from
Feb 21, 2024
15 changes: 11 additions & 4 deletions beacon_chain/beacon_chain_db.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1120,8 +1120,9 @@ proc getStateOnlyMutableValidators(
dstValidator = addr output.validators.data[i]

assign(
dstValidator.pubkey,
immutableValidators[i].pubkey.toPubKey())
dstValidator.pubkeyData,
HashedValidatorPubKey.init(
immutableValidators[i].pubkey.toPubKey()))
assign(
dstValidator.withdrawal_credentials,
immutableValidators[i].withdrawal_credentials)
Expand Down Expand Up @@ -1158,7 +1159,10 @@ proc getStateOnlyMutableValidators(
# Bypass hash cache invalidation
let dstValidator = addr output.validators.data[i]

assign(dstValidator.pubkey, immutableValidators[i].pubkey.toPubKey())
assign(
dstValidator.pubkeyData,
HashedValidatorPubKey.init(
immutableValidators[i].pubkey.toPubKey()))
assign(
dstValidator.withdrawal_credentials,
immutableValidators[i].withdrawal_credentials)
Expand Down Expand Up @@ -1195,7 +1199,10 @@ proc getStateOnlyMutableValidators(
for i in prevNumValidators ..< numValidators:
# Bypass hash cache invalidation
let dstValidator = addr output.validators.data[i]
assign(dstValidator.pubkey, immutableValidators[i].pubkey.toPubKey())
assign(
dstValidator.pubkeyData,
HashedValidatorPubKey.init(
immutableValidators[i].pubkey.toPubKey()))
output.validators.clearCaches(i)

true
Expand Down
16 changes: 12 additions & 4 deletions beacon_chain/networking/eth2_network.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1276,11 +1276,15 @@ proc toPeerAddr*(r: enr.TypedRecord,
case proto
of tcpProtocol:
if r.ip.isSome and r.tcp.isSome:
let ip = ipv4(r.ip.get)
let ip = IpAddress(
family: IpAddressFamily.IPv4,
address_v4: r.ip.get)
addrs.add MultiAddress.init(ip, tcpProtocol, Port r.tcp.get)

if r.ip6.isSome:
let ip = ipv6(r.ip6.get)
let ip = IpAddress(
family: IpAddressFamily.IPv6,
address_v6: r.ip6.get)
if r.tcp6.isSome:
addrs.add MultiAddress.init(ip, tcpProtocol, Port r.tcp6.get)
elif r.tcp.isSome:
Expand All @@ -1290,11 +1294,15 @@ proc toPeerAddr*(r: enr.TypedRecord,

of udpProtocol:
if r.ip.isSome and r.udp.isSome:
let ip = ipv4(r.ip.get)
let ip = IpAddress(
family: IpAddressFamily.IPv4,
address_v4: r.ip.get)
addrs.add MultiAddress.init(ip, udpProtocol, Port r.udp.get)

if r.ip6.isSome:
let ip = ipv6(r.ip6.get)
let ip = IpAddress(
family: IpAddressFamily.IPv6,
address_v6: r.ip6.get)
if r.udp6.isSome:
addrs.add MultiAddress.init(ip, udpProtocol, Port r.udp6.get)
elif r.udp.isSome:
Expand Down
2 changes: 1 addition & 1 deletion beacon_chain/spec/beaconstate.nim
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func get_validator_from_deposit*(deposit: DepositData):
amount - amount mod EFFECTIVE_BALANCE_INCREMENT, MAX_EFFECTIVE_BALANCE)

Validator(
pubkey: deposit.pubkey,
pubkeyData: HashedValidatorPubKey.init(deposit.pubkey),
withdrawal_credentials: deposit.withdrawal_credentials,
activation_eligibility_epoch: FAR_FUTURE_EPOCH,
activation_epoch: FAR_FUTURE_EPOCH,
Expand Down
31 changes: 28 additions & 3 deletions beacon_chain/spec/datatypes/base.nim
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,16 @@ type
pubkey*: CookedPubKey
withdrawal_credentials*: Eth2Digest

HashedValidatorPubKeyItem* = object
key*: ValidatorPubKey
root*: Eth2Digest

HashedValidatorPubKey* = object
value*: ptr HashedValidatorPubKeyItem

# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.6/specs/phase0/beacon-chain.md#validator
Validator* = object
pubkey*: ValidatorPubKey
pubkeyData*: HashedValidatorPubKey
arnetheduck marked this conversation as resolved.
Show resolved Hide resolved

withdrawal_credentials*: Eth2Digest
## Commitment to pubkey for withdrawals and transfers
Expand Down Expand Up @@ -441,7 +448,7 @@ type
# serialized. They're represented in memory to allow in-place SSZ reading
# and writing compatibly with the full Validator object.

pubkey* {.dontSerialize.}: ValidatorPubKey
pubkeyData* {.dontSerialize.}: HashedValidatorPubKey

withdrawal_credentials* {.dontSerialize.}: Eth2Digest
## Commitment to pubkey for withdrawals
Expand All @@ -467,7 +474,7 @@ type
# serialized. They're represented in memory to allow in-place SSZ reading
# and writing compatibly with the full Validator object.

pubkey* {.dontSerialize.}: ValidatorPubKey
pubkeyData* {.dontSerialize.}: HashedValidatorPubKey

withdrawal_credentials*: Eth2Digest
## Commitment to pubkey for withdrawals
Expand Down Expand Up @@ -545,6 +552,24 @@ type

flags*: set[RewardFlags]

func pubkey*(v: HashedValidatorPubKey): ValidatorPubKey =
if isNil(v.value):
# This should never happen but we guard against it in case a
# default-initialized Validator instance makes it through the other safety
# nets
Comment on lines +557 to +559
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if in such a hypothetical scenario, a log + exit wouldn't be more suitable. it starts injecting nasty bugs into surrounding logic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using a zero-inited pubkey is the current behavior in this scenario (it's used in test cases)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it then return the correct hash_tree_root of the zero validator instead of zero hash?

ValidatorPubKey()
else:
v.value[].key

template pubkey*(v: Validator): ValidatorPubKey =
v.pubkeyData.pubkey

func hash_tree_root*(v: HashedValidatorPubKey): Eth2Digest =
if isNil(v.value):
Eth2Digest() # Safety net
else:
v.value[].root

func getImmutableValidatorData*(validator: Validator): ImmutableValidatorData2 =
let cookedKey = validator.pubkey.loadValid() # `Validator` has valid key
ImmutableValidatorData2(
Expand Down
11 changes: 11 additions & 0 deletions beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,17 @@ proc readValue*(reader: var JsonReader[RestJson], value: var ValidatorPubKey) {.
else:
reader.raiseUnexpectedValue($res.error())

proc readValue*(reader: var JsonReader[RestJson], value: var HashedValidatorPubKey) {.
raises: [IOError, SerializationError].} =
var key: ValidatorPubKey
readValue(reader, key)

value = HashedValidatorPubKey.init(key)

proc writeValue*(
writer: var JsonWriter[RestJson], value: HashedValidatorPubKey) {.raises: [IOError].} =
writeValue(writer, value.pubkey)

## BitSeq
proc readValue*(reader: var JsonReader[RestJson], value: var BitSeq) {.
raises: [IOError, SerializationError].} =
Expand Down
41 changes: 41 additions & 0 deletions beacon_chain/spec/eth2_merkleization.nim
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@

import
stew/endians2,
std/sets,
ssz_serialization/[merkleization, proofs],
./ssz_codec

from ./datatypes/base import HashedValidatorPubKeyItem
from ./datatypes/phase0 import HashedBeaconState, SignedBeaconBlock
from ./datatypes/altair import HashedBeaconState, SignedBeaconBlock
from ./datatypes/bellatrix import HashedBeaconState, SignedBeaconBlock
Expand Down Expand Up @@ -66,3 +68,42 @@ func toDepositContractState*(merkleizer: DepositsMerkleizer): DepositContractSta

func getDepositsRoot*(m: var DepositsMerkleizer): Eth2Digest =
mixInLength(m.getFinalHash, int m.totalChunks)

func hash*(v: ref HashedValidatorPubKeyItem): Hash =
if not isNil(v):
hash(v[].key)
else:
default(Hash)

func `==`*(a, b: ref HashedValidatorPubKeyItem): bool =
if isNil(a):
isNil(b)
elif isNil(b):
false
else:
a[].key == b[].key

func init*(T: type HashedValidatorPubKey, key: ValidatorPubKey): HashedValidatorPubKey =
{.noSideEffect.}:
var keys {.threadvar.}: HashSet[ref HashedValidatorPubKeyItem]

let
tmp = (ref HashedValidatorPubKeyItem)(
key: key,
root: hash_tree_root(key)
)
cached =
# raising `KeyError` is 1000x slower at least than `'in`-checking..
if tmp in keys:
try:
# The interface of HashSet is such that we must construct a full
# instance to check if it's in the set - then we can return that
# instace and discard the one we just created temporarily
keys[tmp]
except KeyError:
raiseAssert "just checked"
else:
keys.incl tmp
tmp
arnetheduck marked this conversation as resolved.
Show resolved Hide resolved

HashedValidatorPubKey(value: addr cached[])
8 changes: 8 additions & 0 deletions beacon_chain/spec/eth2_ssz_serialization.nim
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,11 @@ func readSszBytes(
var res: T
readSszBytes(data, res, updateRoot)
res

proc fromSszBytes*(
T: type HashedValidatorPubKey, bytes: openArray[byte]
): T {.raises: [SszError].} =
let
key = ValidatorPubKey.fromSszBytes(bytes)

HashedValidatorPubKey.init(key)
2 changes: 2 additions & 0 deletions beacon_chain/spec/ssz_codec.nim
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,5 @@ func fromSszBytes*(
# TODO https://github.com/nim-lang/Nim/issues/21123
let tmp = cast[ptr List[ParticipationFlags, Limit VALIDATOR_REGISTRY_LIMIT]](addr result)
readSszValue(bytes, tmp[])

template toSszType*(v: HashedValidatorPubKey): auto = toRaw(v.pubkey)
2 changes: 1 addition & 1 deletion beacon_chain/statediff.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func applyValidatorIdentities(
hl: auto) =
for item in hl:
if not validators.add Validator(
pubkey: item.pubkey.toPubKey(),
pubkeyData: HashedValidatorPubKey.init(item.pubkey.toPubKey()),
withdrawal_credentials: item.withdrawal_credentials):
raiseAssert "cannot readd"

Expand Down
Loading